1318539 - RTL text using colored font is invisible with non-opaque text color

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[corbett.dav]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20161115182233

Steps to reproduce:

1. I downloaded Amiri Quran Colored from https://github.com/khaledhosny/amiri-font/blob/eb309222e715615c872d625943a74c741646d1c7/amiri-quran-colored.ttf. This font uses COLR and CPAL tables to color the dots of Arabic letters red while leaving their skeletons in the text foreground color.
2. I rendered this document: data:text/html;charset=utf-8,<span style="font: 200px Amiri Quran Colored; color:%238888">(%D9%82)</span>


Actual results:

Between two #8888 parentheses was an invisible Arabic letter qaf.


Expected results:

Between two #8888 parentheses should be a #8888 Arabic letter qaf with opaque red dots.

For comparison, if you set the color to #888 or any other opaque color, it is rendered as expected.

Comment 1

[Loic]

Your thoughts, Jonathan?

Comment 2

[Peter Chang[:pchang]]

I can't reproduce this with FF 48 but got problem after FF 50 on mac.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

One possible regression candidate would be:
https://hg.mozilla.org/mozilla-central/rev/e666f640f6434593a6077b4dc24e96bfdf868166
(but I didn't test!).

Comment 4

[Khaled Hosny]

Quite possibly, but I’ve no idea what is going on. If I try to print the layer colors, I get:

r: 0.533333, g: 0.533333, b: 0.533333, a: 1.00000
r: 0.800000, g: 0.200000, b: 0.200000, a: 1.00000

So the first layer is getting the text color (since it has no assigned color in the font) and the second layer is getting the reddish color from the font. Not sure about the alpha values.

Comment 5

[Alice0775 White]

regression window(gray text -> invisible)
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1fecbe3e55c98eca2ba43600fad3db357e3eca60&tochange=d2c74a5856983742da3d22209637ff483ee5e83c

Comment 6

[Loic]

Khaled, could you check this issue after your patch in bug 1283932.

Comment 7

[Gerry Chang [:gchang]]

Track 51+/52+/53+ as regression.

Comment 8

[Jonathan Kew [:jfkthame]]

Hmm, after considerable puzzling about this, I ran across an interesting quirk: the problem seems to be related to the colored Arabic font being RTL. If we override the character properties so that the entire testcase has LTR directionality, suddenly the Qaf appears:

data:text/html;charset=utf-8,<div style="unicode-bidi:bidi-override;"><span style="color:rgba(0,255,0,.5); font: 200px Amiri Quran Colored;">(%D9%82)</span>

I haven't yet identified exactly where things are going wrong, but this may provide a clue...

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Jonathan, any recent insights on this?

Comment 10

[Jonathan Kew [:jfkthame]]

Some additional testcases that may help narrow down the failure...

Similar failure with a color-emoji character (reproduced on macOS, but I expect it would fail similarly on other platforms provided there's a color emoji font):
  data:text/html;charset=utf-8,<div dir=rtl style="opacity:0.5; font:100px serif;">(%F0%9F%98%80)

Removing the dir=rtl attribute makes all the text appear as expected:
  data:text/html;charset=utf-8,<div style="opacity:0.5; font:100px serif;">(%F0%9F%98%80)

Removing the opacity:0.5 property makes all the text appear as expected:
  data:text/html;charset=utf-8,<div dir=rtl style="font:100px serif;">(%F0%9F%98%80)

Adding a strong-LTR character to the content makes all the text appear as expected:
  data:text/html;charset=utf-8,<div dir=rtl style="opacity:0.5; font:100px serif;">(%F0%9F%98%80x)

One key observation here is that it's not merely color-font + partial-opacity that leads to the failure; it is also necessary for the text involved to be RTL (either inherently, as in the Arabic example, or through the use of neutral-direction characters in an RTL context).

Even that isn't quite the whole story; in this example, the emoji (neutral) appear RTL because of the surrounding Arabic letters, yet the text still renders fine, because the surrounding parens are LTR:
  data:text/html;charset=utf-8,<div style="opacity:0.5; font:100px serif;">(%D9%81%F0%9F%98%80%F0%9F%98%81%D9%82)

Remove the parens, and it fails:
  data:text/html;charset=utf-8,<div style="opacity:0.5; font:100px serif;">%D9%81%F0%9F%98%80%F0%9F%98%81%D9%82

Comment 11

[Jonathan Kew [:jfkthame]]

Another note: with the testcases in comment 10, selecting the invisible text (e.g. with Cmd-A) makes it suddenly become visible. However, after deselecting it again, it may vanish as soon as a repaint is triggered (e.g. by resizing the window).

Comment 12

[Jonathan Kew [:jfkthame]]

I have a patch that I think fixes this, just testing a bit before I post it for review.

Comment 13

[Jonathan Kew [:jfkthame]]

Attachment: Reftests for RTL rendering of color glyph with partial opacity (currently failing)

Comment 14

[Jonathan Kew [:jfkthame]]

Attachment: Correct the bounds of the buffer used for drawing colored text with opacity when the run direction is RTL

Comment 15

[Jonathan Kew [:jfkthame]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51a5dd861e090cfc9e813db2e5a6eaea98caf47d

Comment 16

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9b98ddcdbfdae76e5cd45decbc2249e5b7df837a
Bug 1318539 - Reftests for RTL rendering of color glyph with partial opacity (currently failing). r=jrmuizel

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c364b17c93ffb5745d283596bdc788bc028f675
Bug 1318539 - Correct the bounds of the buffer used for drawing colored text with opacity when the run direction is RTL. r=jrmuizel

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9b98ddcdbfda
https://hg.mozilla.org/mozilla-central/rev/1c364b17c93f

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Looks like a pretty small patch with tests included. Please request Aurora/Beta approval on this when you get a chance.

Comment 19

[Jonathan Kew [:jfkthame]]

Comment on attachment 8839404 [details] [diff] [review]
Correct the bounds of the buffer used for drawing colored text with opacity when the run direction is RTL

AFAICS this is not actually a regression from bug 1283932, as suggested earlier;  it has probably existed since color font support was initially implemented. The fix in bug 1283932 made the issue appear with the Amiri Colored font because previously, we didn't handle its color table at all, but this bug existed prior to that and could have been demonstrated with other fonts such as emoji in RTL content.

I don't think this is critical enough to justify a late-beta uplift; it's an extremely specific case that isn't likely to be common in the wild. But it would be good to take on Aurora, given that it is such a simple and small fix.

Approval Request Comment
[Feature/Bug causing the regression]: color font support

[User impact if declined]: RTL text using colored fonts (e.g. emoji) with partial opacity fails to render (is invisible)

[Is this code covered by automated tests?]: yes

[Has the fix been verified in Nightly?]: yes

[Needs manual test from QE? If yes, steps to reproduce]: no

[List of other uplifts needed for the feature/fix]: none

[Is the change risky?]: no

[Why is the change risky/not risky?]: localized change affecting only this particular code path (RTL + color-font or synthetic bold + transparency), so cannot regress any other case

[String changes made/needed]: none

Comment 20

[Julien Cristau [:jcristau]]

wontfix for 52 per comment 19.  Thanks Jonathan.

Comment 21

[Sylvestre Ledru [:Sylvestre]]

Doesn't apply to aurora
warning: conflicts while merging layout/reftests/text/reftest.list!

Comment 22

[Jonathan Kew [:jfkthame]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #21)
> Doesn't apply to aurora
> warning: conflicts while merging layout/reftests/text/reftest.list!

The reftest patch (with tests annotated as "fails") lands first, then the actual bug-fix lands (and removes the annotations).

Comment 23

[Gerry Chang [:gchang]]

Comment on attachment 8839404 [details] [diff] [review]
Correct the bounds of the buffer used for drawing colored text with opacity when the run direction is RTL

Fix an RTL text issue. Aurora53+.

Comment 24

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ad6299b023505915fce271cd9518e85a210cb9cb
https://hg.mozilla.org/releases/mozilla-aurora/rev/6ce37482387bc35b3724620b51c556eaa0312724