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

RESOLVED FIXED

Status

()

Firefox
Theme
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

(Blocks: 1 bug, {perf})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M9][Australis:P1])

Attachments

(1 attachment, 4 obsolete attachments)

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.
Created attachment 810603 [details] [diff] [review]
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.
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.
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.
Created attachment 811218 [details] [diff] [review]
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.
Attachment #810603 - Attachment is obsolete: true
Created attachment 811232 [details] [diff] [review]
921051-poc-v1.2.diff

Whoops - accidentally snuck a bunch of SVG caching stuff in there. Fixed.
Attachment #811218 - Attachment is obsolete: true
Created attachment 811301 [details] [diff] [review]
921051-poc-v1.3

K, OSX works, moving on to Linux now...
Attachment #811232 - Attachment is obsolete: true
Created attachment 811309 [details] [diff] [review]
921051-v1.4.diff

Whoops, forgot to add the OS X SVG files.
Attachment #811301 - Attachment is obsolete: true
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
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.
Keywords: perf
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.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [Australis:M?][Australis:P1] → [Australis:M9][Australis:P1]
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.
(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.
Resolution: FIXED → WONTFIX
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.
Resolution: WONTFIX → FIXED
You need to log in before you can comment on or make changes to this bug.