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.

Sign up for GitHub

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

Jump to bottom

Drop repeated frame IDs #244

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Drop repeated frame IDs #244

wants to merge 3 commits into from
+71 −19

Conversation

jp7677
Copy link
Owner

@jp7677 jp7677 commented Jan 20, 2025 •
edited
Loading

Not tested ...

Based on HansKristian-Work/vkd3d-proton#2294

@jp7677 jp7677 force-pushed the repeated-frame-ids branch 2 times, most recently from a112a9b to fd50d85 Compare January 20, 2025 19:33
Saancreed
Saancreed reviewed Jan 20, 2025
View reviewed changes
Copy link
Collaborator

@Saancreed Saancreed left a comment

Choose a reason for hiding this comment

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

I'd move this to LowLatencyFrameIdGenerator (so something like bool LowLatencyFrameIdGenerator::CheckRepeatedMarker(frameID, markerType)) but the general idea appears to be sound.

src/nvapi/nvapi_d3d_low_latency_device.cpp Outdated Show resolved Hide resolved
src/nvapi/nvapi_d3d_low_latency_device.cpp Outdated Show resolved Hide resolved
src/nvapi/nvapi_d3d_low_latency_device.cpp Outdated Show resolved Hide resolved
src/nvapi/nvapi_d3d_low_latency_device.h Outdated Show resolved Hide resolved
@jp7677 jp7677 force-pushed the repeated-frame-ids branch from fd50d85 to 652e56a Compare January 21, 2025 06:57
@jp7677 jp7677 marked this pull request as ready for review January 21, 2025 06:58
@jp7677 jp7677 force-pushed the repeated-frame-ids branch from 652e56a to b873cba Compare January 21, 2025 07:04
@jp7677 jp7677 mentioned this pull request Jan 21, 2025
Open
Saancreed
Saancreed approved these changes Jan 21, 2025
View reviewed changes
Copy link
Collaborator

@Saancreed Saancreed left a comment

Choose a reason for hiding this comment

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

Even though it doesn't fix the issue, it's still a bug on our side so it's probably a good idea do this anyway…

(I'm not happy with the map but ultimately it's not important.)

Maybe we should ask Eric to take a look?

src/nvapi/nvapi_d3d_low_latency_device.cpp Outdated Show resolved Hide resolved
@jp7677 jp7677 force-pushed the repeated-frame-ids branch 2 times, most recently from b715f96 to 0ef44d7 Compare January 21, 2025 15:10
Saancreed
Saancreed approved these changes Jan 21, 2025
View reviewed changes
Copy link
Collaborator

@Saancreed Saancreed left a comment

Choose a reason for hiding this comment

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

👌

jp7677 added 3 commits January 26, 2025 09:17
@jp7677
meta: Simplify ignores
dbf3e4e
@jp7677
nvapi-d3d: Formatting
7df0756
@jp7677
nvapi-d3d: Drop repeated frame IDs
53c17e9 
Frame IDs must be monotonic per marker type and
device/command queue.
@jp7677 jp7677 force-pushed the repeated-frame-ids branch from 0ef44d7 to 53c17e9 Compare January 26, 2025 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Saancreed Saancreed Saancreed 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
@jp7677 @Saancreed