1400917 - wr-text: implement tofu glyphs

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P1)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-84ec09a4-b7b1-42e8-ac89-470180170918.
=============================================================

There are 5 crashes (from 3 installs) in nightly 57 with buildid 20170918100059. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1400411.

[1] https://hg.mozilla.org/mozilla-central/rev?node=8a0b8dfa2d7df3e13bea7f81ed7a17c360eef5d7

Comment 1

[Aria Beingessner [:Gankra]]

Yes, this is my fault. Tofu glyphs take the StrokeRect path; should be easy to fix.

Comment 2

[Aria Beingessner [:Gankra]]

Reverted the problem patch; hijacking this for "implement this properly".

Comment 3

[Calixte Denizet (:calixte)]

:Gankro, there are 8 crashes from the same installation with signature "mozilla::layout::TextDrawTarget::CreatePathBuilder" (see [1]).  

[1] https://crash-stats.mozilla.com/report/index/1edee317-2ac4-46e8-bed2-535280170919

Comment 4

[Julien Cristau [:jcristau]]

Removing the regression/crash keywords since this was repurposed, and setting 57:unaffected because webrender won't ship in 57.

Comment 5

[beastman665]

https://crash-stats.mozilla.com/report/index/b83f8b1d-7b08-49d3-876a-833f20170919
Many crashes by loading Websites

Comment 6

[Aria Beingessner [:Gankra]]

deprioritizing; mostly just a (minor) performance thing. Tofu just triggers fallback on the text-run, which does lose subpixel-AA, but is otherwise fine.

Comment 7

[Lee Salzman [:lsalzman]]

Attachment: render missing glyphs with WebRender

On TextDrawTarget, I implemented StrokeRect for dealing with the tofu border, and FillRect for dealing with the mini font rects.

Since TextDrawTarget doesn't really deal with transforms, and we still need to support writing modes with tofu, I pass in a matrix representing the state of the current glyph transform flags to the render code. The border rect then gets transformed by this like a normal glyph would. It then uses the basis vectors of the matrix to step along the mini font grid, rendering via FillRect.

I tried as much as possible to avoid forking/copy-pasta-ing the missing glyph drawing code, so the final code change is quite small and still easy enough to work with, considering.

Comment 8

[Aria Beingessner [:Gankra]]

Comment on attachment 8943686 [details] [diff] [review]
render missing glyphs with WebRender

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

Sick!

::: gfx/thebes/gfxFont.cpp
@@ +2014,5 @@
>          Float advanceDevUnits =
>              Float(ToDeviceUnits(advance, aRunParams.devPerApp));
>          Float height = GetMetrics(eHorizontal).maxAscent;
> +        // Horizontally center if drawing vertically upright with no sideways transform.
> +        Rect glyphRect = aFontParams.isVerticalFont && !mat.HasNonAxisAlignedTransform() ?

When would a NonAxisAligned transform happen? It seems like WR would mishandle it..?

Comment 9

[Lee Salzman [:lsalzman]]

(In reply to Alexis Beingessner [:Gankro] from comment #8)
> Comment on attachment 8943686 [details] [diff] [review]
> render missing glyphs with WebRender
> 
> Review of attachment 8943686 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Sick!
> 
> ::: gfx/thebes/gfxFont.cpp
> @@ +2014,5 @@
> >          Float advanceDevUnits =
> >              Float(ToDeviceUnits(advance, aRunParams.devPerApp));
> >          Float height = GetMetrics(eHorizontal).maxAscent;
> > +        // Horizontally center if drawing vertically upright with no sideways transform.
> > +        Rect glyphRect = aFontParams.isVerticalFont && !mat.HasNonAxisAlignedTransform() ?
> 
> When would a NonAxisAligned transform happen? It seems like WR would
> mishandle it..?

Here I am just checking if we generated a TRANSPOSE matrix as the comment indicates. The function name is a bit confusing.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36bc055f7ad0
render missing glyphs with WebRender. r=gankro

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/36bc055f7ad0