790861 - SVG creates unnecessary nsDisplayTransform items with the identity transform

Status: NEW

(Core :: SVG, defect)

Description

[Matt Woodrow (:mattwoodrow)]

Attachment: Check if the actual transform is the identity

SVG is frequently creating nsDisplayTransform display items with the identity transform. This seems fairly pointless, and wasteful.

The attached patch fixes it, but doesn't seem like a great way to go.

Making nsSVGSVGElement::HasChildrenOnlyTransform return false when PrependLocalTransformsTo is going to return the identity transform seems better, but it wasn't obvious (to me) how to do this.

Comment 1

[Jonathan Watt [:jwatt]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #0)
> SVG is frequently creating nsDisplayTransform display items with the
> identity transform.

When are you seeing this? It should really be very rare - at least when it comes to the part of the code that you're patching. For SVGSVGElement::HasChildrenOnlyTransform to return true for the identity matrix, we pretty much have to have a viewBox set to the value "0 0 w h" where w and h exactly match the width and height of the viewport, respectively. The arbitrary nature of the dimensions of the viewport make that pretty unlikely.

Comment 2

[Jonathan Watt [:jwatt]]

Fixing bug 828240 would make this pretty much a non-issue for children-only transforms (what you're patching).

Comment 3

[Jonathan Watt [:jwatt]]

I also think that I did this deliberately...something to do with animations _to_/_through_ the identity transform messing up at the point that they hit identity. I can't quite recall the details now though.

Comment 4

[Jonathan Watt [:jwatt]]

Comment on attachment 660695 [details] [diff] [review]
Check if the actual transform is the identity

Cancelling review request for now until we have an example of how this can hurt us, or some other argument for making a change like this.

Comment 5

[Tom Klein]

I think we're creating a transform for every anonymous child now since bug 1458968, even if it's the identity.  I guess that's considered necessary though :) so not sure what that means for this bug.