921051 - Test the effect of switching Australis selected tab curves from clip-paths to SVG curves with and without caching

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

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

On all three platforms, the selected tab is drawn as follows:

1) Fill the tab-background-start::before, tab-background-middle, tab-background-end::before elements with -moz-dialog as a background-color
2) Apply a linear-gradient on top of those elements. That linear-gradient does not use any system colours.
3) Clip the tab-background-start::before and tab-background-end::before elements to the curve shape using an SVG clip-path
4) Put a tab stroke image on top of the tab-background-start::before, tab-background-middle, tab-background-end::before elements.

Bug 921038 is exploring moving the linear-gradient from step 2 into the image of step 4.

According to the Graphics team, clip-path might not be our best option here. Every pixel that might be drawn in the layers with clip-paths need to check to see whether or not they're within the clipped region. This is expensive - especially for things like the tab label.

Graphics has suggested we switch back to our original approach of drawing the selected tab curve with SVG. This, coupled with the SVG caching in bug 764299, could supply us with a nice all-around TART win.

Comment 1

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

Attachment: POC patch - switches from clip-path to ugly SVG curves.

This patch does the job of swapping out clip-paths in favour of SVG curves. Note that it also does away with the linear gradient that's normally applied on top of -moz-dialog, as I'm hoping we will eventually merge that into the stroke image in bug 921038.

Comment 2

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

Ok, pushed these curves to try, along with a baseline, and a version with the caching patch landed.

Baseline: https://tbpl.mozilla.org/?tree=Try&rev=f98a22410276
SVG curves (no caching): https://tbpl.mozilla.org/?tree=Try&rev=f893739f5b9f
SVG curves (with caching): https://tbpl.mozilla.org/?tree=Try&rev=4ffede0e9c59

I'm running TART, ts_paint, and tpaint.

Comment 3

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

Ok, data's in. I did multiple runs on Windows XP, 7 and 8, for both tpaint, ts_paint, and TART.

Here's the compare-talos for SVG curves (no caching) vs SVG curves (with caching):

http://compare-talos.mattn.ca/?oldRevs=f893739f5b9f&newRev=4ffede0e9c59&server=graphs.mozilla.org&submit=true

Here's the compare-talos for Baseline vs SVG curves (no caching):

http://compare-talos.mattn.ca/?oldRevs=f98a22410276&newRev=f893739f5b9f&server=graphs.mozilla.org&submit=true

And here's the compare-talos for Baseline vs SVG curves (with caching):

http://compare-talos.mattn.ca/?oldRevs=f98a22410276&newRev=4ffede0e9c59&server=graphs.mozilla.org&submit=true

And for good measure, here's a recent m-c vs SVG curves (with caching):

http://compare-talos.mattn.ca/?oldRevs=f77c6ef21e9d&newRev=4ffede0e9c59&server=graphs.mozilla.org&submit=true

So here are some conclusions that I'm drawing from this data:

1) TART performance suffers if we just switch to SVG curves and do nothing else - check out the breakdown on XP:

http://compare-talos.mattn.ca/breakdown.html?oldTestIds=29895107,29896437,29896455,29896615&newTestIds=29895563,29896467,29896489,29896537&testName=tart&osName=Windows%20XP&server=graphs.mozilla.org

When we switched from SVG curves to the SVG clip-path's, we made the right call - SVG curves without caching seem to be more expensive than clipping gradients and colours.

2) Caching *definitely* helps. Here's the breakdown of SVG curves (no caching) vs SVG curves (with caching) on Windows XP:

http://compare-talos.mattn.ca/breakdown.html?oldTestIds=29895563,29896467,29896489,29896537&newTestIds=29895623,29896481,29896505,29896513&testName=tart&osName=Windows%20XP&server=graphs.mozilla.org

3) It seems that if we switch to SVG curves and we have SVG caching, we'll gain a nice performance boost. Here's a breakdown of baseline vs SVG curves (with caching) on Windows XP:

http://compare-talos.mattn.ca/breakdown.html?oldTestIds=29895107,29896437,29896455,29896615&newTestIds=29895623,29896481,29896505,29896513&testName=tart&osName=Windows%20XP&server=graphs.mozilla.org

4) I think this performance boost will neutralize the TART regression on Windows XP. Here's the breakdown of a recent m-c vs SVG curves (with caching) on Windows XP:

http://compare-talos.mattn.ca/breakdown.html?oldTestIds=29885313&newTestIds=29895623,29896481,29896505,29896513&testName=tart&osName=Windows%20XP&server=graphs.mozilla.org

5) SVG curves (with caching) does not appear to introduce any new tpaint regressions on Windows.

6) SVG curves (with caching) *may* introduce a 1.36% regression for ts_paint on Windows 8, although that, again, could be measurement noise.


So, big take-away's:

1) Caching works! Seth rocks!
2) I think we should strongly consider switching to SVG curves with caching for the Australis curvy tabs. I think I'd like to test the effects on OS X and Linux as well before giving it my 100% let's-do-this.

Comment 4

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

