Skip to content

Conversation

@mborgerson
Copy link
Member

@mborgerson mborgerson commented Feb 9, 2026

Not all Vulkan implementations support geometry shaders (e.g. MoltenVK, KosmicKrisp). As a step towards enabling xemu on such implementations, move primitive rewriting out of geometry shaders and just do it client side when necessary.

Work in progress.

@gemini-code-assist
Copy link

Summary of Changes

Hello @mborgerson, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the NV2A graphics pipeline by shifting primitive rewriting from geometry shaders to the CPU. This change enhances compatibility with various Vulkan implementations, particularly those lacking geometry shader support, and streamlines the graphics rendering process by centralizing primitive decomposition. The previous GPU property detection mechanism has been removed as it is no longer relevant to the new client-side approach.

Highlights

  • Client-Side Primitive Rewriting: Moved the logic for rewriting graphics primitives (like triangle strips, fans, quads, and polygons) from GPU geometry shaders to the CPU client-side. This change allows for broader compatibility with Vulkan implementations that may not support geometry shaders, such as MoltenVK.
  • Removal of GPU Property Detection: Eliminated the gpuprops.c files and associated code responsible for dynamically determining GPU geometry shader winding properties. This information is no longer needed as primitive rewriting is now handled on the CPU.
  • Simplified Shader Generation: The GLSL geometry shader generation logic has been significantly simplified, as it no longer needs to handle complex primitive types or their specific provoking vertex rules. The shaders now primarily deal with basic points, lines, and triangles.
  • Integration into Rendering Paths: The new client-side primitive rewriting functions (pgraph_prim_rewrite_ranges, pgraph_prim_rewrite_indexed, pgraph_prim_rewrite_sequential) have been integrated into both the OpenGL and Vulkan rendering paths to process drawing commands before they are sent to the GPU.
