From f1b160db044060d93361331ddbf1c1b061efe52d Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 14 Aug 2023 14:32:55 -0700 Subject: [PATCH] remove backend wait(timeout) API The only use of this API was with a timeout 0 to check the fence status. Timeouts other than zero could be very dangerous and since we're not using that feature for now, we just get rid of it. wait() is replaced with getFenceStatus(). It is currently only used by the FrameSkipper. This is not a public API. --- filament/backend/include/private/backend/DriverAPI.inc | 2 +- filament/backend/src/metal/MetalDriver.mm | 4 ++-- filament/backend/src/noop/NoopDriver.cpp | 2 +- filament/backend/src/opengl/OpenGLDriver.cpp | 7 +++---- filament/backend/src/vulkan/VulkanDriver.cpp | 4 ++-- filament/backend/test/BackendTest.cpp | 5 +---- filament/backend/test/BackendTest.h | 2 +- filament/backend/test/ComputeTest.cpp | 5 +---- filament/backend/test/ComputeTest.h | 2 +- filament/src/FrameSkipper.cpp | 2 +- 10 files changed, 14 insertions(+), 21 deletions(-) diff --git a/filament/backend/include/private/backend/DriverAPI.inc b/filament/backend/include/private/backend/DriverAPI.inc index 150615767fb..2cb16a60fed 100644 --- a/filament/backend/include/private/backend/DriverAPI.inc +++ b/filament/backend/include/private/backend/DriverAPI.inc @@ -287,7 +287,7 @@ DECL_DRIVER_API_SYNCHRONOUS_N(void, setStreamDimensions, backend::StreamHandle, DECL_DRIVER_API_SYNCHRONOUS_N(int64_t, getStreamTimestamp, backend::StreamHandle, stream) DECL_DRIVER_API_SYNCHRONOUS_N(void, updateStreams, backend::DriverApi*, driver) DECL_DRIVER_API_SYNCHRONOUS_N(void, destroyFence, backend::FenceHandle, fh) -DECL_DRIVER_API_SYNCHRONOUS_N(backend::FenceStatus, wait, backend::FenceHandle, fh, uint64_t, timeout) +DECL_DRIVER_API_SYNCHRONOUS_N(backend::FenceStatus, getFenceStatus, backend::FenceHandle, fh) DECL_DRIVER_API_SYNCHRONOUS_N(bool, isTextureFormatSupported, backend::TextureFormat, format) DECL_DRIVER_API_SYNCHRONOUS_0(bool, isTextureSwizzleSupported) DECL_DRIVER_API_SYNCHRONOUS_N(bool, isTextureFormatMipmappable, backend::TextureFormat, format) diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index 0c3d9c43e9d..4476da77371 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -607,12 +607,12 @@ } } -FenceStatus MetalDriver::wait(Handle fh, uint64_t timeout) { +FenceStatus MetalDriver::getFenceStatus(Handle fh) { auto* fence = handle_cast(fh); if (!fence) { return FenceStatus::ERROR; } - return fence->wait(timeout); + return fence->wait(0); } bool MetalDriver::isTextureFormatSupported(TextureFormat format) { diff --git a/filament/backend/src/noop/NoopDriver.cpp b/filament/backend/src/noop/NoopDriver.cpp index 40a617f4a17..19b1cb5380f 100644 --- a/filament/backend/src/noop/NoopDriver.cpp +++ b/filament/backend/src/noop/NoopDriver.cpp @@ -132,7 +132,7 @@ void NoopDriver::updateStreams(CommandStream* driver) { void NoopDriver::destroyFence(Handle fh) { } -FenceStatus NoopDriver::wait(Handle fh, uint64_t timeout) { +FenceStatus NoopDriver::getFenceStatus(Handle fh) { return FenceStatus::CONDITION_SATISFIED; } diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index 86aa94c8a9f..47cc50edfc2 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -1682,7 +1682,7 @@ void OpenGLDriver::destroyFence(Handle fh) { } } -FenceStatus OpenGLDriver::wait(Handle fh, uint64_t timeout) { +FenceStatus OpenGLDriver::getFenceStatus(Handle fh) { if (fh) { GLFence* f = handle_cast(fh); if (mPlatform.canCreateFence() || mContext.isES2()) { @@ -1695,14 +1695,13 @@ FenceStatus OpenGLDriver::wait(Handle fh, uint64_t timeout) { // - wait() was called before the fence was asynchronously created. return FenceStatus::TIMEOUT_EXPIRED; } - return mPlatform.waitFence(f->fence, timeout); + return mPlatform.waitFence(f->fence, 0); } #ifndef FILAMENT_SILENCE_NOT_SUPPORTED_BY_ES2 else { assert_invariant(f->state); std::unique_lock lock(f->state->lock); - f->state->cond.wait_for(lock, - std::chrono::nanoseconds(timeout), [&state = f->state]() { + f->state->cond.wait_for(lock, std::chrono::nanoseconds(0), [&state = f->state]() { return state->status != FenceStatus::TIMEOUT_EXPIRED; }); return f->state->status; diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index f506ae59be6..374f6ffe876 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -664,7 +664,7 @@ void VulkanDriver::destroyFence(Handle fh) { mHandleAllocator.destruct(fh); } -FenceStatus VulkanDriver::wait(Handle fh, uint64_t timeout) { +FenceStatus VulkanDriver::getFenceStatus(Handle fh) { auto& cmdfence = mHandleAllocator.handle_cast(fh)->fence; if (!cmdfence) { // If wait is called before a fence actually exists, we return timeout. This matches the @@ -684,7 +684,7 @@ FenceStatus VulkanDriver::wait(Handle fh, uint64_t timeout) { lock.unlock(); } VkResult result = - vkWaitForFences(mPlatform->getDevice(), 1, &cmdfence->fence, VK_TRUE, timeout); + vkWaitForFences(mPlatform->getDevice(), 1, &cmdfence->fence, VK_TRUE, 0); return result == VK_SUCCESS ? FenceStatus::CONDITION_SATISFIED : FenceStatus::TIMEOUT_EXPIRED; } diff --git a/filament/backend/test/BackendTest.cpp b/filament/backend/test/BackendTest.cpp index 4e8309ca36c..c061d721f7f 100644 --- a/filament/backend/test/BackendTest.cpp +++ b/filament/backend/test/BackendTest.cpp @@ -85,13 +85,10 @@ void BackendTest::executeCommands() { } } -void BackendTest::flushAndWait(uint64_t timeout) { +void BackendTest::flushAndWait() { auto& api = getDriverApi(); - auto fence = api.createFence(); api.finish(); executeCommands(); - api.wait(fence, timeout); - api.destroyFence(fence); } Handle BackendTest::createSwapChain() { diff --git a/filament/backend/test/BackendTest.h b/filament/backend/test/BackendTest.h index d538301fd84..933c076d679 100644 --- a/filament/backend/test/BackendTest.h +++ b/filament/backend/test/BackendTest.h @@ -43,7 +43,7 @@ class BackendTest : public ::testing::Test { void initializeDriver(); void executeCommands(); - void flushAndWait(uint64_t timeout = 1000); + void flushAndWait(); filament::backend::Handle createSwapChain(); diff --git a/filament/backend/test/ComputeTest.cpp b/filament/backend/test/ComputeTest.cpp index 604bcecb50d..6ec640dbedd 100644 --- a/filament/backend/test/ComputeTest.cpp +++ b/filament/backend/test/ComputeTest.cpp @@ -87,11 +87,8 @@ void ComputeTest::executeCommands() { } } -void ComputeTest::flushAndWait(uint64_t timeout) { +void ComputeTest::flushAndWait() { auto& api = getDriverApi(); - auto fence = api.createFence(); api.finish(); executeCommands(); - api.wait(fence, timeout); - api.destroyFence(fence); } diff --git a/filament/backend/test/ComputeTest.h b/filament/backend/test/ComputeTest.h index 3eb48748367..83d14261135 100644 --- a/filament/backend/test/ComputeTest.h +++ b/filament/backend/test/ComputeTest.h @@ -34,7 +34,7 @@ class ComputeTest : public ::testing::Test { void TearDown() override; void executeCommands(); - void flushAndWait(uint64_t timeout = 1000); + void flushAndWait(); filament::backend::DriverApi& getDriverApi() { return *commandStream; } filament::backend::Driver& getDriver() { return *driver; } diff --git a/filament/src/FrameSkipper.cpp b/filament/src/FrameSkipper.cpp index acb27958cf5..7942ecfa5ad 100644 --- a/filament/src/FrameSkipper.cpp +++ b/filament/src/FrameSkipper.cpp @@ -43,7 +43,7 @@ bool FrameSkipper::beginFrame(DriverApi& driver) noexcept { auto& fences = mDelayedFences; auto fence = fences.front(); if (fence) { - auto status = driver.wait(fence, 0); + auto status = driver.getFenceStatus(fence); if (status == FenceStatus::TIMEOUT_EXPIRED) { // Sync not ready, skip frame return false;