[webkit-reviews] review canceled: [Bug 94752] [chromium] Create sharedQuadState at same time as creating quads and give them to the quadSink : [Attachment 160027] Patch

[bugzilla-daemon at webkit.org]

Dana Jansens <danakj at chromium.org> has canceled Dana Jansens
<danakj at chromium.org>'s request for review:
Bug 94752: [chromium] Create sharedQuadState at same time as creating quads and
give them to the quadSink
https://bugs.webkit.org/show_bug.cgi?id=94752

Attachment 160027: Patch
https://bugs.webkit.org/attachment.cgi?id=160027&action=review

------- Additional Comments from Dana Jansens <danakj at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=160027&action=review


>> Source/WebCore/platform/graphics/chromium/cc/CCQuadCuller.cpp:101
>> +}
> 
> A potential foot gun is that you might think you could call
generateSharedQuadStateId() n times and then add n states, which is wrong.  
> 
> As a potential suggestion, what about always having to appendSharedQuadState
first prior to appending quads.  That function sets the id internally on the
shared quad state and returns a raw pointer to the modified shared quad state
(for convenience, since you released the other pointer).  You would use this
pointer to pass to quads that you append (since you set the shared quad state
id on quads and need it there).  Maybe CCDrawQuad could assert if you gave it a
shared quad state that hadn't been appended yet.  At least that'd be a runtime
check that you'd done the right thing.

Brilliant. I was not satisfied with this either but was not sure what to do
with it, this is perfect. Thanks.