bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Fix Gizmo joint rendering in webgpu

Open akimakinai opened this issue 1 year ago • 7 comments

Objective

  • Gizmo rendering on WebGPU has been fixed by #14653, but gizmo joints still cause error (https://github.com/bevyengine/bevy/issues/14696#issuecomment-2283689669) when enabled.

Solution

  • Applies the same fix as #14653 to Gizmo joints.

I'm noob and just copied their solution, please correct me if I did something wrong.

Testing

  • Tested 2d-gizmos and 3d-gizmos examples in WebGPU on Chrome. No rendering errors, and the gizmo joints are apparently rendered ok.

akimakinai avatar Aug 12 '24 11:08 akimakinai

@alice-i-cecile looks like @irate-devil has no GitHub activity over the last month and change... Maybe choose a different reviewer?

simbleau avatar Aug 18 '24 16:08 simbleau

Yep, I'll ask another couple of folks :)

alice-i-cecile avatar Aug 18 '24 21:08 alice-i-cecile

Was also able to trigger in the 2d example:

wgpu error: Validation Error

Caused by:
    In a RenderPass
      note: encoder = `<CommandBuffer-(2692, 1, Metal)>`
    In a draw command, indexed:false indirect:false
      note: render pipeline = `LineGizmo Pipeline 2D`
    Instance 149 extends beyond limit 148 imposed by the buffer in slot 3. Did you bind the correct `Instance` step-rate vertex buffer?

tychedelia avatar Aug 18 '24 22:08 tychedelia

The examples work fine, still can't figure out how I produced those crashes. If no one else can repro I'll approve with the note that there might be some kind of racy bug we should keep an eye out for.

tychedelia avatar Aug 18 '24 22:08 tychedelia

~~I tested on browsers and Windows native, and I didn't encounter the validation error. Currently I am on mac and it seems to reproduce occasionally for me too. I see Metal also in your log, so this could be Metal-only issue.~~ No, this seems to reproduce on every plaform.

Caused by:
    In a RenderPass
      note: encoder = `<CommandBuffer-(19342, 1, Metal)>`
    In a draw command, indexed:false indirect:false
      note: render pipeline = `LineGizmo Pipeline`
    Instance 555 extends beyond limit 554 imposed by the buffer in slot 3. Did you bind the correct `Instance` step-rate vertex buffer?

I wonder why it's LineGizmo pipeline and not LineJointGizmo pipeline, but evidently triggered by enabling joints.

akimakinai avatar Aug 18 '24 23:08 akimakinai

The same underlying buffer is used as vertex buffer with the same offset but different sizes from two draw functions. TrackedRenderPass::set_vertex_buffer wrongly assumes vertex buffer need not be updated in this case. (#14841)

I've added system ordering so that lines (draw function for lines uses larger size) are always drawn first to workaround vertex buffer not updated. This should fix the occasional validation error. Can you confirm? We may want to fix TrackerRenderPass::set_vertex_buffer to properly update vertex buffer when silce size changed (https://discord.com/channels/691052431525675048/743663924229963868/1275593235535761409), but that should be separate PR, I think.

akimakinai avatar Aug 20 '24 23:08 akimakinai

I've added system ordering so that lines are always drawn first to workaround vertex buffer not updated. This should fix the occasional validation error. Can you confirm?

Thanks for tracking that down. I'm not able to reproduce here, but it sounds like we're more confident in the underlying issue, which can be a follow up. Approving.

tychedelia avatar Aug 21 '24 00:08 tychedelia