From 613637dcaef38b166214cceb2ca7e2acda1a6325 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Mon, 4 Mar 2024 10:49:12 +1100 Subject: [PATCH 1/9] Add AsyncSystem::waitInMainThread. --- CesiumAsync/include/CesiumAsync/AsyncSystem.h | 26 +++++++++++++++++++ CesiumAsync/include/CesiumAsync/Future.h | 3 +++ CesiumAsync/src/AsyncSystem.cpp | 5 ++++ CesiumAsync/test/TestAsyncSystem.cpp | 12 +++++++++ 4 files changed, 46 insertions(+) diff --git a/CesiumAsync/include/CesiumAsync/AsyncSystem.h b/CesiumAsync/include/CesiumAsync/AsyncSystem.h index 76f9d5696..7e98ad64c 100644 --- a/CesiumAsync/include/CesiumAsync/AsyncSystem.h +++ b/CesiumAsync/include/CesiumAsync/AsyncSystem.h @@ -268,6 +268,30 @@ class CESIUMASYNC_API AsyncSystem final { */ bool dispatchOneMainThreadTask(); + /** + * @brief Waits for the given future to resolve or reject in the main thread + * while also processing main-thread tasks. + * + * The function does not return until {@link Future::isReady} returns true. + * In the meantime, main-thread tasks are processed by calling + * {@link dispatchMainThreadTasks} repeatedly. Rather than occupying process + * time spin waiting, time slices are given up while waiting by sleeping the + * thread for 0ms. + * + * @return The value if the future resolves successfully. + * @throws An exception if the future rejected. + */ + template T waitInMainThread(Future&& future) { + while (!future.isReady()) { + this->dispatchMainThreadTasks(); + if (future.isReady()) + break; + this->giveUpTimeSlice(); + } + + return future.wait(); + } + /** * @brief Creates a new thread pool that can be used to run continuations. * @@ -322,6 +346,8 @@ class CESIUMASYNC_API AsyncSystem final { return Future>(this->_pSchedulers, std::move(task)); } + void giveUpTimeSlice() const noexcept; + std::shared_ptr _pSchedulers; template friend class Future; diff --git a/CesiumAsync/include/CesiumAsync/Future.h b/CesiumAsync/include/CesiumAsync/Future.h index 13347f87a..a3adc0644 100644 --- a/CesiumAsync/include/CesiumAsync/Future.h +++ b/CesiumAsync/include/CesiumAsync/Future.h @@ -233,6 +233,9 @@ template class Future final { * deadlock because the main thread tasks will never complete while this * method is blocking the main thread. * + * To wait in the main thread, use {@link AsyncSystem::waitInMainThread} + * instead. + * * @return The value if the future resolves successfully. * @throws An exception if the future rejected. */ diff --git a/CesiumAsync/src/AsyncSystem.cpp b/CesiumAsync/src/AsyncSystem.cpp index 7446d9d8d..70713c895 100644 --- a/CesiumAsync/src/AsyncSystem.cpp +++ b/CesiumAsync/src/AsyncSystem.cpp @@ -3,6 +3,7 @@ #include "CesiumAsync/ITaskProcessor.h" #include +#include namespace CesiumAsync { AsyncSystem::AsyncSystem( @@ -31,4 +32,8 @@ bool AsyncSystem::operator!=(const AsyncSystem& rhs) const noexcept { return this->_pSchedulers != rhs._pSchedulers; } +void AsyncSystem::giveUpTimeSlice() const noexcept { + std::this_thread::sleep_for(std::chrono::duration(0.0)); +} + } // namespace CesiumAsync diff --git a/CesiumAsync/test/TestAsyncSystem.cpp b/CesiumAsync/test/TestAsyncSystem.cpp index a6656a2c6..f1179b185 100644 --- a/CesiumAsync/test/TestAsyncSystem.cpp +++ b/CesiumAsync/test/TestAsyncSystem.cpp @@ -572,4 +572,16 @@ TEST_CASE("AsyncSystem") { CHECK(checksCompleted); } + + SECTION("waitInMainThread") { + bool called = false; + auto future = + asyncSystem.createResolvedFuture().thenInMainThread([&called]() { + called = true; + return 4; + }); + int value = asyncSystem.waitInMainThread(std::move(future)); + CHECK(called); + CHECK(value == 4); + } } From e4462336ea7f9ffeef19b3e163025c466fc28e4d Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Mon, 4 Mar 2024 19:09:24 +1100 Subject: [PATCH 2/9] Update CHANGES.md. --- CHANGES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 4d70588f4..7e5781edc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,11 @@ # Change Log +### ? - ? + +##### Additions :tada: + +- Added `waitInMainThread` method to `AsyncSystem`. + ### v0.33.0 - 2024-03-01 ##### Breaking Changes :mega: From 0428735c98f4a957733a3998cb0ed8022f38c6c9 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 4 Apr 2024 19:04:10 +1100 Subject: [PATCH 3/9] No spin waiting, add SharedFuture::waitInMainThread. --- CesiumAsync/include/CesiumAsync/AsyncSystem.h | 26 -------- CesiumAsync/include/CesiumAsync/Future.h | 22 ++++++- .../CesiumAsync/Impl/QueuedScheduler.h | 43 ++++++++++++- .../include/CesiumAsync/SharedFuture.h | 21 +++++++ CesiumAsync/src/AsyncSystem.cpp | 5 -- CesiumAsync/src/QueuedScheduler.cpp | 61 +++++++++++++++++-- CesiumAsync/test/TestAsyncSystem.cpp | 51 +++++++++++++--- 7 files changed, 180 insertions(+), 49 deletions(-) diff --git a/CesiumAsync/include/CesiumAsync/AsyncSystem.h b/CesiumAsync/include/CesiumAsync/AsyncSystem.h index 7e98ad64c..76f9d5696 100644 --- a/CesiumAsync/include/CesiumAsync/AsyncSystem.h +++ b/CesiumAsync/include/CesiumAsync/AsyncSystem.h @@ -268,30 +268,6 @@ class CESIUMASYNC_API AsyncSystem final { */ bool dispatchOneMainThreadTask(); - /** - * @brief Waits for the given future to resolve or reject in the main thread - * while also processing main-thread tasks. - * - * The function does not return until {@link Future::isReady} returns true. - * In the meantime, main-thread tasks are processed by calling - * {@link dispatchMainThreadTasks} repeatedly. Rather than occupying process - * time spin waiting, time slices are given up while waiting by sleeping the - * thread for 0ms. - * - * @return The value if the future resolves successfully. - * @throws An exception if the future rejected. - */ - template T waitInMainThread(Future&& future) { - while (!future.isReady()) { - this->dispatchMainThreadTasks(); - if (future.isReady()) - break; - this->giveUpTimeSlice(); - } - - return future.wait(); - } - /** * @brief Creates a new thread pool that can be used to run continuations. * @@ -346,8 +322,6 @@ class CESIUMASYNC_API AsyncSystem final { return Future>(this->_pSchedulers, std::move(task)); } - void giveUpTimeSlice() const noexcept; - std::shared_ptr _pSchedulers; template friend class Future; diff --git a/CesiumAsync/include/CesiumAsync/Future.h b/CesiumAsync/include/CesiumAsync/Future.h index a3adc0644..59d9d6cbc 100644 --- a/CesiumAsync/include/CesiumAsync/Future.h +++ b/CesiumAsync/include/CesiumAsync/Future.h @@ -233,14 +233,32 @@ template class Future final { * deadlock because the main thread tasks will never complete while this * method is blocking the main thread. * - * To wait in the main thread, use {@link AsyncSystem::waitInMainThread} - * instead. + * To wait in the main thread, use {@link waitInMainThread} instead. * * @return The value if the future resolves successfully. * @throws An exception if the future rejected. */ T wait() { return this->_task.get(); } + /** + * @brief Waits for this future to resolve or reject in the main thread while + * also processing main-thread tasks. + * + * This method must be called from the main thread. + * + * The function does not return until {@link Future::isReady} returns true. + * In the meantime, main-thread tasks are processed as necessary. This method + * does not spin wait; it suspends the calling thread by waiting on a + * condition variable when there is no work to do. + * + * @return The value if the future resolves successfully. + * @throws An exception if the future rejected. + */ + T waitInMainThread() { + return this->_pSchedulers->mainThread.dispatchUntilTaskCompletes( + std::move(this->_task)); + } + /** * @brief Determines if this future is already resolved or rejected. * diff --git a/CesiumAsync/include/CesiumAsync/Impl/QueuedScheduler.h b/CesiumAsync/include/CesiumAsync/Impl/QueuedScheduler.h index 6db6d4fdb..d1c370c3c 100644 --- a/CesiumAsync/include/CesiumAsync/Impl/QueuedScheduler.h +++ b/CesiumAsync/include/CesiumAsync/Impl/QueuedScheduler.h @@ -8,14 +8,55 @@ namespace CesiumImpl { class QueuedScheduler { public: + QueuedScheduler(); + ~QueuedScheduler(); + void schedule(async::task_run_handle t); void dispatchQueuedContinuations(); bool dispatchZeroOrOneContinuation(); + template T dispatchUntilTaskCompletes(async::task&& task) { + // Set up a continuation to unblock the blocking dispatch when this task + // completes. + async::task unblockTask = + task.then(async::inline_scheduler(), [this](auto&& value) { + this->unblock(); + return std::move(value); + }); + + while (!unblockTask.ready()) { + this->dispatchInternal(true); + } + + return std::move(unblockTask).get(); + } + + template + T dispatchUntilTaskCompletes(const async::shared_task& task) { + // Set up a continuation to unblock the blocking dispatch when this task + // completes. Unlike the non-shared future case above, we don't need to pass + // the future value through because shared_task supports multiple + // continuations. + async::task unblockTask = + task.then(async::inline_scheduler(), [this](const auto&) { + this->unblock(); + }); + + while (!task.ready()) { + this->dispatchInternal(true); + } + + return task.get(); + } + ImmediateScheduler immediate{this}; private: - async::fifo_scheduler _scheduler; + bool dispatchInternal(bool blockIfNoTasks); + void unblock(); + + struct Impl; + std::unique_ptr _pImpl; }; } // namespace CesiumImpl diff --git a/CesiumAsync/include/CesiumAsync/SharedFuture.h b/CesiumAsync/include/CesiumAsync/SharedFuture.h index 0ce84ddf0..6f6ea814b 100644 --- a/CesiumAsync/include/CesiumAsync/SharedFuture.h +++ b/CesiumAsync/include/CesiumAsync/SharedFuture.h @@ -217,6 +217,8 @@ template class SharedFuture final { * deadlock because the main thread tasks will never complete while this * method is blocking the main thread. * + * To wait in the main thread, use {@link waitInMainThread} instead. + * * @return The value if the future resolves successfully. * @throws An exception if the future rejected. */ @@ -236,6 +238,25 @@ template class SharedFuture final { this->_task.get(); } + /** + * @brief Waits for this future to resolve or reject in the main thread while + * also processing main-thread tasks. + * + * This method must be called from the main thread. + * + * The function does not return until {@link Future::isReady} returns true. + * In the meantime, main-thread tasks are processed as necessary. This method + * does not spin wait; it suspends the calling thread by waiting on a + * condition variable when there is no work to do. + * + * @return The value if the future resolves successfully. + * @throws An exception if the future rejected. + */ + T waitInMainThread() { + return this->_pSchedulers->mainThread.dispatchUntilTaskCompletes( + std::move(this->_task)); + } + /** * @brief Determines if this future is already resolved or rejected. * diff --git a/CesiumAsync/src/AsyncSystem.cpp b/CesiumAsync/src/AsyncSystem.cpp index 70713c895..0bfd60ed4 100644 --- a/CesiumAsync/src/AsyncSystem.cpp +++ b/CesiumAsync/src/AsyncSystem.cpp @@ -2,7 +2,6 @@ #include "CesiumAsync/ITaskProcessor.h" -#include #include namespace CesiumAsync { @@ -32,8 +31,4 @@ bool AsyncSystem::operator!=(const AsyncSystem& rhs) const noexcept { return this->_pSchedulers != rhs._pSchedulers; } -void AsyncSystem::giveUpTimeSlice() const noexcept { - std::this_thread::sleep_for(std::chrono::duration(0.0)); -} - } // namespace CesiumAsync diff --git a/CesiumAsync/src/QueuedScheduler.cpp b/CesiumAsync/src/QueuedScheduler.cpp index ceae230c0..a83399b6d 100644 --- a/CesiumAsync/src/QueuedScheduler.cpp +++ b/CesiumAsync/src/QueuedScheduler.cpp @@ -1,17 +1,66 @@ #include "CesiumAsync/Impl/QueuedScheduler.h" -using namespace CesiumAsync::CesiumImpl; +#include +#include + +// Hackily use Async++'s internal fifo_queue. We could copy it instead - it's +// not much code - but why create the duplication? However, we are assuming that +// Async++'s source (not just headers) are available while building +// cesium-native. If that's a problem in some context, we'll need to do that +// duplication of fifo_queue after all. +#include + +namespace CesiumAsync::CesiumImpl { + +struct QueuedScheduler::Impl { + async::detail::fifo_queue queue; + std::mutex mutex; + std::condition_variable conditionVariable; +}; + +QueuedScheduler::QueuedScheduler() : _pImpl(std::make_unique()) {} +QueuedScheduler::~QueuedScheduler() = default; void QueuedScheduler::schedule(async::task_run_handle t) { - this->_scheduler.schedule(std::move(t)); + std::lock_guard guard(this->_pImpl->mutex); + this->_pImpl->queue.push(std::move(t)); + + // Notify listeners that there is new work. + this->_pImpl->conditionVariable.notify_all(); } void QueuedScheduler::dispatchQueuedContinuations() { - auto scope = this->immediate.scope(); - this->_scheduler.run_all_tasks(); + while (this->dispatchZeroOrOneContinuation()) { + } } bool QueuedScheduler::dispatchZeroOrOneContinuation() { - auto scope = this->immediate.scope(); - return this->_scheduler.try_run_one_task(); + return this->dispatchInternal(false); +} + +bool QueuedScheduler::dispatchInternal(bool blockIfNoTasks) { + async::task_run_handle t; + + { + std::unique_lock guard(this->_pImpl->mutex); + t = this->_pImpl->queue.pop(); + if (blockIfNoTasks && !t) { + this->_pImpl->conditionVariable.wait(guard); + } + } + + if (t) { + auto scope = this->immediate.scope(); + t.run(); + return true; + } else { + return false; + } } + +void QueuedScheduler::unblock() { + std::lock_guard guard(this->_pImpl->mutex); + this->_pImpl->conditionVariable.notify_all(); +} + +} // namespace CesiumAsync::CesiumImpl diff --git a/CesiumAsync/test/TestAsyncSystem.cpp b/CesiumAsync/test/TestAsyncSystem.cpp index f1179b185..775a53cb1 100644 --- a/CesiumAsync/test/TestAsyncSystem.cpp +++ b/CesiumAsync/test/TestAsyncSystem.cpp @@ -574,14 +574,47 @@ TEST_CASE("AsyncSystem") { } SECTION("waitInMainThread") { - bool called = false; - auto future = - asyncSystem.createResolvedFuture().thenInMainThread([&called]() { - called = true; - return 4; - }); - int value = asyncSystem.waitInMainThread(std::move(future)); - CHECK(called); - CHECK(value == 4); + SECTION("Future returning a value") { + bool called = false; + Future future = + asyncSystem.createResolvedFuture().thenInMainThread([&called]() { + called = true; + return 4; + }); + int value = std::move(future).waitInMainThread(); + CHECK(called); + CHECK(value == 4); + } + + SECTION("Future returning void") { + bool called = false; + Future future = asyncSystem.createResolvedFuture().thenInMainThread( + [&called]() { called = true; }); + std::move(future).waitInMainThread(); + CHECK(called); + } + + SECTION("SharedFuture returning a value") { + bool called = false; + SharedFuture future = asyncSystem.createResolvedFuture() + .thenInMainThread([&called]() { + called = true; + return 4; + }) + .share(); + int value = future.waitInMainThread(); + CHECK(called); + CHECK(value == 4); + } + + SECTION("SharedFuture returning void") { + bool called = false; + SharedFuture future = + asyncSystem.createResolvedFuture() + .thenInMainThread([&called]() { called = true; }) + .share(); + future.waitInMainThread(); + CHECK(called); + } } } From 2b14da94ae538ca5729372562fe6afa7743f2780 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 4 Apr 2024 19:19:02 +1100 Subject: [PATCH 4/9] Update CHANGES.md. --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 7736fbfdf..d65e85187 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -16,7 +16,7 @@ ##### Additions :tada: -- Added `waitInMainThread` method to `AsyncSystem`. +- Added `waitInMainThread` method to `Future` and `SharedFuture`. ##### Fixes :wrench: From 942af01dc51ded6a9ec60705043ef601d7635b75 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 4 Apr 2024 19:26:59 +1100 Subject: [PATCH 5/9] Move changes to the right release. --- CHANGES.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d65e85187..9bc7db08c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,11 @@ # Change Log +### ? - ? + +##### Additions :tada: + +- Added `waitInMainThread` method to `Future` and `SharedFuture`. + ### v0.34.0 - 2024-04-01 ##### Breaking Changes :mega: @@ -14,10 +20,6 @@ - Added overloads of `ImplicitTilingUtilities::computeBoundingVolume` that take an `S2CellBoundingVolume` and an `OctreeTileID`. Previously only `QuadtreeTileID` was supported. - Added `setOrientedBoundingBox`, `setBoundingRegion`, `setBoundingSphere`, and `setS2CellBoundingVolume` functions to `TileBoundingVolumes`. -##### Additions :tada: - -- Added `waitInMainThread` method to `Future` and `SharedFuture`. - ##### Fixes :wrench: - Fixed a bug where coordinates returned from `SimplePlanarEllipsoidCurve` were inverted if one of the input points had a negative height. From cd232f6b504b482351fdb78e30db341d8e8fb189 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Thu, 4 Apr 2024 21:33:28 +1100 Subject: [PATCH 6/9] Add a couple more tests, remove unneeded header. --- CesiumAsync/src/AsyncSystem.cpp | 2 -- CesiumAsync/test/TestAsyncSystem.cpp | 47 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/CesiumAsync/src/AsyncSystem.cpp b/CesiumAsync/src/AsyncSystem.cpp index 0bfd60ed4..551d200f5 100644 --- a/CesiumAsync/src/AsyncSystem.cpp +++ b/CesiumAsync/src/AsyncSystem.cpp @@ -2,8 +2,6 @@ #include "CesiumAsync/ITaskProcessor.h" -#include - namespace CesiumAsync { AsyncSystem::AsyncSystem( const std::shared_ptr& pTaskProcessor) noexcept diff --git a/CesiumAsync/test/TestAsyncSystem.cpp b/CesiumAsync/test/TestAsyncSystem.cpp index 775a53cb1..23ca9a913 100644 --- a/CesiumAsync/test/TestAsyncSystem.cpp +++ b/CesiumAsync/test/TestAsyncSystem.cpp @@ -616,5 +616,52 @@ TEST_CASE("AsyncSystem") { future.waitInMainThread(); CHECK(called); } + + SECTION("Future resolving while main thread is waiting") { + bool called1 = false; + bool called2 = false; + Future future = + asyncSystem.createResolvedFuture() + .thenInWorkerThread([&called1]() { + using namespace std::chrono_literals; + // should be long enough for the main thread to start waiting on + // the conditional, without slowing the test down too much. + std::this_thread::sleep_for(20ms); + called1 = true; + }) + .thenInMainThread([&called2]() { called2 = true; }); + future.waitInMainThread(); + CHECK(called1); + CHECK(called2); + } + + SECTION("Future resolving from a worker while main thread is waiting") { + bool called1 = false; + bool called2 = false; + bool called3 = false; + Future future = + asyncSystem.createResolvedFuture() + .thenInWorkerThread([&called1]() { + using namespace std::chrono_literals; + // should be long enough for the main thread to start waiting on + // the conditional, without slowing the test down too much. + std::this_thread::sleep_for(20ms); + called1 = true; + }) + .thenInMainThread([&called2]() { called2 = true; }) + .thenInWorkerThread([&called3]() { + using namespace std::chrono_literals; + // Sufficient time for the main thread to drop back into waiting + // on the conditional again after it was awakened by the + // scheduling of the main thread continuation above. It should + // awaken again when this continuation completes. + std::this_thread::sleep_for(20ms); + called3 = true; + }); + future.waitInMainThread(); + CHECK(called1); + CHECK(called2); + CHECK(called3); + } } } From c09e2f07579c6f9071ea95f303f7f57088689cba Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Fri, 5 Apr 2024 14:57:55 +1100 Subject: [PATCH 7/9] Fix race condition. --- .../CesiumAsync/Impl/QueuedScheduler.h | 38 ++++++++++++++++--- CesiumAsync/src/QueuedScheduler.cpp | 4 +- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/CesiumAsync/include/CesiumAsync/Impl/QueuedScheduler.h b/CesiumAsync/include/CesiumAsync/Impl/QueuedScheduler.h index d1c370c3c..91302714e 100644 --- a/CesiumAsync/include/CesiumAsync/Impl/QueuedScheduler.h +++ b/CesiumAsync/include/CesiumAsync/Impl/QueuedScheduler.h @@ -3,6 +3,8 @@ #include "ImmediateScheduler.h" #include "cesium-async++.h" +#include + namespace CesiumAsync { namespace CesiumImpl { @@ -18,13 +20,38 @@ class QueuedScheduler { template T dispatchUntilTaskCompletes(async::task&& task) { // Set up a continuation to unblock the blocking dispatch when this task // completes. + // + // We use the `isDone` flag as the loop termination condition to + // avoid a race condition that can lead to a deadlock. If we used + // `unblockTask.ready()` as the termination condition instead, then it's + // possible for events to happen as follows: + // + // 1. The original `task` completes in a worker thread and the `unblockTask` + // continuation is invoked immediately in the same thread. + // 2. The unblockTask continuation calls `unblock`, which terminates the + // `wait` on the condition variable in the main thread. + // 3. The main thread resumes and the while loop in this function spins back + // around and evaluates `unblockTask.ready()`. This returns false because + // the unblockTask continuation has not actually finished running in the + // worker thread yet. The main thread starts waiting on the condition + // variable again. + // 4. The `unblockTask` continuation finally finishes, making + // `unblockTask.ready()` return true, but it's too late. The main thread is + // already waiting on the condition variable. + // + // By setting the atomic `isDone` flag before calling `unblock`, we ensure + // that the loop termination condition is satisfied before the main thread + // is awoken, avoiding the potential deadlock. + + std::atomic isDone = false; async::task unblockTask = - task.then(async::inline_scheduler(), [this](auto&& value) { + task.then(async::inline_scheduler(), [this, &isDone](auto&& value) { + isDone = true; this->unblock(); return std::move(value); }); - while (!unblockTask.ready()) { + while (!isDone) { this->dispatchInternal(true); } @@ -34,9 +61,10 @@ class QueuedScheduler { template T dispatchUntilTaskCompletes(const async::shared_task& task) { // Set up a continuation to unblock the blocking dispatch when this task - // completes. Unlike the non-shared future case above, we don't need to pass - // the future value through because shared_task supports multiple - // continuations. + // completes. This case is simpler than the one above because a SharedFuture + // supports multiple continuations. We can use readiness of the _original_ + // task to terminate the loop while unblocking in a separate continuation + // guaranteed to run only after that termination condition is satisfied. async::task unblockTask = task.then(async::inline_scheduler(), [this](const auto&) { this->unblock(); diff --git a/CesiumAsync/src/QueuedScheduler.cpp b/CesiumAsync/src/QueuedScheduler.cpp index a83399b6d..f8cd95093 100644 --- a/CesiumAsync/src/QueuedScheduler.cpp +++ b/CesiumAsync/src/QueuedScheduler.cpp @@ -22,7 +22,7 @@ QueuedScheduler::QueuedScheduler() : _pImpl(std::make_unique()) {} QueuedScheduler::~QueuedScheduler() = default; void QueuedScheduler::schedule(async::task_run_handle t) { - std::lock_guard guard(this->_pImpl->mutex); + std::unique_lock guard(this->_pImpl->mutex); this->_pImpl->queue.push(std::move(t)); // Notify listeners that there is new work. @@ -59,7 +59,7 @@ bool QueuedScheduler::dispatchInternal(bool blockIfNoTasks) { } void QueuedScheduler::unblock() { - std::lock_guard guard(this->_pImpl->mutex); + std::unique_lock guard(this->_pImpl->mutex); this->_pImpl->conditionVariable.notify_all(); } From ee357293c06a0b0c1c85963a6d230cf7ef92a143 Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Mon, 8 Apr 2024 14:24:31 +1000 Subject: [PATCH 8/9] Updates to generator config to work with updated glTF repo. --- .../ExtensionKhrDracoMeshCompression.h | 2 +- .../ExtensionMeshPrimitiveKhrMaterialsVariants.h | 2 +- tools/generate-classes/glTF.json | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/CesiumGltf/generated/include/CesiumGltf/ExtensionKhrDracoMeshCompression.h b/CesiumGltf/generated/include/CesiumGltf/ExtensionKhrDracoMeshCompression.h index a2d67aa44..08a869177 100644 --- a/CesiumGltf/generated/include/CesiumGltf/ExtensionKhrDracoMeshCompression.h +++ b/CesiumGltf/generated/include/CesiumGltf/ExtensionKhrDracoMeshCompression.h @@ -11,7 +11,7 @@ namespace CesiumGltf { /** - * @brief KHR_draco_mesh_compression extension + * @brief KHR_draco_mesh_compression glTF Mesh Primitive Extension */ struct CESIUMGLTF_API ExtensionKhrDracoMeshCompression final : public CesiumUtility::ExtensibleObject { diff --git a/CesiumGltf/generated/include/CesiumGltf/ExtensionMeshPrimitiveKhrMaterialsVariants.h b/CesiumGltf/generated/include/CesiumGltf/ExtensionMeshPrimitiveKhrMaterialsVariants.h index c9cc7023b..0b10bb312 100644 --- a/CesiumGltf/generated/include/CesiumGltf/ExtensionMeshPrimitiveKhrMaterialsVariants.h +++ b/CesiumGltf/generated/include/CesiumGltf/ExtensionMeshPrimitiveKhrMaterialsVariants.h @@ -11,7 +11,7 @@ namespace CesiumGltf { /** - * @brief KHR_materials_variants mesh primitive extension + * @brief KHR_materials_variants glTF Mesh Primitive Extension */ struct CESIUMGLTF_API ExtensionMeshPrimitiveKhrMaterialsVariants final : public CesiumUtility::ExtensibleObject { diff --git a/tools/generate-classes/glTF.json b/tools/generate-classes/glTF.json index 0ff787f41..68bb6e80b 100644 --- a/tools/generate-classes/glTF.json +++ b/tools/generate-classes/glTF.json @@ -27,7 +27,7 @@ "CESIUM_RTC": { "overrideName": "ExtensionCesiumRTC" }, - "KHR_texture_basisu glTF extension": { + "KHR_texture_basisu glTF Texture Extension": { "overrideName": "ExtensionKhrTextureBasisu" }, "EXT_texture_webp glTF extension": { @@ -108,16 +108,16 @@ "No Data Value": { "overrideName": "CesiumUtility::JsonValue" }, - "KHR_draco_mesh_compression extension": { + "KHR_draco_mesh_compression glTF Mesh Primitive Extension": { "overrideName": "ExtensionKhrDracoMeshCompression" }, - "KHR_materials_variants glTF extension": { + "KHR_materials_variants glTF Document Extension": { "overrideName": "ExtensionModelKhrMaterialsVariants" }, - "KHR_materials_variants mesh primitive extension": { + "KHR_materials_variants glTF Mesh Primitive Extension": { "overrideName": "ExtensionMeshPrimitiveKhrMaterialsVariants" }, - "KHR_materials_unlit glTF extension": { + "KHR_materials_unlit glTF Material Extension": { "overrideName": "ExtensionKhrMaterialsUnlit" }, "MAXAR_mesh_variants glTF extension": { @@ -126,7 +126,7 @@ "MAXAR_mesh_variants node extension": { "overrideName": "ExtensionNodeMaxarMeshVariants" }, - "KHR_texture_transform textureInfo extension": { + "KHR_texture_transform glTF TextureInfo Extension": { "overrideName": "ExtensionKhrTextureTransform" } }, @@ -179,7 +179,7 @@ }, { "extensionName": "KHR_materials_unlit", - "schema": "Khronos/KHR_materials_unlit/schema/glTF.KHR_materials_unlit.schema.json", + "schema": "Khronos/KHR_materials_unlit/schema/material.KHR_materials_unlit.schema.json", "attachTo": [ "material" ] @@ -215,7 +215,7 @@ }, { "extensionName": "KHR_texture_transform", - "schema": "Khronos/KHR_texture_transform/schema/KHR_texture_transform.textureInfo.schema.json", + "schema": "Khronos/KHR_texture_transform/schema/textureInfo.KHR_texture_transform.schema.json", "attachTo": [ "textureInfo", "material.occlusionTextureInfo", From 66ad2c90394423fad6fbe19d8f9eddfc7a4818ee Mon Sep 17 00:00:00 2001 From: Kevin Ring Date: Mon, 8 Apr 2024 14:46:14 +1000 Subject: [PATCH 9/9] Add support for CESIUM_primitive_outline extension. --- .../ExtensionCesiumPrimitiveOutline.h | 29 +++++ .../ExtensionCesiumPrimitiveOutlineReader.h | 76 +++++++++++++ ...tensionCesiumPrimitiveOutlineJsonHandler.h | 48 ++++++++ .../generated/src/GeneratedJsonHandlers.cpp | 103 ++++++++++++++++++ .../generated/src/registerExtensions.cpp | 4 + .../generated/src/ModelJsonWriter.cpp | 29 +++++ .../generated/src/ModelJsonWriter.h | 13 +++ .../generated/src/registerExtensions.cpp | 4 + tools/generate-classes/glTF.json | 10 ++ 9 files changed, 316 insertions(+) create mode 100644 CesiumGltf/generated/include/CesiumGltf/ExtensionCesiumPrimitiveOutline.h create mode 100644 CesiumGltfReader/generated/include/CesiumGltfReader/ExtensionCesiumPrimitiveOutlineReader.h create mode 100644 CesiumGltfReader/generated/src/ExtensionCesiumPrimitiveOutlineJsonHandler.h diff --git a/CesiumGltf/generated/include/CesiumGltf/ExtensionCesiumPrimitiveOutline.h b/CesiumGltf/generated/include/CesiumGltf/ExtensionCesiumPrimitiveOutline.h new file mode 100644 index 000000000..83bb9cd6d --- /dev/null +++ b/CesiumGltf/generated/include/CesiumGltf/ExtensionCesiumPrimitiveOutline.h @@ -0,0 +1,29 @@ +// This file was generated by generate-classes. +// DO NOT EDIT THIS FILE! +#pragma once + +#include "CesiumGltf/Library.h" + +#include + +#include + +namespace CesiumGltf { +/** + * @brief glTF extension for indicating that some edges of a primitive's + * triangles should be outlined. + */ +struct CESIUMGLTF_API ExtensionCesiumPrimitiveOutline final + : public CesiumUtility::ExtensibleObject { + static inline constexpr const char* TypeName = + "ExtensionCesiumPrimitiveOutline"; + static inline constexpr const char* ExtensionName = + "CESIUM_primitive_outline"; + + /** + * @brief The index of the accessor providing the list of highlighted lines at + * the edge of this primitive's triangles. + */ + int32_t indices = -1; +}; +} // namespace CesiumGltf diff --git a/CesiumGltfReader/generated/include/CesiumGltfReader/ExtensionCesiumPrimitiveOutlineReader.h b/CesiumGltfReader/generated/include/CesiumGltfReader/ExtensionCesiumPrimitiveOutlineReader.h new file mode 100644 index 000000000..9a38831d8 --- /dev/null +++ b/CesiumGltfReader/generated/include/CesiumGltfReader/ExtensionCesiumPrimitiveOutlineReader.h @@ -0,0 +1,76 @@ +// This file was generated by generate-classes. +// DO NOT EDIT THIS FILE! +#pragma once + +#include +#include +#include +#include + +#include +#include + +#include + +namespace CesiumGltf { +struct ExtensionCesiumPrimitiveOutline; +} + +namespace CesiumGltfReader { + +/** + * @brief Reads {@link ExtensionCesiumPrimitiveOutline} instances from JSON. + */ +class CESIUMGLTFREADER_API ExtensionCesiumPrimitiveOutlineReader { +public: + /** + * @brief Constructs a new instance. + */ + ExtensionCesiumPrimitiveOutlineReader(); + + /** + * @brief Gets the options controlling how the JSON is read. + */ + CesiumJsonReader::JsonReaderOptions& getOptions(); + + /** + * @brief Gets the options controlling how the JSON is read. + */ + const CesiumJsonReader::JsonReaderOptions& getOptions() const; + + /** + * @brief Reads an instance of ExtensionCesiumPrimitiveOutline from a byte + * buffer. + * + * @param data The buffer from which to read the instance. + * @return The result of reading the instance. + */ + CesiumJsonReader::ReadJsonResult + readFromJson(const gsl::span& data) const; + + /** + * @brief Reads an instance of ExtensionCesiumPrimitiveOutline from a + * rapidJson::Value. + * + * @param data The buffer from which to read the instance. + * @return The result of reading the instance. + */ + CesiumJsonReader::ReadJsonResult + readFromJson(const rapidjson::Value& value) const; + + /** + * @brief Reads an array of instances of ExtensionCesiumPrimitiveOutline from + * a rapidJson::Value. + * + * @param data The buffer from which to read the array of instances. + * @return The result of reading the array of instances. + */ + CesiumJsonReader::ReadJsonResult< + std::vector> + readArrayFromJson(const rapidjson::Value& value) const; + +private: + CesiumJsonReader::JsonReaderOptions _options; +}; + +} // namespace CesiumGltfReader \ No newline at end of file diff --git a/CesiumGltfReader/generated/src/ExtensionCesiumPrimitiveOutlineJsonHandler.h b/CesiumGltfReader/generated/src/ExtensionCesiumPrimitiveOutlineJsonHandler.h new file mode 100644 index 000000000..a604f1a9e --- /dev/null +++ b/CesiumGltfReader/generated/src/ExtensionCesiumPrimitiveOutlineJsonHandler.h @@ -0,0 +1,48 @@ +// This file was generated by generate-classes. +// DO NOT EDIT THIS FILE! +#pragma once + +#include +#include +#include + +namespace CesiumJsonReader { +class JsonReaderOptions; +} + +namespace CesiumGltfReader { +class ExtensionCesiumPrimitiveOutlineJsonHandler + : public CesiumJsonReader::ExtensibleObjectJsonHandler, + public CesiumJsonReader::IExtensionJsonHandler { +public: + using ValueType = CesiumGltf::ExtensionCesiumPrimitiveOutline; + + static inline constexpr const char* ExtensionName = + "CESIUM_primitive_outline"; + + ExtensionCesiumPrimitiveOutlineJsonHandler( + const CesiumJsonReader::JsonReaderOptions& options) noexcept; + void reset( + IJsonHandler* pParentHandler, + CesiumGltf::ExtensionCesiumPrimitiveOutline* pObject); + + virtual IJsonHandler* readObjectKey(const std::string_view& str) override; + + virtual void reset( + IJsonHandler* pParentHandler, + CesiumUtility::ExtensibleObject& o, + const std::string_view& extensionName) override; + + virtual IJsonHandler& getHandler() override { return *this; } + +protected: + IJsonHandler* readObjectKeyExtensionCesiumPrimitiveOutline( + const std::string& objectType, + const std::string_view& str, + CesiumGltf::ExtensionCesiumPrimitiveOutline& o); + +private: + CesiumGltf::ExtensionCesiumPrimitiveOutline* _pObject = nullptr; + CesiumJsonReader::IntegerJsonHandler _indices; +}; +} // namespace CesiumGltfReader diff --git a/CesiumGltfReader/generated/src/GeneratedJsonHandlers.cpp b/CesiumGltfReader/generated/src/GeneratedJsonHandlers.cpp index 014b1722f..096fa2275 100644 --- a/CesiumGltfReader/generated/src/GeneratedJsonHandlers.cpp +++ b/CesiumGltfReader/generated/src/GeneratedJsonHandlers.cpp @@ -1951,6 +1951,109 @@ ExtensionTextureWebpReader::readArrayFromJson( return CesiumJsonReader::JsonReader::readJson(value, handler); } +} // namespace CesiumGltfReader +// This file was generated by generate-classes. +// DO NOT EDIT THIS FILE! +#include "ExtensionCesiumPrimitiveOutlineJsonHandler.h" +#include "registerExtensions.h" + +#include +#include +#include +#include + +#include +#include + +namespace CesiumGltfReader { + +ExtensionCesiumPrimitiveOutlineJsonHandler:: + ExtensionCesiumPrimitiveOutlineJsonHandler( + const CesiumJsonReader::JsonReaderOptions& options) noexcept + : CesiumJsonReader::ExtensibleObjectJsonHandler(options), _indices() {} + +void ExtensionCesiumPrimitiveOutlineJsonHandler::reset( + CesiumJsonReader::IJsonHandler* pParentHandler, + CesiumGltf::ExtensionCesiumPrimitiveOutline* pObject) { + CesiumJsonReader::ExtensibleObjectJsonHandler::reset(pParentHandler, pObject); + this->_pObject = pObject; +} + +CesiumJsonReader::IJsonHandler* +ExtensionCesiumPrimitiveOutlineJsonHandler::readObjectKey( + const std::string_view& str) { + assert(this->_pObject); + return this->readObjectKeyExtensionCesiumPrimitiveOutline( + CesiumGltf::ExtensionCesiumPrimitiveOutline::TypeName, + str, + *this->_pObject); +} + +void ExtensionCesiumPrimitiveOutlineJsonHandler::reset( + CesiumJsonReader::IJsonHandler* pParentHandler, + CesiumUtility::ExtensibleObject& o, + const std::string_view& extensionName) { + std::any& value = + o.extensions + .emplace(extensionName, CesiumGltf::ExtensionCesiumPrimitiveOutline()) + .first->second; + this->reset( + pParentHandler, + &std::any_cast(value)); +} + +CesiumJsonReader::IJsonHandler* ExtensionCesiumPrimitiveOutlineJsonHandler:: + readObjectKeyExtensionCesiumPrimitiveOutline( + const std::string& objectType, + const std::string_view& str, + CesiumGltf::ExtensionCesiumPrimitiveOutline& o) { + using namespace std::string_literals; + + if ("indices"s == str) + return property("indices", this->_indices, o.indices); + + return this->readObjectKeyExtensibleObject(objectType, str, *this->_pObject); +} + +ExtensionCesiumPrimitiveOutlineReader::ExtensionCesiumPrimitiveOutlineReader() { + registerExtensions(this->_options); +} + +CesiumJsonReader::JsonReaderOptions& +ExtensionCesiumPrimitiveOutlineReader::getOptions() { + return this->_options; +} + +const CesiumJsonReader::JsonReaderOptions& +ExtensionCesiumPrimitiveOutlineReader::getOptions() const { + return this->_options; +} + +CesiumJsonReader::ReadJsonResult +ExtensionCesiumPrimitiveOutlineReader::readFromJson( + const gsl::span& data) const { + ExtensionCesiumPrimitiveOutlineJsonHandler handler(this->_options); + return CesiumJsonReader::JsonReader::readJson(data, handler); +} + +CesiumJsonReader::ReadJsonResult +ExtensionCesiumPrimitiveOutlineReader::readFromJson( + const rapidjson::Value& value) const { + ExtensionCesiumPrimitiveOutlineJsonHandler handler(this->_options); + return CesiumJsonReader::JsonReader::readJson(value, handler); +} + +CesiumJsonReader::ReadJsonResult< + std::vector> +ExtensionCesiumPrimitiveOutlineReader::readArrayFromJson( + const rapidjson::Value& value) const { + CesiumJsonReader::ArrayJsonHandler< + CesiumGltf::ExtensionCesiumPrimitiveOutline, + ExtensionCesiumPrimitiveOutlineJsonHandler> + handler(this->_options); + return CesiumJsonReader::JsonReader::readJson(value, handler); +} + } // namespace CesiumGltfReader // This file was generated by generate-classes. // DO NOT EDIT THIS FILE! diff --git a/CesiumGltfReader/generated/src/registerExtensions.cpp b/CesiumGltfReader/generated/src/registerExtensions.cpp index ed3d42c4a..f2706cd24 100644 --- a/CesiumGltfReader/generated/src/registerExtensions.cpp +++ b/CesiumGltfReader/generated/src/registerExtensions.cpp @@ -5,6 +5,7 @@ #include "ExtensionBufferExtMeshoptCompressionJsonHandler.h" #include "ExtensionBufferViewExtMeshoptCompressionJsonHandler.h" +#include "ExtensionCesiumPrimitiveOutlineJsonHandler.h" #include "ExtensionCesiumRTCJsonHandler.h" #include "ExtensionCesiumTileEdgesJsonHandler.h" #include "ExtensionExtInstanceFeaturesJsonHandler.h" @@ -65,6 +66,9 @@ void registerExtensions(CesiumJsonReader::JsonReaderOptions& options) { options.registerExtension< CesiumGltf::MeshPrimitive, ExtensionMeshPrimitiveKhrMaterialsVariantsJsonHandler>(); + options.registerExtension< + CesiumGltf::MeshPrimitive, + ExtensionCesiumPrimitiveOutlineJsonHandler>(); options.registerExtension< CesiumGltf::Node, ExtensionExtInstanceFeaturesJsonHandler>(); diff --git a/CesiumGltfWriter/generated/src/ModelJsonWriter.cpp b/CesiumGltfWriter/generated/src/ModelJsonWriter.cpp index 5533bece4..ea15713e3 100644 --- a/CesiumGltfWriter/generated/src/ModelJsonWriter.cpp +++ b/CesiumGltfWriter/generated/src/ModelJsonWriter.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -167,6 +168,11 @@ void writeJson( CesiumJsonWriter::JsonWriter& jsonWriter, const CesiumJsonWriter::ExtensionWriterContext& context); +void writeJson( + const CesiumGltf::ExtensionCesiumPrimitiveOutline& obj, + CesiumJsonWriter::JsonWriter& jsonWriter, + const CesiumJsonWriter::ExtensionWriterContext& context); + void writeJson( const CesiumGltf::ExtensionNodeMaxarMeshVariantsMappingsValue& obj, CesiumJsonWriter::JsonWriter& jsonWriter, @@ -920,6 +926,22 @@ void writeJson( jsonWriter.EndObject(); } +void writeJson( + const CesiumGltf::ExtensionCesiumPrimitiveOutline& obj, + CesiumJsonWriter::JsonWriter& jsonWriter, + const CesiumJsonWriter::ExtensionWriterContext& context) { + jsonWriter.StartObject(); + + if (obj.indices > -1) { + jsonWriter.Key("indices"); + writeJson(obj.indices, jsonWriter, context); + } + + writeExtensibleObject(obj, jsonWriter, context); + + jsonWriter.EndObject(); +} + void writeJson( const CesiumGltf::ExtensionNodeMaxarMeshVariantsMappingsValue& obj, CesiumJsonWriter::JsonWriter& jsonWriter, @@ -2451,6 +2473,13 @@ void ExtensionTextureWebpJsonWriter::write( writeJson(obj, jsonWriter, context); } +void ExtensionCesiumPrimitiveOutlineJsonWriter::write( + const CesiumGltf::ExtensionCesiumPrimitiveOutline& obj, + CesiumJsonWriter::JsonWriter& jsonWriter, + const CesiumJsonWriter::ExtensionWriterContext& context) { + writeJson(obj, jsonWriter, context); +} + void ExtensionNodeMaxarMeshVariantsMappingsValueJsonWriter::write( const CesiumGltf::ExtensionNodeMaxarMeshVariantsMappingsValue& obj, CesiumJsonWriter::JsonWriter& jsonWriter, diff --git a/CesiumGltfWriter/generated/src/ModelJsonWriter.h b/CesiumGltfWriter/generated/src/ModelJsonWriter.h index 870caf02e..fd4c1ded3 100644 --- a/CesiumGltfWriter/generated/src/ModelJsonWriter.h +++ b/CesiumGltfWriter/generated/src/ModelJsonWriter.h @@ -28,6 +28,7 @@ struct ExtensionModelMaxarMeshVariants; struct ExtensionNodeMaxarMeshVariants; struct ExtensionKhrTextureTransform; struct ExtensionTextureWebp; +struct ExtensionCesiumPrimitiveOutline; struct ExtensionNodeMaxarMeshVariantsMappingsValue; struct ExtensionModelMaxarMeshVariantsValue; struct ExtensionMeshPrimitiveKhrMaterialsVariantsMappingsValue; @@ -277,6 +278,18 @@ struct ExtensionTextureWebpJsonWriter { const CesiumJsonWriter::ExtensionWriterContext& context); }; +struct ExtensionCesiumPrimitiveOutlineJsonWriter { + using ValueType = CesiumGltf::ExtensionCesiumPrimitiveOutline; + + static inline constexpr const char* ExtensionName = + "CESIUM_primitive_outline"; + + static void write( + const CesiumGltf::ExtensionCesiumPrimitiveOutline& obj, + CesiumJsonWriter::JsonWriter& jsonWriter, + const CesiumJsonWriter::ExtensionWriterContext& context); +}; + struct ExtensionNodeMaxarMeshVariantsMappingsValueJsonWriter { using ValueType = CesiumGltf::ExtensionNodeMaxarMeshVariantsMappingsValue; diff --git a/CesiumGltfWriter/generated/src/registerExtensions.cpp b/CesiumGltfWriter/generated/src/registerExtensions.cpp index ec1552dfb..59d6564a8 100644 --- a/CesiumGltfWriter/generated/src/registerExtensions.cpp +++ b/CesiumGltfWriter/generated/src/registerExtensions.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -66,6 +67,9 @@ void registerExtensions(CesiumJsonWriter::ExtensionWriterContext& context) { context.registerExtension< CesiumGltf::MeshPrimitive, ExtensionMeshPrimitiveKhrMaterialsVariantsJsonWriter>(); + context.registerExtension< + CesiumGltf::MeshPrimitive, + ExtensionCesiumPrimitiveOutlineJsonWriter>(); context.registerExtension< CesiumGltf::Node, ExtensionExtInstanceFeaturesJsonWriter>(); diff --git a/tools/generate-classes/glTF.json b/tools/generate-classes/glTF.json index 68bb6e80b..9081c1be2 100644 --- a/tools/generate-classes/glTF.json +++ b/tools/generate-classes/glTF.json @@ -128,6 +128,9 @@ }, "KHR_texture_transform glTF TextureInfo Extension": { "overrideName": "ExtensionKhrTextureTransform" + }, + "CESIUM_primitive_outline glTF primitive extension": { + "overrideName": "ExtensionCesiumPrimitiveOutline" } }, "extensions": [ @@ -230,6 +233,13 @@ "attachTo": [ "texture" ] + }, + { + "extensionName": "CESIUM_primitive_outline", + "schema": "Vendor/CESIUM_primitive_outline/schema/primitive.CESIUM_primitive_outline.schema.json", + "attachTo": [ + "mesh.primitive" + ] } ] }