1170688 - Text rgba alpha ignored for synthetic bold text

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Mike Taylor [:miketaylr]]

Attachment: text-shadow-alpha.html

If you open the attached test case (also available at http://output.jsbin.com/tugiraquhi), compare the rendering of the Japanese text and the English text.

Expected: Very faint text-shadow on both.
Actual: Solid text-shadow on Japanese text.

Comment 1

[Mike Taylor [:miketaylr]]

Originally reported at https://github.com/webcompat/web-bugs/issues/1142.

Comment 2

[Mike Taylor [:miketaylr]]

This works as expected on OSX.

Comment 3

[Brian Birtles (:birtles)]

Digging into this a little bit I notice:

* This works fine on Firefox OS
* The font being used for the Japanese glyphs is 'MotoyaLMaru W3 mono'
* It is only the alpha channel being ignored (I can change the color of the shadow)
* The alpha channel is basically being ceiled to 1 (so 0->0, 0.01 -> 1)
* If I make the string 'あa' both glyphs show the problem--i.e. even the Latin 'a' ignores the alpha despite using a different font

Comment 4

[Brian Birtles (:birtles)]

It also only happens when the blur radius is 0.

Comment 5

[Brian Birtles (:birtles)]

This is going to take quite a while before I've got an Android debug build environment up and running due to lack of disk space but so far I can see:

* This isn't about text-shadow, but applies to text alpha in general if you set, e.g. color: rgba(100, 100, 200, 0.5);
* It only appears when we set font: bold

It may be related to doing synthetic bolding since we handle that differently in gfxTextRun::Draw.

At a guess, there may be some odd font metrics in the Motoya fonts that cause us to get the size of the synthetic bold buffer wrong? I tried reproducing this on desktop using the same font but didn't succeed. Perhaps I'm using a newer version of the font, perhaps text-inflation is causing differences on Android, or perhaps there are platform-specific differences here I'm not aware of.

FWIW, the version of the Motoya fonts I'm trying is here:

https://github.com/android/platform_frameworks_base/blob/master/data/fonts/MTLmr3m.ttf

Comment 6

[Brian Birtles (:birtles)]

Narrowing this down a bit further I see the same bug when simply using a font without a bold variant.

  http://jsfiddle.net/gxmhhskj/

Seems like we're not applying the alpha for synthetic bold on Android correctly. Works fine for me on Firefox OS, Windows, and Linux.

Comment 7

[Brian Birtles (:birtles)]

(In reply to Brian Birtles (:birtles) from comment #6)
> Works fine for me on Firefox OS, Windows, and Linux.

I forgot to mention, for Windows and Linux I tested nightly but for Firefox OS I only tested Firefox OS 2.0. I'll check a more recent build of Firefox OS.

Comment 8

[Brian Birtles (:birtles)]

I can see this bug in a more recent Firefox OS build (apparently based on Gecko 41 although I don't remember flashing the device that recently) but not on Firefox OS 2.0 based on Gecko 32.

Comment 9

[John Daggett (:jtd)]

Potentially related enhancement:
https://bugzilla.mozilla.org/show_bug.cgi?id=1173221

It may be that one simple way to fix this would be to use the FreeType embolden feature to solve this problem. That's a better way to synthetically bold than using the generic "stamp" method.

Comment 10

[Brian Birtles (:birtles)]

Since I can repro this on Firefox OS it should be easier to debug.

Comment 11

[John Daggett (:jtd)]

Attachment: testcase, synthetic bold with rgba color ignores alpha

Comment 12

[John Daggett (:jtd)]

The bug will occur on any platform where mApplySyntheticBold is used to enable synthetic bolding - OSX, Android, FFOS, GDI.

Comment 13

[John Daggett (:jtd)]

Just a guess but I suspect this happened when the gfxFont::Draw code was refactored in bug 1037340.

Comment 14

[Brian Birtles (:birtles)]

You can reproduce this on Windows by turning off Hardware Acceleration (under Preferences -> General). 
With Hardware acceleration on, the two lines in test case in attachment 8617729 [details] are DIFFERENT (the second one is translucent).
With Hardware acceleration off, the two lines are the SAME.

Comment 15

[Brian Birtles (:birtles)]

(In reply to Brian Birtles (:birtles) from comment #14)
> You can reproduce this on Windows by turning off Hardware Acceleration
> (under Preferences -> General). 

Sorry, Preferences -> Advanced (requires restart).

Comment 16

[Alice0775 White]

With HWA off and attachment 8617729 [details]

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bd4287c14070&tochange=a6758d669350

Regressed by: bug 1037340

Comment 17

[Brian Birtles (:birtles)]

Thanks Alice!

Comment 18

[Brian Birtles (:birtles)]

Turns out the bug is in part 2 of bug 1037340. Seems we have an old gfxContext. If we pass the gfxContext around instead of relying on the one in the params struct the bug goes away.

Comment 19

[Brian Birtles (:birtles)]

Attachment: Move initialization of text-run parameters until *after* the draw target has been updated

This fixes a regression when painting semi-transparent synthetic bold text. When
we do that we update the draw target but refactoring in bug 1037340 part
2 caused us to store the draw target before it was updated.

Comment 20

[Brian Birtles (:birtles)]

I'm still trying to work out a simple regression test for this. Any ideas appreciated.

Comment 21

[Jonathan Kew (:jfkthame)]

Thanks for the fix!

Seems like you could at least have a != reftest based on the second testcase here, I think?

Comment 22

[Brian Birtles (:birtles)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=67347731e38d

Comment 23

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ecd0e715d948

Comment 24

[Brian Birtles (:birtles)]

(In reply to Jonathan Kew (:jfkthame) from comment #21)
> Thanks for the fix!
> 
> Seems like you could at least have a != reftest based on the second testcase
> here, I think?

Yes, I think so. Thanks!

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ecd0e715d948

Comment 26

[Brian Birtles (:birtles)]

Attachment: part 2 - Add test for text alpha on synthetic bold

Comment 27

[Brian Birtles (:birtles)]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4d0eb1370349
With part 1 reverted: https://treeherder.mozilla.org/#/jobs?repo=try&revision=66a5d4c047f0

Comment 28

[Jonathan Kew (:jfkthame)]

Comment on attachment 8626557 [details] [diff] [review]
part 2 - Add test for text alpha on synthetic bold

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

::: layout/reftests/text/1170688.html
@@ +7,5 @@
> +  font-family: Calibri, Verdana, sans-serif;
> +}
> +@font-face {
> +  font-family: local-arial;
> +  src: local(ArialMT), local(Arial);

I'd suggest using one of the fonts we have in the tree for test purposes, such as

  src: url(../fonts/sil/GenR102.ttf);

so that we can be sure we're using our single-weight family on all platforms.

(The test will also need HTTP(..) in the manifest to enable this.)

Comment 29

[Brian Birtles (:birtles)]

Attachment: part 2 - Add test for text alpha on synthetic bold

Comment 30

[Brian Birtles (:birtles)]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c0a7c15d65d
With part 1 reverted: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8973270c7992

Comment 31

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e7d5ce4c80f

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1e7d5ce4c80f