Address TQ PR Comments

Author: jc3265

filament/backend/src/webgpu/WGPUTimerQuery.cpp

@@ -19,40 +19,35 @@
 
 #include <chrono>
 
-
 namespace filament::backend {
 
-void WGPUTimerQuery::beginTimeElapsedQuery(WGPUTimerQuery* query) {
-    query->status->elapsed = 0;
-    query->status->available.store(false);
-
-    // Capture the timer query status via a weak_ptr because the MetalTimerQuery could be destroyed
+void WGPUTimerQuery::beginTimeElapsedQuery() {
+    status->elapsedNanoseconds = 0;
+    // Capture the timer query status via a weak_ptr because the WGPUTimerQuery could be destroyed
     // before the block executes.
-    std::weak_ptr<WGPUTimerQuery::Status> status = query->status;
+    std::weak_ptr<WGPUTimerQuery::Status> statusPtr = status;
 
-    if (auto s = status.lock()) {
-        s->elapsed = std::chrono::steady_clock::now().time_since_epoch().count();
+    if (auto s = statusPtr.lock()) {
+        s->elapsedNanoseconds = std::chrono::steady_clock::now().time_since_epoch().count();
     }
-
-    ;
 }
 
-void WGPUTimerQuery::endTimeElapsedQuery(WGPUTimerQuery* query) {
+void WGPUTimerQuery::endTimeElapsedQuery() {
     // Capture the timer query status via a weak_ptr because the WGPUTimerQuery could be destroyed
     // before the block executes.
-    std::weak_ptr<WGPUTimerQuery::Status> status = query->status;
-    if (auto s = status.lock()) {
-        s->elapsed = std::chrono::steady_clock::now().time_since_epoch().count() - s->elapsed;
-        s->available.store(true);
+    std::weak_ptr<WGPUTimerQuery::Status> statusPtr = status;
+    if (auto s = statusPtr.lock()) {
+        s->previousElapsed = s->elapsedNanoseconds = std::chrono::steady_clock::now().time_since_epoch().count() - s->elapsedNanoseconds;
     }
 }
 
-bool WGPUTimerQuery::getQueryResult(WGPUTimerQuery* query, uint64_t* outElapsedTime) {
-    if (!query->status->available.load()) {
+bool WGPUTimerQuery::getQueryResult(uint64_t* outElapsedTime) {
+    if (status->previousElapsed == 0) {
         return false;
     }
     if (outElapsedTime) {
-        *outElapsedTime = query->status->elapsed;
+        *outElapsedTime = status->previousElapsed;
+        status->previousElapsed = 0;
     }
     return true;
 }

filament/backend/src/webgpu/WebGPUDriver.cpp

@@ -15,6 +15,7 @@
  */
 
 #include "webgpu/WebGPUDriver.h"
+
 #include "WebGPUSwapChain.h"
 #include "webgpu/WebGPUConstants.h"
 #include "webgpu/WebGPUHandles.h"
@@ -30,14 +31,21 @@
 #include <utils/CString.h>
 #include <utils/ostream.h>
 
+#if FWGPU_ENABLED(FWGPU_PRINT_SYSTEM)
+#include <dawn/webgpu_cpp_print.h>
+#endif
 #include <webgpu/webgpu_cpp.h>
 
 #include <algorithm>
 #include <cstddef>
-
+#include <cstdint>
+#if FWGPU_ENABLED(FWGPU_PRINT_SYSTEM)
+#include <sstream>
+#include <string_view>
+#endif
 #include <utility>
 #include <variant>
-
+#include <iostream>
 namespace filament::backend {
 
 namespace {
@@ -357,6 +365,8 @@ Handle<HwFence> WebGPUDriver::createFenceS() noexcept {
 }
 
 Handle<HwTimerQuery> WebGPUDriver::createTimerQueryS() noexcept {
+    // The handle must be constructed here, as a synchronous call to getTimerQueryValue might happen
+    // before createTimerQueryR is executed.
     return allocAndConstructHandle<WGPUTimerQuery, HwTimerQuery>();
 }
 
@@ -371,18 +381,16 @@ void WebGPUDriver::destroyTimerQuery(Handle<HwTimerQuery> tqh) {
 }
 
 void WebGPUDriver::beginTimerQuery(Handle<HwTimerQuery> tqh) {
-    auto* tq = handleCast<WGPUTimerQuery>(tqh);
-    tq->beginTimeElapsedQuery(tq);
+    mTimerQuery = handleCast<WGPUTimerQuery>(tqh);
 }
 
 void WebGPUDriver::endTimerQuery(Handle<HwTimerQuery> tqh) {
-    auto* tq = handleCast<WGPUTimerQuery>(tqh);
-    tq->endTimeElapsedQuery(tq);
+    mTimerQuery = handleCast<WGPUTimerQuery>(tqh);
 }
 
 TimerQueryResult WebGPUDriver::getTimerQueryValue(Handle<HwTimerQuery> tqh, uint64_t* elapsedTime) {
     auto* tq = handleCast<WGPUTimerQuery>(tqh);
-    return tq->getQueryResult(tq, elapsedTime) ? TimerQueryResult::AVAILABLE
+    return tq->getQueryResult(elapsedTime) ? TimerQueryResult::AVAILABLE
                                                : TimerQueryResult::NOT_READY;
 }
 
@@ -719,6 +727,13 @@ void WebGPUDriver::endRenderPass(int) {
         .label = "command_buffer",
     };
     mCommandBuffer = mCommandEncoder.Finish(&commandBufferDescriptor);
+    mQueue.OnSubmittedWorkDone(wgpu::CallbackMode::AllowSpontaneous, [this](auto const& status) {
+        if (status == wgpu::QueueWorkDoneStatus::Success) {
+            if (mTimerQuery) {
+                mTimerQuery->endTimeElapsedQuery();
+            }
+        }
+    });
     assert_invariant(mCommandBuffer);
 }
 
@@ -741,6 +756,7 @@ void WebGPUDriver::makeCurrent(Handle<HwSwapChain> drawSch, Handle<HwSwapChain>
 
 void WebGPUDriver::commit(Handle<HwSwapChain> sch) {
     mCommandEncoder = nullptr;
+    mTimerQuery->beginTimeElapsedQuery();
     mQueue.Submit(1, &mCommandBuffer);
     mCommandBuffer = nullptr;
     mTextureView = nullptr;

filament/backend/src/webgpu/WebGPUDriver.h

@@ -39,7 +39,9 @@
 namespace filament::backend {
 
 class WebGPUSwapChain;
+class WGPUTimerQuery;
 struct WGPURenderTarget;
+
 /**
  * WebGPU backend (driver) implementation
  */
@@ -92,7 +94,7 @@ class WebGPUDriver final : public DriverBase {
     wgpu::RenderPassEncoder mRenderPassEncoder = nullptr;
     wgpu::CommandBuffer mCommandBuffer = nullptr;
     WGPURenderTarget* mDefaultRenderTarget = nullptr;
-
+    WGPUTimerQuery* mTimerQuery = nullptr;
 
     /*
      * Driver interface

filament/backend/src/webgpu/WebGPUHandles.h

@@ -25,9 +25,10 @@
 
 #include <utils/FixedCapacityVector.h>
 
+#include <webgpu/webgpu_cpp.h>
+
 #include <cstdint>
 #include <vector>
-#include <webgpu/webgpu_cpp.h>
 
 namespace filament::backend {
 
@@ -161,13 +162,14 @@ class WGPUTimerQuery : public HwTimerQuery {
     WGPUTimerQuery()
         : status(std::make_shared<Status>()) {}
 
-    void beginTimeElapsedQuery(WGPUTimerQuery* timerQuery);
-    void endTimeElapsedQuery(WGPUTimerQuery* timerQuery);
-    bool getQueryResult(WGPUTimerQuery* query, uint64_t* outElapsedTime);
+    void beginTimeElapsedQuery();
+    void endTimeElapsedQuery();
+    bool getQueryResult(uint64_t* outElapsedTimeNanoseconds);
 
+private:
     struct Status {
-        std::atomic<bool> available{ false };
-        std::atomic<uint64_t> elapsed{ 0 };// only valid if available is true
+        std::atomic<uint64_t> elapsedNanoseconds{ 0 };
+        std::atomic<uint64_t> previousElapsed{ 0 };
     };
 
     std::shared_ptr<Status> status;