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

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: 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).

Comment 1

[Lee Salzman [:lsalzman]]

Attachment: part 1 - add DrawTarget::StrokeGlyphs fast path for stroked glyphs

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.

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: part 2 - implement StrokeGlyphs for DrawTargetSkia

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.

Comment 3

[Mason Chang [Inactive] [: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?

Comment 4

[Boris Zbarsky [:bzbarsky]]

> 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.  ;)

Comment 5

[Boris Zbarsky [:bzbarsky]]

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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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.

Comment 7

[Milan Sreckovic [:milan] (needinfo for best results)]

This is on Windows?  What happens if you switch canvas to skia from d2d?

Comment 8

[Boris Zbarsky [:bzbarsky]]

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....

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2a7c829a8ac
followup - fix reftests on Android. r=me

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/43bc01e57bfa
followup - fix GC hazard in Canvas2D. r=me

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1e6afa7836f
followup - fix OSX unexpected reftest passes. r=me

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/da0241dcbd90
https://hg.mozilla.org/mozilla-central/rev/0ef7908abd6c
https://hg.mozilla.org/mozilla-central/rev/d2a7c829a8ac
https://hg.mozilla.org/mozilla-central/rev/43bc01e57bfa
https://hg.mozilla.org/mozilla-central/rev/a1e6afa7836f

Comment 14

[Kartikaya Gupta (email:[e-mail])]

I suspect this bug caused the dom/canvas/test/reftest/1177726-text-stroke-bounds.html reftest to start passing with QuantumRender.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b9e865bb255
Followup to mark a reftest passing with webrender. r=me

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4b9e865bb255