Skip to content

nv2a/vk: Update vertex ram buffer to finish only when drawing. #2345

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

Closed
wants to merge 2 commits into from
Closed

nv2a/vk: Update vertex ram buffer to finish only when drawing. #2345

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3a023f
Update vertex ram buffer to handle bitmap calls when not drawing.
MattPonton Jul 19, 2025
45d8aa9
Clean up logic and comments.
MattPonton Jul 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions hw/xbox/nv2a/pgraph/vk/vertex.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ void pgraph_vk_update_vertex_ram_buffer(PGRAPHState *pg, hwaddr offset,
size_t end_bit = TARGET_PAGE_ALIGN(offset + size) / TARGET_PAGE_SIZE;
size_t nbits = end_bit - start_bit;

if (find_next_bit(r->uploaded_bitmap, start_bit + nbits, start_bit) <
end_bit) {
if (find_next_bit(r->uploaded_bitmap, start_bit + nbits, start_bit) < end_bit && r->in_draw) {
Copy link
Member

@mborgerson mborgerson Sep 3, 2025

Choose a reason for hiding this comment

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

When this function is called, r->in_draw should be false, which means this path will never be triggered

Copy link
Author

@MattPonton MattPonton Sep 4, 2025

Choose a reason for hiding this comment

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

Is there a case then where we even would need this if check and its proceeding functionality? I ask since I haven't found a situation where this path never being triggered has caused greater issues than when it has been triggered. But I also haven't tested the entire xbox library with this change yet. I admit it's possible that my change here could just be exposing a symptom to a deeper problem.

Copy link
Member

@mborgerson mborgerson Sep 4, 2025

Choose a reason for hiding this comment

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

where we even would need this if check and its proceeding functionality

Yes, as the comment below indicates, if vertex data are changed while the draw command buffer is being built (but before it is executed), then we need to finish any pending drawing. There are ways to make this faster, but this simple logic here prioritizes correctness. There's also a chance there's some pathological case, or we are being too conservative--since you've found an instance where this path is triggered often you can find what data are changing (via the bitmap).

// Vertex data changed while building the draw list. Finish drawing
// before updating RAM buffer.
pgraph_vk_finish(pg, VK_FINISH_REASON_VERTEX_BUFFER_DIRTY);
Expand Down