921673 - Figure out how to improve the performance of the Australis tab curves

Status: RESOLVED WONTFIX

(Firefox :: Theme, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

In bug 921051, we noticed that just swapping in SVG curves for the clip-paths to draw the tab curves for Australis results in a performance loss on the TART (tab animation regression) talos test.

Note that performance got much better once coupled with the patches in bug 764299.

Still, jrmuizel from graphics was surprised that the clip-path still beat straight-up SVG, and asked me to produce a profile for him.

So here's a profile of a TART run on Windows XP using SVG to draw the curves, without caching:

http://people.mozilla.org/~bgirard/cleopatra/#report=34a584daa62bf81bd4ac5c7b0b2f09ff0f6264ca

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Windows 7: http://people.mozilla.org/~bgirard/cleopatra/#report=bd83ec13415ebf20e0f969bd62be62c25437d6eb
Windows 8: http://people.mozilla.org/~bgirard/cleopatra/#report=9e5fbfbea191685c3889f113a5430c8280a131f0

Let me know if you want OS X.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Proactively assigning this to jrmuizel, since I'm pretty sure he's going to be the one who will do the investigating here.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Snow Leopard: http://people.mozilla.org/~bgirard/cleopatra/#report=0792fd00ef18a6e080f5a895877231c4d9174987
Lion: http://people.mozilla.org/~bgirard/cleopatra/#report=0f42b5a3e5b9ddafe63940ca66ccc2f8d5ee30b9
Mountain Lion: http://people.mozilla.org/~bgirard/cleopatra/#report=90351e210514a12a3383852f36abff454ea3a28d

Comment 4

[Mike Conley (:mconley) (:⚙️)]

This may or may not be relevant to BenWa's interests as well.

Comment 5

[Jonathan Watt [:jwatt]]

As I just mentioned in bug 921051 comment 9, the characterization of the two implementation strategies as "SVG curves" and "SVG clip-path" is somewhat misleading. My initial thought was that we were talking about curves and clip-path that are inline (in the document that is using them), in which case the performance of stroked curves being worse than clip-path would be very surprising (but we're not).

Less misleading (if somewhat verbose) terms might be "SVG clip-path from a resource document" and "SVG-as-an-image containing curves".

We talked about this at the ongoing Rendering work week in Paris, and it's worth noting that SVG-as-an-image has some significant overhead that clip-path referenced from a resource document does not have. Specifically, embedding an SVG-as-an-image results in the creation of a separate layer manager, layer builder and display list builder, and the running lots of code related to those objects during the layout and paint passes. Note that the presence of the 'viewBox' attribute on the root <svg> tag in the embedded SVG also results in an implicit transform, which results in the creation of an nsDisplayTransform.

So this is very far from a simple "path stroking" vs "path clipping" perf comparison here.

Comment 6

[Jonathan Watt [:jwatt]]

Fixes to bug 869505's dependencies might help a little here, but it may be that for the most part the layer manager, layer builder and display list builder overhead is the majority of the overhead and I'm not sure how avoidable that is.

Comment 7

[Matthew N. [:MattN]]

(In reply to Jonathan Watt [:jwatt] from comment #5)
> Specifically, embedding an SVG-as-an-image results in the creation of a separate layer
> manager, layer builder and display list builder, and the running lots of
> code related to those objects during the layout and paint passes.

This overhead should only happen for the initial rasterization after bug 764299, right? That would mean it may affect startup (ts_paint) a bit but afterwards it shouldn't?

> Note that
> the presence of the 'viewBox' attribute on the root <svg> tag in the
> embedded SVG also results in an implicit transform, which results in the
> creation of an nsDisplayTransform.

I can look into removing the viewBox but I put it in before due to rounding issues at the edge of the shape IIRC.

Comment 8

[Jonathan Watt [:jwatt]]

(In reply to Matthew N. [:MattN] (catching up on reviews) from comment #7)
> This overhead should only happen for the initial rasterization after bug
> 764299, right? That would mean it may affect startup (ts_paint) a bit but
> afterwards it shouldn't?

Yes, but in fact I have an idea on how to get rid of the startup overhead too. I'll file a bug about that and note it here.

Comment 9

[Jonathan Watt [:jwatt]]

Backing up a bit, what's actually needed here? Do you just need to be able to clip an element to a curved path?

If so, maybe we should push to get path() added to CSS basic-shapes:

http://dev.w3.org/csswg/css-shapes-1/#typedef-basic-shape

so that the 'clip-path' property:

http://dev.w3.org/fxtf/css-masking-1/#the-clip-path

can use SVG paths without requiring any type of SVG document to be created at all. There seems to be some support for doing so:

http://lists.w3.org/Archives/Public/www-style/2013Oct/0199.html

Would that meet the needs of the Australis UI?

Comment 10

[Seth Fowler [:seth] [:s2h]]

Mike, we're discussing this at the work week and trying to decide how we might push this forward. Could you comment on comment 9?

Comment 11

[Mike Conley (:mconley) (:⚙️)]

I... think that would satisfy our needs? MattN, sanity-check me?

Comment 12

[Matthew N. [:MattN]]

To summarize what we're doing now: 
If you load chrome://browser/skin/tabbrowser/tab-selected-start.svg in your browser you will see the result of a bunch of pre-processing and the usage of clip-path that was a performance issue. This clip-path rule used to be apply to each end of the tab in browser.xul but it wasn't cached so was expensive. My hack was to move this clip-path into a SVG image since we got SVG-as-an-image caching implemented and this way the result of the clip-path is cached.  

The theory is that first-paint would be faster if we didn't have to construct 2 SVG document (tab-selected-end.svg and tab-selected-start.svg) at startup in order to get a cached clipping and instead could just clip the elements in browser.xul and have their results cached so that tab animations (TART) stays performant. We could get rid of the mess in https://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/tab-selected.svg?raw=1 if this pans out. I filed bug 914617 for clip-path caching.

(In reply to Jonathan Watt [:jwatt] from comment #9)
> Do you just need to be able to clip an element to a curved path?

With our current (less than ideal but mostly working) architecture: Yes. We basically need to clip an element that has the tab background colour (solid or linear gradient) to the tab curve for each side of a tab.

(In reply to Jonathan Watt [:jwatt] from comment #9)
> Would that meet the needs of the Australis UI?

It sounds like that may allow us to get rid of tab-selected*.svg but I don't know if that will help or hinder performance since we will lose our SVG-as-in-image caching and unless we have bug 914617 then that means we have no caching so it could actually be worse overall.

Comment 13

[Jonathan Watt [:jwatt]]

Actually it's unlikely bug 869505 is much of a factor in this case.

Comment 14

[Jonathan Watt [:jwatt]]

I think that comment 5 answered the original question raised by this bug. But since the bug has morphed I'll change the Summary and Component rather than closing it. MattN: please file specific things you want done by Platform (if they're not already filed) as follow-ups.

Comment 15

[Ben Kelly [:bkelly, not reviewing]]

FWIW our tabs seems pretty janky with the container color decoration applied to them in nightly.  See bug 1349211.

Comment 16

[Florian Quèze [:florian]]

I'm afraid we don't care about optimizing the Australis tab curves anymore.