Cocoa2d-HTLM5 game apepars to use wrong vertex buffer data
Created attachment 451486 [details] Patch
<rdar://problem/87136345>
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Looks good as an interim solution with an addition: Please add a // FIXME: explaining why consulting mElementArrayDirty is necessary when it should not be necessary. Since you have the repro: Could we add an assert at the end of ContextMtl::setupDraw that mRenderEncoder state matches the expected ProgramMtl state? So recursively in in ProgramMtl assertion that VertexArrayMtl state? And in VertexArrayMtl, assertion that mCurrentArrayBuffers, offsets etc are in RenderCommandEncoder state cache correctly? With the assumption that RenderCommandEncoder state is really diverging from expected state (your explanation in the commit message): I'd imagine this assertion to fail with the test case for the first draw call that overrides the expected state, e.g. to reveal the real bug? With the assumption that RenderCommandEncoder reflects the expected state correctly but that the dirty bits that sync the expected state are wrong: I'd imagine this assertion to fail with the test case for the first draw call that expects the expected state and does not set anything, but then the expected state was not updated because some omission with the dirty bit handling.
(In reply to Kimmo Kinnunen from comment #4) > Looks good as an interim solution with an addition: > Please add a // FIXME: explaining why consulting mElementArrayDirty is > necessary when it should not be necessary. > > Since you have the repro: > Could we add an assert at the end of ContextMtl::setupDraw that > mRenderEncoder state matches the expected ProgramMtl state? So recursively > in in ProgramMtl assertion that VertexArrayMtl state? And in VertexArrayMtl, > assertion that mCurrentArrayBuffers, offsets etc are in > RenderCommandEncoder state cache correctly? I tried just this, though 'assert' might have been incorrect. Unfortunately, this is an odd case where I suspect the vertex data and the program data match, but we're getting stale vertex data. I went through the vertex attribute handling flow 3 or 4 times yesterday, but for the life of me can't figure out where we've diverged. Only some elements are using the wrong vertex data, not all. > > With the assumption that RenderCommandEncoder state is really diverging from > expected state (your explanation in the commit message): > I'd imagine this assertion to fail with the test case for the first draw > call that overrides the expected state, e.g. to reveal the real bug? I'll certainly continue to investigate, once we confirm this patches up our repros. > With the assumption that RenderCommandEncoder reflects the expected state > correctly but that the dirty bits that sync the expected state are wrong: > I'd imagine this assertion to fail with the test case for the first draw > call that expects the expected state and does not set anything, but then the > expected state was not updated because some omission with the dirty bit > handling.
For completeness: in a conference call today Geoff Lang from the ANGLE team suggested that the bug might be that when the element array changes, any vertex attributes that previously required format conversion need to be re-converted. Perhaps this hint could help reduce the amount of work that needs to be done when the element array changes, though this can be revisited later if that's too complicated or fragile a change.
Created attachment 451719 [details] Patch for landing
Created attachment 451726 [details] Patch for landing
ChangeLog entry in Source/ThirdParty/ANGLE/ChangeLog contains OOPS!.
Created attachment 451732 [details] Patch for landing
Committed r289664 (247152@main): <https://commits.webkit.org/247152@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 451732 [details].