This patch (coming shortly) extends the comments in calculateDrawTransformsAndVisibility, so that this complex function is hopefully a bit more readable. This change was originally part of https://bugs.webkit.org/show_bug.cgi?id=69197 but it made sense to separate them since feedback may still continue on this one.
Created attachment 110396 [details] Patch
Comment on attachment 110396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110396&action=review > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:117 > + // 4. Definition of various transformsed used: typo "transformsed"
Created attachment 110417 [details] Patch
Comment on attachment 110417 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=110417&action=review Awesome. This is much more understandable. :) > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:102 > + // projection applied at render time flips the Y axis appropriately. s/render/draw/, for consistency > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:122 > + // Tr[o2a] is the translation from the layer's origin to its anchor point > + // Tr[o2c] is the translation from the layer's origin to its center I realize it's wordy, but originToAnchor, originToCenter, anchorToCenter would be better than abbreviating. > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:129 > + // Tr[a2c] = T[o2a].inverse * T[o2c] T => Tr?, inverse => inverse()? > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:140 > + // The draw transform for the layer is: It is a little inconsistent to me that draw transforms go to the center of the layer, but screen space transforms go to the origin. Can you maybe call this out explicitly in these comments? I know it's obvious from the math, but... > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:146 > + // = M[root] * Tr[layer->position()] * M[l] * Tr[o2a].inverse() M[l] => M[layer]?
OK thanks for the comments! Will upload a new patch tonight.
Created attachment 110465 [details] addressed review comments
Looks unofficially good to me. Thanks for doing this. :)
Comment on attachment 110465 [details] addressed review comments View in context: https://bugs.webkit.org/attachment.cgi?id=110465&action=review This is looking great. Just one more possible addition to consider. Otherwise, an (unofficial) r+ from me. > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostCommon.cpp:144 > + // Interpreting the math left-to-right, this transforms from the layer's render surface to the center of the layer. I think it would be nice to put another point in the list above about render surfaces, since you're mentioning them here. Something to help folks understand that the draw transform is relative to the layer's target render surface, not to the viewport.
Created attachment 110702 [details] added point 5 about render surface
(In reply to comment #9) > Created an attachment (id=110702) [details] > added point 5 about render surface Looks great. Thanks.
Comment on attachment 110702 [details] added point 5 about render surface Clearing flags on attachment: 110702 Committed r97685: <http://trac.webkit.org/changeset/97685>
All reviewed patches have been landed. Closing bug.