[chromium] Make all compositor screen space transforms operate on content rects
Created attachment 153388 [details] Patch
Comment on attachment 153388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=153388&action=review This LGTM! > Source/WebCore/platform/graphics/chromium/cc/CCDebugRectHistory.cpp:77 > + updateContentRect.scale(layer->contentBounds().width() / static_cast<float>(layer->bounds().width()), layer->contentBounds().height() / static_cast<float>(layer->bounds().height())); want updateRect in content space again.. > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:737 > - WebTransformationMatrix screenSpaceTransform = layer->screenSpaceTransform(); > - // FIXME: These should be consistent. > - // The layer's screen space transform operates on layer rects, but the surfaces > - // screen space transform operates on surface rects, which are in physical pixels, > - // so we have to 'undo' the scale here. > - screenSpaceTransform.scale(1 / contentsScale); > - renderSurface->setScreenSpaceTransform(screenSpaceTransform); > + renderSurface->setScreenSpaceTransform(layer->screenSpaceTransform()); yayyy. this makes me super happy.
Created attachment 153656 [details] Rebased onto patch sequence
Comment on attachment 153656 [details] Rebased onto patch sequence View in context: https://bugs.webkit.org/attachment.cgi?id=153656&action=review > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:588 > + nextHierarchyMatrix.scale(1 / contentsScale); Other than test changes, this is the difference between this patch and the previous. Without this, the duplicateChildNonOwner case in CCLayerTreeHostCommonTest.verifyRenderSurfaceTransformsInHighDPI gets a double-scaled screen space matrix. Since originTransform (i.e. layer->drawTransform) includes a content scale, but nextHierarchyMatrix already included a content scale, then this is a double-scale and we need to undo one of them. I think this just happened to cancel out previously because screen space transforms were dealing with layer rects.
Created attachment 153683 [details] Rebased
I sorted it out. The 1 / contentsScale line needed to be there because of the change to drawTransform and is now a part of the patch in bug 91417. Everything now is explained. This patch is now equivalent to patch 1, except for some extra test changes.
Comment on attachment 153683 [details] Rebased View in context: https://bugs.webkit.org/attachment.cgi?id=153683&action=review ok still lgtm (with nit similar to that other cl) > Source/WebKit/chromium/tests/CCLayerTreeHostCommonTest.cpp:3688 > + EXPECT_TRANSFORMATION_MATRIX_EQ(parent->screenSpaceTransform(), expectedParentTransform); nit: expected result should come first.
Created attachment 154392 [details] Rebased
@kbr or @senorblanco for review
Comment on attachment 154392 [details] Rebased rs=me based on danakj's review.
Comment on attachment 154392 [details] Rebased Rejecting attachment 154392 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: CLayerTreeHostCommonTest.cpp Hunk #1 succeeded at 2806 (offset -100 lines). Hunk #2 succeeded at 3303 (offset -110 lines). Hunk #3 succeeded at 3319 (offset -110 lines). Hunk #4 succeeded at 3338 (offset -110 lines). Hunk #5 succeeded at 3385 (offset -117 lines). patching file Source/WebKit/chromium/tests/TiledLayerChromiumTest.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Kenneth Ru..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/13342420
Created attachment 154485 [details] Patch for landing
Comment on attachment 154485 [details] Patch for landing Rejecting attachment 154485 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/13342445
Committed r123685: <http://trac.webkit.org/changeset/123685>