Joel Maher ( :jmaher ) (UTC -8) Reporter
	Description • 10 years ago

we have a svg regression that appears to be windows 8 only:
http://graphs.mozilla.org/graph.html#tests=[[281,131,31]]&sel=1403153338000,1403326138000&displayrange=7&datatype=running

I did some retriggers and we go from ~211 -> ~222:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&fromchange=ac426472ceec&tochange=8bc5711c589e&jobname=WINNT%206.2%20mozilla-inbound%20talos%20svgr

Here is some information about the tsvgx test:
https://wiki.mozilla.org/Buildbot/Talos/Tests#tsvg.2C_tsvgx

You should be able to test a fix locally or via try server.

	Timothy Nikkel (:tnikkel) Assignee
	Comment 1 • 10 years ago

One step ahead of you ;) already investigating this.

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 2 • 10 years ago

thanks!  Glad to see you taking action on the automated alerts, you make me happy :)

	Timothy Nikkel (:tnikkel) Assignee
	Comment 3 • 10 years ago

try confirms this fixes it.

	Markus Stange [:mstange]
	Comment 4 • 10 years ago

Comment on attachment 8447588 [details] [diff] [review]
patch

Review of attachment 8447588 [details] [diff] [review]:
-----------------------------------------------------------------

Should we also add a check to ScaleRegionToOutsidePixels? I suppose the place that now turns empty regions into non-empty ones is this one: http://hg.mozilla.org/mozilla-central/annotate/2f5df65e3662/layout/base/FrameLayerBuilder.cpp#l2045

	Timothy Nikkel (:tnikkel) Assignee
	Comment 5 • 10 years ago

ScaleTo(?)Pixels just calls the request function on the region directly. ScaleToNearestPixels and ScaleToOutsidePixels just use a region iterator in a while loop, so they return an empty region when the given region is empty (it has 0 rects). ScaleToInsidePixels is specialized, but it also returns an empty return for 0 rect regions. So empty regions should stay empty with ScaleTo(?)Pixels.

	Timothy Nikkel (:tnikkel) Assignee
	Comment 6 • 10 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/2c0234a7ba1e

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 7 • 10 years ago

Thanks for fixing this!

	Markus Stange [:mstange]
	Comment 8 • 10 years ago

(In reply to Timothy Nikkel (:tn) from comment #5)
> so they return an empty region when the given region is empty
> (it has 0 rects). ScaleToInsidePixels is specialized, but it also returns an
> empty return for 0 rect regions. So empty regions should stay empty with
> ScaleTo(?)Pixels.

Oh good. So you didn't actually regress bug 1016525, at least not in the sense of re-adding expensive region unioning operations. So what was the perf problem here? Just the operation of transforming an empty rect?

	Timothy Nikkel (:tnikkel) Assignee
	Comment 9 • 10 years ago

Just getting the transform matrix for the frame and transforming it I guess. We should probably expose functions on nsLayoutUtils to get the matrix once and then do the transforming several times.

	Ryan VanderMeulen [:RyanVM]
	Comment 10 • 10 years ago

https://hg.mozilla.org/mozilla-central/rev/2c0234a7ba1e