Skip to content
This repository has been archived by the owner on Oct 28, 2024. It is now read-only.

Add synchronization2_timeline_semaphore_simple #237

Merged
merged 2 commits into from
May 9, 2022
Merged

Add synchronization2_timeline_semaphore_simple #237

merged 2 commits into from
May 9, 2022

Conversation

bjoeris
Copy link
Contributor

@bjoeris bjoeris commented May 2, 2022

This is based on the timeline_semaphore_simple sample, but uses the synchronization2 api.

@bjoeris bjoeris force-pushed the pr-synchronization2_timeline_semaphore_simple branch 2 times, most recently from 289b971 to 27e3edb Compare May 2, 2022 19:48
@bjoeris bjoeris requested a review from RenfengLiu May 2, 2022 19:57
@RenfengLiu
Copy link
Contributor

The app seems crash when exiting.
image

RenfengLiu
RenfengLiu reviewed May 9, 2022
View reviewed changes
application_sandbox/synchronization2_timeline_semaphore_simple/cube.frag Show resolved Hide resolved
application_sandbox/synchronization2_timeline_semaphore_simple/cube.vert Show resolved Hide resolved
application_sandbox/synchronization2_timeline_semaphore_simple/main.cpp Outdated
vulkan::VulkanApplication app(
data->allocator(), data->logger(), data,
vulkan::VulkanApplicationOptions()
.SetHostBufferSize(131072)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use a constant and give some comments about the size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a constant. In terms of a comment, I'm not sure why a different buffer size is wanted here. This comes from the original timeline_semaphore_simple sample.

@bjoeris bjoeris force-pushed the pr-synchronization2_timeline_semaphore_simple branch from 27e3edb to 1649156 Compare May 9, 2022 17:36
@bjoeris
Copy link
Contributor Author

bjoeris commented May 9, 2022

The app seems crash when exiting.

I am seeing identical crashes on the original timeline_semaphore_simple sample. I think part of the issue is that the loop in main_entry loops forever, with no exit condition. I tried updating this to while (!app.should_exit() && !data->WindowClosing()), based on some of the other samples, but I'm still seeing some sort of crash that for some reason also crashes the debugger. I think we might want to look into this more, but that issue is independent of this PR.

@RenfengLiu
Copy link
Contributor

The app seems crash when exiting.

I am seeing identical crashes on the original timeline_semaphore_simple sample. I think part of the issue is that the loop in main_entry loops forever, with no exit condition. I tried updating this to while (!app.should_exit() && !data->WindowClosing()), based on some of the other samples, but I'm still seeing some sort of crash that for some reason also crashes the debugger. I think we might want to look into this more, but that issue is independent of this PR.

Then probably we can create an issue so that we will not forget this issue.

@bjoeris
Copy link
Contributor Author

bjoeris commented May 9, 2022

Then probably we can create an issue so that we will not forget this issue.
Done: #239

@bjoeris bjoeris merged commit bfa9418 into master May 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@RenfengLiu RenfengLiu RenfengLiu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bjoeris @RenfengLiu