Skip to content

Conversation

@asuessenbach
Copy link
Contributor

@asuessenbach asuessenbach commented Nov 5, 2025

Only the semaphores signalled on finished rendering need to be indexed by the image index. All other resources (command buffers, fences, semaphores signalled on present completion) are indexed by the frame index.

Includes some minor clean-up on chapter 15 as well.

As usual, if this is considered to be ok, I can carry this change to all the other chapters.

@asuessenbach asuessenbach changed the title Fix vector sizes for presentCompleteSemaphores in chapter 16. Fix vector size for presentCompleteSemaphores in chapter 16. Nov 5, 2025
@SaschaWillems
Copy link
Collaborator

As usual, if this is considered to be ok, I can carry this change to all the other chapters.

Definitively yes.

Without this change, the tutorial is not spec compliant, see #65

@asuessenbach asuessenbach force-pushed the 16_frames_in_flight branch 2 times, most recently from 0faa53f to 9a803ea Compare November 10, 2025 12:16
@asuessenbach asuessenbach changed the title Fix vector size for presentCompleteSemaphores in chapter 16. Fix vector size for presentCompleteSemaphores. Nov 10, 2025
Copy link
Collaborator

@SaschaWillems SaschaWillems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good, but tutorial text needs to be adjusted to. Did a quick compare of the frames in flight chapter, and it no longer matches the semaphore setup now used in code.

@asuessenbach
Copy link
Contributor Author

but tutorial text needs to be adjusted to

If you could give me a hint, where the corresponding tutorial text is, I could try to adjust it accordingly.

@SaschaWillems
Copy link
Collaborator

E.g. "03_Frames_in_flight.adoc". Looking at this, it does at least partially differ from the updated code and might need to be updated.

@asuessenbach
Copy link
Contributor Author

@SaschaWillems Ok, I've adjusted some documentation. Please have a closer look at it!

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.

2 participants