RESOLVED FIXED 55984
[chromium] Allow "large" composited layers to have complex draw transforms 
https://bugs.webkit.org/show_bug.cgi?id=55984
Summary [chromium] Allow "large" composited layers to have complex draw transforms
Vangelis Kokkevis
Reported 2011-03-08 16:58:30 PST
The compositor currently skips drawing "large" layers whose draw transform isn't a pure translation. This restriction is unnecessary as long as the visible portion of the layer still fits within the max texture size.
Attachments
Patch (2.69 KB, patch)
2011-03-08 17:17 PST, Vangelis Kokkevis 		no flags
DetailsFormatted DiffDiff
Patch (244.04 KB, patch)
2011-03-11 09:57 PST, Vangelis Kokkevis 		jamesr: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Vangelis Kokkevis
Comment 1 2011-03-08 17:17:36 PST
Created attachment 85113 [details] Patch
James Robinson
Comment 2 2011-03-08 17:19:51 PST
Comment on attachment 85113 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=85113&action=review Can we get a test for a layer that is large, has a transform, but is still renderable? > Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:105 > + // If we cannot invert the layer matrix then we cannot compute how large the visible > + // area is. This shouldn't be an issue unless the surface of the layer is aligned > + // with the Z axis in which case the layer contents won't be visible anyway. > + if (!layerOriginTransform.isInvertible()) { I don't quite get what the relationship is between the comment and the code - the comment seems to say that we don't care if the matrix is invertible, but the code definitely does.
James Robinson
Comment 3 2011-03-08 18:10:12 PST
Comment on attachment 85113 [details] Patch R- for lack of test
Vangelis Kokkevis
Comment 4 2011-03-08 18:33:28 PST
(In reply to comment #2) > (From update of attachment 85113 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=85113&action=review > > Can we get a test for a layer that is large, has a transform, but is still renderable? > > > Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:105 > > + // If we cannot invert the layer matrix then we cannot compute how large the visible > > + // area is. This shouldn't be an issue unless the surface of the layer is aligned > > + // with the Z axis in which case the layer contents won't be visible anyway. > > + if (!layerOriginTransform.isInvertible()) { > > I don't quite get what the relationship is between the comment and the code - the comment seems to say that we don't care if the matrix is invertible, but the code definitely does. Need to work on the wording of the comment I guess. What I'm trying to say is that if the transform is not invertible then we cannot figure out which parts of the layer contents are visible and therefore we have to bail. The most common case of a non-invertible matrix in this case is if the layer is perpendicular to the "page" (or containing rendersurface) in which case it really doesn't matter if we don't render it.
Vangelis Kokkevis
Comment 5 2011-03-11 09:57:22 PST
Created attachment 85485 [details] Patch
Vangelis Kokkevis
Comment 6 2011-03-11 09:59:07 PST
(In reply to comment #3) > (From update of attachment 85113 [details]) > R- for lack of test Please have another look. I had to tweak the math for computing the visible part of the large layer as what we had previously didn't work for arbitrary transformations. Also added a layout test.
James Robinson
Comment 7 2011-03-11 10:53:48 PST
Comment on attachment 85485 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=85485&action=review R=me. hurray for tests finding bugs > LayoutTests/platform/chromium/compositing/huge-layer-rotated.html:18 > +<body> please set overflow:hidden on the <body> so it doesn't have a visible scrollbar - that way we'll be able to share pixel results across platforms.
Vangelis Kokkevis
Comment 8 2011-03-11 11:51:24 PST
Committed r80867: <http://trac.webkit.org/changeset/80867>
WebKit Review Bot
Comment 9 2011-03-11 12:19:04 PST
http://trac.webkit.org/changeset/80867 might have broken Windows Release (Build) and Windows Debug (Build)
Note You need to log in before you can comment on or make changes to this bug.