[webkit-reviews] review granted: [Bug 87167] [chromium] Cleanup scissor rect computation/use with damage : [Attachment 145444] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Jun 2 12:52:06 PDT 2012
Adrienne Walker <enne at google.com> has granted zlieber at chromium.org's request
for review:
Bug 87167: [chromium] Cleanup scissor rect computation/use with damage
https://bugs.webkit.org/show_bug.cgi?id=87167
Attachment 145444: Patch
https://bugs.webkit.org/attachment.cgi?id=145444&action=review
------- Additional Comments from Adrienne Walker <enne at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=145444&action=review
Thanks for the changes. R=me, with two small changes:
> Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:1275
> + // One scissor for clearing surface, another for drawing the quads
> + harness.mustSetScissor(0, 8, 2, 2);
> + harness.mustSetScissor(0, 8, 2, 2);
Hmm. I still think mustSetScissor should allow any number of redundant scissor
settings. If we start shadowing state locally in LRC and not repeatedly
setting scissors, then this test would needlessly fail because of that
implementation detail.
> Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:1421
> +TEST_F(CCLayerTreeHostImplTest, quadsToFillScreen)
I still don't think this really adds anything over the other test (or occlusion
tests that have multiple surfaces) and should probably just get removed. If
you're going to add a gutter quad test related to partial swap, then it should
probably be something where the scissor is smaller than the viewport and then
you verify that the quads fill the scissor.
More information about the webkit-reviews
mailing list