Attachment: 921051-poc-v1.1.diff

Cleaned up proof-of-concept patch a little, and getting ready to implement SVG curves for OS X and Linux for testing.

Comment 5

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

Attachment: 921051-poc-v1.2.diff

Whoops - accidentally snuck a bunch of SVG caching stuff in there. Fixed.

Comment 6

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

Attachment: 921051-poc-v1.3

K, OSX works, moving on to Linux now...

Comment 7

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

Attachment: 921051-v1.4.diff

Whoops, forgot to add the OS X SVG files.

Comment 8

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

Ok, I've ported my POC SVG curves to Linux GTK, and I've pushed non-PGO to try:

Baseline: https://tbpl.mozilla.org/?tree=Try&rev=7e3b9d66da9c
SVG curves (no caching): https://tbpl.mozilla.org/?tree=Try&rev=6e4666dd04d0
SVG curves (caching): https://tbpl.mozilla.org/?tree=Try&rev=5b7260dfa417

I've also pushed a build that does SVG curves without caching that's going to spew an SPS profile for us, since jrmuizel is curious why SVG curves without caching is slower than clipping:

SVG curves (no caching) - profiled: https://tbpl.mozilla.org/?tree=Try&rev=ef9826b7bf70

Comment 9

[Jonathan Watt [:jwatt]]

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". I'll comment in the spinoff bug, bug 921673, as to why the latter is likely slower than the former.

The above said, there's a third implementation strategy (what I initially thought you were doing), "inline SVG curves" that we've discussed at the Rendering work week. I'd expect this approach would give better performance than any of the alternatives explored above (assuming inline SVG in XUL works well enough to be usable). This would mean something along the lines of changing:

  <xul:hbox xbl:inherits="pinned,selected,titlechanged"
            class="tab-background-start"/>

to:

  <xul:hbox xbl:inherits="pinned,selected,titlechanged"
            class="tab-background-start">
    <svg:svg flex="1">
      <!-- curve here -->
    </svg:svg>
  </xul:hbox>

I know that inline-SVG-in-XUL works to a certain extent, but it's not something I've tried doing myself and XUL is not something I'm familiar with. Hopefully it can be made to work without too many issues though.

Comment 10

[Jonathan Watt [:jwatt]]

Regarding the markup in comment 9, I'm assuming this is the right code I'm looking at:

http://hg.mozilla.org/projects/ux/file/0745d47f1cbb/browser/base/content/tabbrowser.xml#l4627

Comment 11

[Matthew N. [:MattN]]

The approach in bug 921038 is working out so I think we can call the investigation fixed, at least for now.

(In reply to Jonathan Watt [:jwatt] from 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". I'll comment in
> the spinoff bug, bug 921673, as to why the latter is likely slower than the
> former.

"SVG-as-an-image containing curves" ended up being faster than "SVG clip-path from a resource document" once caching (bug 764299) landed. Bug 914617 may make the "clip-path from a resource document" faster again but we don't want to wait on that as we want to be ready to land near the beginning of Firefox 28 on m-c.

> The above said, there's a third implementation strategy (what I initially
> thought you were doing), "inline SVG curves" that we've discussed at the
> Rendering work week. I'd expect this approach would give better performance
> than any of the alternatives explored above (assuming inline SVG in XUL
> works well enough to be usable).

Sure, this is an obvious way to implement it if we didn't allow changing themes in the browser. The problem is that Australis is a theme/skin change whereas changes to browser/base/ affect all themes. If we did this approach, all third-party themes would have their tabs look broken from partial Australis tab styling on the ends. It's also a matter of separating content and style. When I considered this approach before, I also wondered about the overhead of all of the additional DOM nodes this would cause but I don't know how much that would add up to.

I suppose our themes may have been able to do a XUL overlay on the content which would keep the separation. I didn't think about that until now though so unless we run into problems (perf or otherwise), I think we'll proceed with bug 921038 since we bug 914617 is too far out.

Thanks for the info. I'll definitely look at this approach again if we need to.

Comment 12

[Jonathan Watt [:jwatt]]

Sure thing. And ah, I see.

One other thing to note is that the caching doesn't help on browser startup (obviously, since there's nothing cached) so choosing "SVG-as-an-image containing curves" with caching probably results in slower startup than "SVG clip-path from a resource document". Not saying that isn't the right trade-off - just something to note.

Comment 13

[Avi Halachmi (:avih)]

(In reply to Jonathan Watt [:jwatt] from comment #12)
> ... so choosing "SVG-as-an-image
> containing curves" with caching probably results in slower startup than "SVG
> clip-path from a resource document". Not saying that isn't the right
> trade-off - just something to note.

If we do try to answer this question, considering that tab animations are way more frequent then startups and happen in a phase which gets much more scrutiny by users (browsing/usage), I'd generally favor performance of usage/common-operations over few ms of startup.

Comment 14

[Matthew N. [:MattN]]

Ryan, this bug was to test things and that is now done so the bug is fixed. Fixed doesn't mean code had to land on central.