Shave off a TransformationMatrix copy if RenderLayer's transparencyClipBox()
Created attachment 267091 [details] Patch
Comment on attachment 267091 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267091&action=review > Source/WebCore/rendering/RenderLayer.cpp:1762 > + transform.multiply(*layer.transform()); According to the comment for TransformationMatrix::multiply(), this change affects multiplication order.
Comment on attachment 267091 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267091&action=review >> Source/WebCore/rendering/RenderLayer.cpp:1762 >> + transform.multiply(*layer.transform()); > > According to the comment for TransformationMatrix::multiply(), this change affects multiplication order. The comments for the * and *= operators in TransformationMatrix are a bit misleading. Both `A = A * B` and `A *= B` call `A.multiply(B)`, which sets the matrix data of A to the results of a BA matrix product. So A *= B behaves the same as A.multiply(B), and both yield the same result as `A = A * B`, but the latter is more wasteful since it first copies the A matrix into a temporary object, multiplies B into that, and assigns the temporary object to A (which may produce another copy, not sure).
Comment on attachment 267091 [details] Patch OK. Do tests break if you deliberately change the order?
(In reply to comment #4) > Comment on attachment 267091 [details] > Patch > > OK. Do tests break if you deliberately change the order? No, at least the CSS and fast/ tests don't.
Comment on attachment 267091 [details] Patch Clearing flags on attachment: 267091 Committed r194576: <http://trac.webkit.org/changeset/194576>
All reviewed patches have been landed. Closing bug.
(In reply to comment #3) > So A *= B behaves the same as A.multiply(B), and both yield the same result > as `A = A * B`, but the latter is more wasteful since it first copies the A > matrix into a temporary object, multiplies B into that, and assigns the > temporary object to A (which may produce another copy, not sure). Is there something straightforward we can do to fix A.multiply(B) so that it is not more wasteful than A *= B? Or perhaps we need to instead remove the multiply function.
Oh, I guess I got it backwards and it's the multiply function that is more efficient.
`A *= B` and `A.multiply(B)` are identical, the o*= operator implementation calls multiply(). The * operator is still useful, but it's easy enough to misuse it like `A = A * B` does. As to spotting these occurrences -- some time ago I was playing around with Clang's AST matching facilities which allow the user to inspect the AST for specific code patterns. I used that mostly to spot wasteful container copies, and was also able to spot this specific issue. Extending that to cover additional patterns that we would want to eliminate from the WebKit codebase would probably be a worthwhile side-project.