Closed Bug 752918 Opened 12 years ago Closed 12 years ago

Convert expensive SVG masks to clip-paths

Categories

(Firefox :: Theme, defect)

Product:
Firefox
Component:
Theme
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 15

People

(Reporter: bugs, Assigned: bugs)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [Snappy])

Attachments

(2 files, 3 obsolete files)

screenshot of winstripe-urlbar-back-button-mask (above) and ...-clip-path (below)
(deleted), image/png
Details
This patch reduces precision for less verbose SVG code.
(deleted), patch
dao
: review+
Details | Diff | Splinter Review 
per Roc: I think we might also be able to replace the SVG mask with an SVG clip-path. clip-path should be significantly more efficient since it doesn't require per-pixel luminance calculations.
Attachment #621984 - Flags: feedback?(roc)
Attachment #621984 - Flags: feedback?(ehsan)
No longer blocks: 302667
Comment on attachment 621984 [details] [diff] [review]
Patch converts SVG masks to clipPath elements. Need profiling.

I don't know anything about our theme stuff, Dao should be the person looking at this.
Attachment #621984 - Flags: feedback?(ehsan) → feedback?(dao)
No longer depends on: 743877
Comment on attachment 621984 [details] [diff] [review]
Patch converts SVG masks to clipPath elements. Need profiling.

You need to update browser/themes/winstripe/browser.css too.

nit: please replace "-mask" with "-clipPath" in the SVG node ids.
Component: Tabbed Browser → Theme
QA Contact: tabbed.browser → theme
Attachment #621984 - Flags: feedback?(dao)
Attached patch Final patch per feedback comments. r=dao (obsolete) (deleted) — — Splinter Review 
Addresses feedback comments. Need a final r+ prior to landing.
Assignee: nobody → jet
Attachment #621984 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #622694 - Flags: review?(dao)
Summary: tab UI shouldn't use expensive SVG masks → Convert expensive SVG masks to clip-paths
Note the pixels bleeding through behind the back button especially at the bottom. It looks like the clip path is misaligned or doesn't quite have the right shape.
Attached patch Patch converts SVG masks to clipPath elements. (obsolete) (deleted) — — Splinter Review 
This patch includes clip-paths that very precisely follow the outlines of the original mask shapes they replace. This should resolve any compositing bleed issues.
Attachment #622694 - Attachment is obsolete: true
Attachment #622694 - Flags: review?(dao)
Attachment #623087 - Flags: review?(dao)
This patch simplifies the paths, reduces precision for less verbose SVG markup. Note: this SVG is generated by the Inkscape SVG editor.
Attachment #623087 - Attachment is obsolete: true
Attachment #623087 - Flags: review?(dao)
Attachment #623098 - Flags: review?(dao)
Comment on attachment 623098 [details] [diff] [review]
This patch reduces precision for less verbose SVG code.

Thanks!
Attachment #623098 - Flags: review?(dao) → review+
Keywords: perf
Keywords: checkin-needed
http://hg.mozilla.org/integration/mozilla-inbound/rev/ec19a7304528
Keywords: checkin-needed
Target Milestone: --- → Firefox 15
Version: unspecified → Trunk
https://hg.mozilla.org/mozilla-central/rev/ec19a7304528
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
It looks like this caused a 2% regression in the Talos SVG benchmark, at least on Mac OS X 10.6:
http://graphs.mozilla.org/graph.html#tests=[[22,63,21]]&sel=1336616135166,1337220935166

Should it be backed out?
I don't believe that's possible :-)
Blocks: tabswitch
No longer blocks: 742594
Depends on: 828073
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: