1287705 - background-clip:text interacts poorly with opacity (and blur) in text-shadow

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Jonathan Kew [:jfkthame]]

Attachment: testcase combining text-shadow and background-clip:text

See testcase. The rendering of the text-shadow's blur (wherever its opacity is < 1.0) is spoiled by interaction with the background color, which should have been restricted to appearing within the clip path of the text.

Comment 1

[Jonathan Kew [:jfkthame]]

CJ, I guess this is connected to the implementation of background-clip from bug 1269971. Do you have time to look into it?

Comment 2

[u459114]

Attachment: Bug 1287705 - Part 1. Do not paint text shadow while generating text mask.

Review commit: https://reviewboard.mozilla.org/r/65236/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65236/

Comment 3

[u459114]

Attachment: Bug 1287705 - Part 2. Add more comments and construct PaintState in PaintTextParams.

Review commit: https://reviewboard.mozilla.org/r/65238/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65238/

Comment 4

[u459114]

Attachment: Bug 1287705 - Part 3. reftest for text shadow.

Review commit: https://reviewboard.mozilla.org/r/65240/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65240/

Comment 5

[u459114]

Comment on attachment 8772420 [details]
Bug 1287705 - Part 3. reftest for text shadow.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65240/diff/1-2/

Comment 6

[Jonathan Kew [:jfkthame]]

Comment on attachment 8772418 [details]
Bug 1287705 - Part 1. Do not paint text shadow while generating text mask.

https://reviewboard.mozilla.org/r/65236/#review62300

Comment 7

[Jonathan Kew [:jfkthame]]

Comment on attachment 8772419 [details]
Bug 1287705 - Part 2. Add more comments and construct PaintState in PaintTextParams.

https://reviewboard.mozilla.org/r/65238/#review62302

::: layout/generic/nsTextFrame.cpp:6014
(Diff revision 1)
>      aParams.textBaselinePt.x - aParams.framePt.x;
>    gfxFloat iOffset, hyphenWidth;
>    Range range; // in transformed string
>    TextRangeStyle rangeStyle;
>    // Draw background colors
> -  if (anyBackgrounds && (!aParams.generateTextMask ||
> +  if (anyBackgrounds && (!aParams.IsGenerateTextMask())) {

You can drop the parens around !aParams.IsGenerateTextMask() here, now that it's not a two-term || expression.

Comment 8

[Jonathan Kew [:jfkthame]]

Comment on attachment 8772420 [details]
Bug 1287705 - Part 3. reftest for text shadow.

https://reviewboard.mozilla.org/r/65240/#review62304

LGTM. Thanks for the instant fix here! :)

Comment 9

[u459114]

Comment on attachment 8772418 [details]
Bug 1287705 - Part 1. Do not paint text shadow while generating text mask.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65236/diff/1-2/

Comment 10

[u459114]

Comment on attachment 8772419 [details]
Bug 1287705 - Part 2. Add more comments and construct PaintState in PaintTextParams.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65238/diff/1-2/

Comment 11

[u459114]

Comment on attachment 8772420 [details]
Bug 1287705 - Part 3. reftest for text shadow.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65240/diff/2-3/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14bb443d532d
Part 1. Do not paint text shadow while generating text mask. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/0f248b2827af
Part 2. Add more comments and construct PaintState in PaintTextParams. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/9be24f375ba6
Part 3. reftest for text shadow. r=jfkthame

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/14bb443d532d
https://hg.mozilla.org/mozilla-central/rev/0f248b2827af
https://hg.mozilla.org/mozilla-central/rev/9be24f375ba6