Changelog
  • hw/xbox/nv2a/pgraph/gl/draw.c
    • Included 'prim_rewrite.h' for new primitive rewriting functions.
    • Added logic to retrieve ShaderPolygonMode and PROVOKING_VERTEX settings.
    • Integrated pgraph_prim_rewrite_ranges and pgraph_prim_rewrite_indexed to rewrite draw arrays and inline elements into basic triangles/lines when necessary, using glDrawElements instead of glMultiDrawArrays.
  • hw/xbox/nv2a/pgraph/gl/gpuprops.c
    • Removed file, eliminating GPU property detection logic.
  • hw/xbox/nv2a/pgraph/gl/meson.build
    • Removed 'gpuprops.c' from the build configuration.
  • hw/xbox/nv2a/pgraph/gl/renderer.c
    • Removed calls to pgraph_gl_determine_gpu_properties during context initialization.
    • Removed the get_gpu_properties function pointer from the GL renderer operations.
  • hw/xbox/nv2a/pgraph/gl/renderer.h
    • Included 'prim_rewrite.h'.
    • Added gl_prim_rewrite_buffer and prim_rewrite_buf members to PGRAPHGLState.
    • Removed declarations for pgraph_gl_determine_gpu_properties and pgraph_gl_get_gpu_properties.
  • hw/xbox/nv2a/pgraph/gl/shaders.c
    • Simplified get_gl_primitive_mode to only return GL_POINTS, GL_LINES, and GL_TRIANGLES, as complex primitive types are now rewritten client-side.
  • hw/xbox/nv2a/pgraph/gl/vertex.c
    • Generated and initialized gl_prim_rewrite_buffer and prim_rewrite_buf.
    • Finalized gl_prim_rewrite_buffer and prim_rewrite_buf during buffer cleanup.
  • hw/xbox/nv2a/pgraph/glsl/geom.c
    • Included 'prim_rewrite.h'.
    • Updated state->primitive_mode using pgraph_prim_rewrite_get_output_mode.
    • Removed first_vertex_is_provoking from GeomState.
    • Simplified geometry shader generation logic, removing specific handling for TRIANGLE_STRIP, TRIANGLE_FAN, QUADS, QUAD_STRIP, and POLYGON primitive types.
    • Removed get_vertex_order helper function and related vertex reordering logic.
  • hw/xbox/nv2a/pgraph/glsl/geom.h
    • Removed first_vertex_is_provoking, tri_rot0, and tri_rot1 fields from the GeomState struct.
  • hw/xbox/nv2a/pgraph/meson.build
    • Added 'prim_rewrite.c' to the build configuration.
  • hw/xbox/nv2a/pgraph/pgraph.h
    • Removed the GPUProperties struct definition.
    • Removed the get_gpu_properties function pointer from PGRAPHRendererOps.
  • hw/xbox/nv2a/pgraph/prim_rewrite.c
    • Added new file implementing pgraph_prim_rewrite_init, pgraph_prim_rewrite_finalize, pgraph_prim_needs_rewrite, pgraph_prim_rewrite_get_output_mode, and various rewrite_* functions for different primitive types (lines, strips, fans, quads, polygons).
    • Implemented pgraph_prim_rewrite_ranges and pgraph_prim_rewrite_indexed to handle primitive rewriting and provoking vertex placement.
  • hw/xbox/nv2a/pgraph/prim_rewrite.h
    • Added new header file defining PrimRewriteBuf and PrimRewrite structs.
    • Declared functions for primitive rewriting: pgraph_prim_rewrite_init, pgraph_prim_rewrite_finalize, pgraph_prim_needs_rewrite, pgraph_prim_rewrite_get_output_mode, pgraph_prim_rewrite_ranges, pgraph_prim_rewrite_indexed, and pgraph_prim_rewrite_sequential.
  • hw/xbox/nv2a/pgraph/vk/buffer.c
    • Initialized prim_rewrite_buf during Vulkan buffer initialization.
    • Finalized prim_rewrite_buf during Vulkan buffer finalization.
  • hw/xbox/nv2a/pgraph/vk/draw.c
    • Included 'prim_rewrite.h'.
    • Simplified get_primitive_topology to return only VK_PRIMITIVE_TOPOLOGY_POINT_LIST, VK_PRIMITIVE_TOPOLOGY_LINE_LIST, and VK_PRIMITIVE_TOPOLOGY_TRIANGLE_LIST.
    • Added logic to retrieve ShaderPolygonMode and PROVOKING_VERTEX settings.
    • Integrated pgraph_prim_rewrite_ranges, pgraph_prim_rewrite_indexed, and pgraph_prim_rewrite_sequential to rewrite draw arrays, inline elements, and inline buffers into basic primitives, using vkCmdDrawIndexed when rewriting is applied.
  • hw/xbox/nv2a/pgraph/vk/gpuprops.c
    • Removed file, eliminating GPU property detection logic.
  • hw/xbox/nv2a/pgraph/vk/meson.build
    • Removed 'gpuprops.c' from the build configuration.
  • hw/xbox/nv2a/pgraph/vk/renderer.c
    • Removed calls to pgraph_vk_determine_gpu_properties during renderer initialization.
    • Removed the get_gpu_properties function pointer from the Vulkan renderer operations.
  • hw/xbox/nv2a/pgraph/vk/renderer.h
    • Included 'prim_rewrite.h'.
    • Added prim_rewrite_buf member to PGRAPHVkState.
    • Removed declarations for pgraph_vk_determine_gpu_properties and pgraph_vk_get_gpu_properties.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a significant and valuable change that refactors primitive rewriting from geometry shaders to the CPU. This improves compatibility with Vulkan implementations that do not support geometry shaders, such as MoltenVK. The implementation is well-structured, with the new logic encapsulated in prim_rewrite.c.

I've found a few issues related to provoking vertex handling for complex primitives like quads and polygons, which could lead to incorrect flat shading. I've added specific comments with suggestions to address these.

Overall, this is a great step forward for xemu's renderer compatibility.

return false;
}
return true;
assert(false);
Copy link
Member

Choose a reason for hiding this comment

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

Just glancing through; a nit in case this bit comes out of draft: assert(false) issues often get reported and go for a long time without being investigated, making it non-trivial to work out what exactly failed. Can we make it a pattern to do assert(!"Some description of why this should never happen") instead? Then it's durable at time of reporting and reduces the need to walk back through blame to try to puzzle out why the assert existed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed--In new code this is being done, but there is still plenty of this pattern scattered throughout existing code. This line is removed anyway.

@mborgerson mborgerson force-pushed the feat/client-side-primitive-rewriting branch 5 times, most recently from 7f84a8f to eafca05 Compare February 10, 2026 06:17
@mborgerson mborgerson force-pushed the feat/client-side-primitive-rewriting branch from eafca05 to 152c324 Compare February 10, 2026 08:25
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