Closed Bug 1335139 Opened 5 years ago Closed 5 years ago

Figure out whether there's a way to stroke text without doing slow path-drawing stuff

Categories

(Core :: Canvas: 2D, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: bzbarsky, Assigned: lsalzman)

Details

(Keywords: perf, testcase, Whiteboard: [gfx-noted])

Attachments

(3 files)

Attached file Testcase
Canvas fillText seems to be a good bit faster than canvas strokeText.  This is apparently because we don't use our glyph cache for strokeText, which means we have to do a bunch of path stuff on every call.

See attached testcase, which lets you test both accelerated and non-accelerated canvas (by relying on the minimum height of 16 we have for accelerated canvases).
Has STR: --- → yes
Keywords: perf, testcase
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All
Whiteboard: [gfx-noted]
This adds a basic interface for DrawTarget::StrokeGlyphs. By default, this doesn't really do anything different than GlyphBufferAzure::FlushStroke did, so at least for the unoptimized case behavior should remain unchanged if draw targets don't wish to implement it.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Attachment #8832177 - Flags: review?(bas)
This just abstracts what used to be DrawTargetSkia::FillGlyphs into DrawGlyphs, which can optionally stroke the glyphs. Now FillGlyphs and StrokeGlyphs route to this as appropriate. The wins from this seem quite gigantic, since we now always hit the glyph cache, and StrokeGlyphs has performance comparable to FillGlyphs.
Attachment #8832178 - Flags: review?(mchang)
Comment on attachment 8832178 [details] [diff] [review]
part 2 - implement StrokeGlyphs for DrawTargetSkia

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

How big is a gigantic win?
Attachment #8832178 - Flags: review?(mchang) → review+
> How big is a gigantic win?

On the microbenchmark, a factor of 3 or so.

On the testcase that prompted this bug originally, looks like about 25% (going from 20 to 25 fps).  But there are other bottlenecks there too that we're still working on removing, so if we removed those first this would be a bigger win in percentage terms, I expect.  ;)
Just as a heads-up, I get try failures with this patch (I happened to have this in my tree when I pushed).  Specifically, layout/reftests/text-stroke/webkit-text-stroke-property-00[1-5].html fail.
Just to be clear about why we want this change at all:

The testcase in question comes from people complaining about how Firefox is slow (and in fact the slowest browser except IE) on hacker news and me asking for examples.  See https://news.ycombinator.com/item?id=13487039

The application in question is an imaging application; it has a bunch of image data it's trying to show the user and then it wants to allow the user to navigate that image data (e.g. move back/forward in time, etc).  There is a bunch of text overlayed on the image bits to describe metadata (e.g. timestamps, etc) attached to the currently viewed image.

Due to how the images are being compressed in their case (not using an image compression format browsers support), they are having to do decompression in JS and draw to canvas.

The specific case I've been looking at an profiling that led to this bug and the others I filed is runs at about 10-11fps in Firefox and about 30fps in Chrome...  There's no one hotspot, but several things that we've now got bugs on.

For this _specific_ bug, I think it's quite surprising to have strokeText and fillText have different performance characteristics, and if we can make them not do that, and in general look more similar, that seems like a pretty obvious win to me.
This is on Windows?  What happens if you switch canvas to skia from d2d?
I'm on Mac, unfortunately, but the fps numbers reported above are on Windows.  That said, I see pretty similar numbers on Mac without the various in-fight patches here.

I'm still waiting to hear back about sharing the link with someone else who would be able to measure/profile on Windows....
Attachment #8832177 - Flags: review?(bas) → review+
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/da0241dcbd90
part 1 - add DrawTarget::StrokeGlyphs fast path for stroked glyphs. r=bas.schouten
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ef7908abd6c
part 2 - implement StrokeGlyphs for DrawTargetSkia. r=mchang
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2a7c829a8ac
followup - fix reftests on Android. r=me
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/43bc01e57bfa
followup - fix GC hazard in Canvas2D. r=me
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1e6afa7836f
followup - fix OSX unexpected reftest passes. r=me
I suspect this bug caused the dom/canvas/test/reftest/1177726-text-stroke-bounds.html reftest to start passing with QuantumRender.
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b9e865bb255
Followup to mark a reftest passing with webrender. r=me
You need to log in before you can comment on or make changes to this bug.