Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timeline semaphores #315

Merged
merged 11 commits into from
Feb 21, 2024
Merged

Timeline semaphores #315

merged 11 commits into from
Feb 21, 2024

Conversation

chaoticbob
Copy link
Contributor

This CL adds timeline semaphore support for both APIs. grfx::Semaphore is modified to support both binary and timeline semaphores similar to how Vulkan does it. This CL also makes the Vulkan queues thread safe.

  • Modified vk::Queue to be thread safe.
  • Added timeline semaphore support for Vulkan and D3D12.
  • Added sample demonstrating timeline semaphore.

include/ppx/grfx/grfx_sync.h Outdated Show resolved Hide resolved
include/ppx/grfx/vk/vk_sync.h Outdated Show resolved Hide resolved
include/ppx/grfx/vk/vk_sync.h Outdated Show resolved Hide resolved
projects/timeline_semaphore/CMakeLists.txt Outdated Show resolved Hide resolved
projects/timeline_semaphore/main.cpp Outdated Show resolved Hide resolved
projects/timeline_semaphore/main.cpp Outdated Show resolved Hide resolved
src/ppx/grfx/grfx_sync.cpp Show resolved Hide resolved
src/ppx/grfx/vk/vk_queue.cpp Show resolved Hide resolved
include/ppx/grfx/grfx_sync.h Show resolved Hide resolved
projects/timeline_semaphore/main.cpp Outdated Show resolved Hide resolved
@chaoticbob chaoticbob requested a review from apazylbe October 17, 2023 05:44
src/ppx/grfx/grfx_sync.cpp Outdated Show resolved Hide resolved
This CL adds timeline semaphore support for both APIs. grfx::Semaphore is modified to support both binary and timeline semaphores similar to how Vulkan does it. This CL also makes the Vulkan queues thread safe.

- Modified vk::Queue to be thread safe.
- Added timeline semaphore support for Vulkan and D3D12.
- Added sample demonstrating timeline semaphore.
- Fixed typos and errors in comments
- Fixed copyright year
- Moved timeline asserts from API level to grfx level
@apazylbe
Copy link
Collaborator

Seems like there is only one thing outstanding, any objections to merging this and addressing in follow up PRs if needed?

This CL adds timeline semaphore support for both APIs. grfx::Semaphore is modified to support both binary and timeline semaphores similar to how Vulkan does it. This CL also makes the Vulkan queues thread safe.

- Modified vk::Queue to be thread safe.
- Added timeline semaphore support for Vulkan and D3D12.
- Added sample demonstrating timeline semaphore.
- Fixed typos and errors in comments
- Fixed copyright year
- Moved timeline asserts from API level to grfx level
- Removed the CPU to CPU sync test cases since there's not really any
  practical use case for this.
- Removed forceMonotonic flag from grfx::Semaphore::Signal since it
  obfuscates the intentions of the underlying APIs. This unfortunately
  subjects an app to UB, but doing otherwise might lead the app to
  believing it's handling its semaphores correctly when it's not.
@chaoticbob
Copy link
Contributor Author

@dnovillo @apazylbe The recent changes are in 06f6f9c. I rebased and it pulled in the history for everything else.

@chaoticbob chaoticbob merged commit e2951c5 into main Feb 21, 2024
7 checks passed
@chaoticbob chaoticbob deleted the timeline_semaphore branch February 21, 2024 17:53
GrantComm pushed a commit to GrantComm/bigwheels that referenced this pull request Mar 20, 2024
This CL adds timeline semaphore support for both APIs. grfx::Semaphore
is modified to support both binary and timeline semaphores similar to
how Vulkan does it. This CL also makes the Vulkan queues thread safe.

- Modified vk::Queue to be thread safe.
- Added timeline semaphore support for Vulkan and D3D12.
- Added sample demonstrating timeline semaphore.

---------

Co-authored-by: Aliya Pazylbekova <39095209+apazylbe@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants