From b056bc1a4be582c3ebad22bb642a8748f5fa408f Mon Sep 17 00:00:00 2001
From: Greggman <github@greggman.com>
Date: Tue, 10 Dec 2024 18:53:39 -0800
Subject: [PATCH] Address Comments for Timestamp example (#481)

It was mentioned that a previous PR did the wrong thing
by possibly applying a time more than once per timer query.
This PR addresses that issue by making the TimestampQueryManager
take a callback at initialization time that is called, only when
a value is available. The value is provided to the callback.
This pattern is similar to ResizeObserver

I agree with another comment, I kind of feel like it would
be more useful to support multiple queries. But, maybe as a
sample it's fine to support just 1.
---
 .../timestampQuery/TimestampQueryManager.ts   | 10 ++++-----
 sample/timestampQuery/main.ts                 | 22 +++++++++----------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/sample/timestampQuery/TimestampQueryManager.ts b/sample/timestampQuery/TimestampQueryManager.ts
index 4d0533d3..86f4a322 100644
--- a/sample/timestampQuery/TimestampQueryManager.ts
+++ b/sample/timestampQuery/TimestampQueryManager.ts
@@ -14,15 +14,15 @@ export default class TimestampQueryManager {
   // A buffer to map this result back to CPU
   #timestampMapBuffer: GPUBuffer;
 
-  // Last queried elapsed time of the pass in nanoseconds.
-  passDurationMeasurementNs: number;
+  // Callback to call when results are available.
+  #callback: (deltaTimeMs: number) => void;
 
   // Device must have the "timestamp-query" feature
-  constructor(device: GPUDevice) {
+  constructor(device: GPUDevice, callback: (deltaTimeNs: number) => void) {
     this.timestampSupported = device.features.has('timestamp-query');
     if (!this.timestampSupported) return;
 
-    this.passDurationMeasurementNs = 0;
+    this.#callback = callback;
 
     // Create timestamp queries
     this.#timestampQuerySet = device.createQuerySet({
@@ -101,7 +101,7 @@ export default class TimestampQueryManager {
       // It's possible elapsedNs is negative which means it's invalid
       // (see spec https://gpuweb.github.io/gpuweb/#timestamp)
       if (elapsedNs >= 0) {
-        this.passDurationMeasurementNs = elapsedNs;
+        this.#callback(elapsedNs);
       }
       buffer.unmap();
     });
diff --git a/sample/timestampQuery/main.ts b/sample/timestampQuery/main.ts
index 374966c0..9aee287c 100644
--- a/sample/timestampQuery/main.ts
+++ b/sample/timestampQuery/main.ts
@@ -33,7 +33,15 @@ quitIfWebGPUNotAvailable(adapter, device);
 // NB: Look for 'timestampQueryManager' in this file to locate parts of this
 // snippets that are related to timestamps. Most of the logic is in
 // TimestampQueryManager.ts.
-const timestampQueryManager = new TimestampQueryManager(device);
+const timestampQueryManager = new TimestampQueryManager(device, (elapsedNs) => {
+  // Convert from nanoseconds to milliseconds:
+  const elapsedMs = Number(elapsedNs) * 1e-6;
+  renderPassDurationCounter.addSample(elapsedMs);
+  perfDisplay.innerHTML = `Render Pass duration: ${renderPassDurationCounter
+    .getAverage()
+    .toFixed(3)} ms ± ${renderPassDurationCounter.getStddev().toFixed(3)} ms`;
+});
+
 const renderPassDurationCounter = new PerfCounter();
 
 const context = canvas.getContext('webgpu') as GPUCanvasContext;
@@ -209,17 +217,7 @@ function frame() {
 
   device.queue.submit([commandEncoder.finish()]);
 
-  if (timestampQueryManager.timestampSupported) {
-    // Show the last successfully downloaded elapsed time.
-    const elapsedNs = timestampQueryManager.passDurationMeasurementNs;
-    // Convert from nanoseconds to milliseconds:
-    const elapsedMs = Number(elapsedNs) * 1e-6;
-    renderPassDurationCounter.addSample(elapsedMs);
-    perfDisplay.innerHTML = `Render Pass duration: ${renderPassDurationCounter
-      .getAverage()
-      .toFixed(3)} ms ± ${renderPassDurationCounter.getStddev().toFixed(3)} ms`;
-  }
-
+  // Try to download the time stamp.
   timestampQueryManager.tryInitiateTimestampDownload();
 
   requestAnimationFrame(frame);