RESOLVED WONTFIX 84187
[Chromium] Content of composited layer is blurry when page is scaled 
https://bugs.webkit.org/show_bug.cgi?id=84187
Summary [Chromium] Content of composited layer is blurry when page is scaled
Xianzhu Wang
Reported 2012-04-17 13:20:50 PDT
Created attachment 137589 [details] test case (blurry text, traces of underlying layer exposed) (can only run as a layout test) Content of composited layers is blurry when page is scaled because 1) the layer is composited on the surface at subpixel position 2) the transformations are computed inaccurately
Attachments
test case (blurry text, traces of underlying layer exposed) (can only run as a layout test) (2.25 KB, text/html)
2012-04-17 13:20 PDT, Xianzhu Wang 		no flags
Details
Another test showing blurry text in chromium-android (959 bytes, text/html)
2012-04-17 15:07 PDT, Xianzhu Wang 		no flags
Details
Screen shot and experiments (89.78 KB, image/png)
2012-04-17 15:12 PDT, Xianzhu Wang 		no flags
Details
Patch for preview (15.01 KB, patch)
2012-04-18 10:23 PDT, Xianzhu Wang 		webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ec2-cr-linux-02 (6.31 MB, application/zip)
2012-04-18 11:48 PDT, WebKit Review Bot 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Xianzhu Wang
Comment 1 2012-04-17 15:07:49 PDT
Created attachment 137614 [details] Another test showing blurry text in chromium-android
Xianzhu Wang
Comment 2 2012-04-17 15:12:17 PDT
Created attachment 137617 [details] Screen shot and experiments There are 5 boxes in this screen shot: 1. Original box in the root layer 2. Original box in a composited layer, showing blurry text 3-5. Boxes drawn on the composited layer with experimental code added in RenderLayerCompositor.cpp 3. accurate scale; pixel not aligned 4. inaccurate scale; pixel aligned 5. accurate scale and pixels aligned
Xianzhu Wang
Comment 3 2012-04-17 15:59:16 PDT
An example showing how the blur occurs and how proposed solution works: <html> <div style="-webkit-translate; -webkit-transform: translateZ(0); position: fixed; top: 2px; left: 2px;> <div style="margin:1px; background-color: black; width: 1px; height: 1px </div> </div> </html> In an un-scaled page, there will be a 1-pixel black dot at (2,2). Assume the current page scale is 1.4. The execution steps of the current code are (first omitting the inaccurate scales): 1) the black dot is drawn at (1.4,1.4)~(2.8,2.8), so actually it is drawn on 4 pixels starting from (1,1) with blended colors. 2) then the layer is composited at (2.8,2.8) of the final surface, and the black dot will be drawn onto 9 pixels starting from (3,3). That is, the dot is blurred onto 9 pixels. My proposal is to align the pixels of the layer surface and the its target surface by applying a pixel-alignment translation on the drawTransform of the layer and translate back with quadTransform when the layer is drawn onto its target surface. In the example, the pixel-alignment translation will be (0.8,0.8). The execution steps will be: 1) the black dot is drawn at (2.2,2.2)-(3.6,3.6), 4 pixels starting from (2,2). 2) then the layer is composited at (2,2) of the final surface. Because of the pixel-alignment, there is no loss at this step. Because of the pixel-alignment translation, the contents may overflow the surface of the layer. The borderTexel can be used to contain the overflowed pixel.
Dana Jansens
Comment 4 2012-04-17 19:36:14 PDT
Similar in spirit to http://trac.webkit.org/changeset/114471
Xianzhu Wang
Comment 5 2012-04-18 10:23:27 PDT
Created attachment 137718 [details] Patch for preview
WebKit Review Bot
Comment 6 2012-04-18 11:48:41 PDT
Comment on attachment 137718 [details] Patch for preview Attachment 137718 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12423939 New failing tests: CCLayerTreeHostImplTest.scrollbarLayerLostContext TiledLayerChromiumTest.verifyUpdateRectWhenContentBoundsAreScaled compositing/reflections/load-video-in-reflection.html CCQuadCullerTest.verifyCullChildLinesUpBottomRight compositing/images/content-image-change.html compositing/culling/tile-occlusion-boundaries.html CCQuadCullerTest.verifyCullCenterTileNonIntegralSize1 platform/chromium/virtual/threaded/compositing/visibility/visibility-image-layers.html CCQuadCullerTest.verifyCullCenterTileNonIntegralSize2 CCLayerTreeHostCommonTest.verifyTransformsForSimpleHierarchy compositing/color-matching/image-color-matching.html CCLayerTreeHostCommonTest.verifyTransformsForSingleLayer compositing/visibility/visibility-image-layers.html compositing/reflections/reflection-in-composited.html CCQuadCullerTest.verifyCullSubRegion2 CCQuadCullerTest.verifyCullCenterTileOnly CCQuadCullerTest.verifyNonAxisAlignedQuadsSafelyCulled CCQuadCullerTest.verifyNonAxisAlignedQuadsDontOcclude compositing/geometry/outline-change.html compositing/iframes/composited-iframe-scroll.html
WebKit Review Bot
Comment 7 2012-04-18 11:48:47 PDT
Created attachment 137734 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Dana Jansens
Comment 8 2012-04-18 14:22:47 PDT
Comment on attachment 137718 [details] Patch for preview View in context: https://bugs.webkit.org/attachment.cgi?id=137718&action=review There's some cleanup to do clearly from the tests :) > Source/WebCore/platform/graphics/chromium/LayerChromium.h:219 > + const TransformationMatrix& targetSurfaceSpaceTransform() const { return m_targetSurfaceSpaceTransform; } > + void setTargetSurfaceSpaceTransform(const TransformationMatrix& matrix) { m_targetSurfaceSpaceTransform = matrix; } I think this should be called originTransform(). That's the terminology we use elsewhere for this transform. > Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp:425 > - float widthScale = bounds().width() / static_cast<float>(contentBounds().width()); > - float heightScale = bounds().height() / static_cast<float>(contentBounds().height()); > - m_updateRect.scale(widthScale, heightScale); > + m_updateRect.scale(1.0 / contentsScale()); I am concerned about this, and expect it is causing a lot of the failures. Shouldn't the contents scale somehow come into play along with the bounds/contentBounds values? > Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h:221 > + const TransformationMatrix& targetSurfaceSpaceTransform() const { return m_targetSurfaceSpaceTransform; } > + void setTargetSurfaceSpaceTransform(const TransformationMatrix& matrix) { m_targetSurfaceSpaceTransform = matrix; } ditto
Shawn Singh
Comment 9 2012-04-18 15:22:59 PDT
Yeah, I'm pretty sure contentBounds need to be there, both width and height separately, just like Dana said =) (just for reference - https://bugs.webkit.org/show_bug.cgi?id=72919)
Xianzhu Wang
Comment 10 2012-04-18 16:04:43 PDT
(In reply to comment #9) > Yeah, I'm pretty sure contentBounds need to be there, both width and height separately, just like Dana said =) (just for reference - https://bugs.webkit.org/show_bug.cgi?id=72919) The reason of bug 72919 was because the scale was not respected, and the patch of it uses contentBounds/bounds to calculate the scale. However the scale is already available and I think using contentsScale() is more accurate than contentBounds/bounds. I believe the situation in TiledLayerChromiumTest.verifyUpdateRectWhenContentBoundsAreScaled (different scales in x and y) won't occur in real world because contentBounds is calculated from bounds*contentsScale (rounded to IntRect). Using contentBounds/bounds to calculate the scale is a source of inaccuracy of quadTransform's scale (as shown in the third attachment). Scales like 0.998 or 1.003 cause unnecessary blurs when the layer is composited onto the target surface.
Dana Jansens
Comment 11 2012-04-18 16:13:36 PDT
Oh okay, that makes a lot of sense. Thanks.
Shawn Singh
Comment 12 2012-04-18 16:29:59 PDT
(In reply to comment #10) > (In reply to comment #9) > > Yeah, I'm pretty sure contentBounds need to be there, both width and height separately, just like Dana said =) (just for reference - https://bugs.webkit.org/show_bug.cgi?id=72919) > > The reason of bug 72919 was because the scale was not respected, and the patch of it uses contentBounds/bounds to calculate the scale. However the scale is already available and I think using contentsScale() is more accurate than contentBounds/bounds. > > I believe the situation in TiledLayerChromiumTest.verifyUpdateRectWhenContentBoundsAreScaled (different scales in x and y) won't occur in real world because contentBounds is calculated from bounds*contentsScale (rounded to IntRect). > > Using contentBounds/bounds to calculate the scale is a source of inaccuracy of quadTransform's scale (as shown in the third attachment). Scales like 0.998 or 1.003 cause unnecessary blurs when the layer is composited onto the target surface. I think what you're saying is true for content layers, but not for image layers. The reason I had done non-uniform scaling via contentBounds is because of ImageLayerChromium. Image layers do not derive their contentBounds from contentScale. Instead, they directly use the image's actual size. If the image layer's contentBounds are not the same aspect ratio as the layer itself, then I think we have to use a non-uniform scale to convert between content space and layer space... am I missing something?
Shawn Singh
Comment 13 2012-04-18 16:31:36 PDT
I haven't thought too deeply about it, but perhaps the solution is to separate contentsScale into their respective x and y scales, and make it virtual so that image layers can override it?
Xianzhu Wang
Comment 14 2012-04-18 16:41:15 PDT
(In reply to comment #13) > I haven't thought too deeply about it, but perhaps the solution is to separate contentsScale into their respective x and y scales, and make it virtual so that image layers can override it? Sorry I missed the image layer. I like your idea of separated x and y scales.
Note You need to log in before you can comment on or make changes to this bug.