Introduce support for text-decoration-skip: ink

Introduce support for text-decoration-skip: ink

In addition to the "objects" default value, support value "ink" for
text-decoration-skip. The spec [1] describes this as:

    "Skip over where glyphs are drawn: interrupt the decoration line to
    let the shape of the text show through where the text decoration
    would otherwise cross over a glyph. The UA must skip a small
    distance to either side of the glyph outline."

This CL supports this for all underline, overline, and line-through and
all text-decoration-style styles: solid, double, dashed, dotted, wavy.

The implementation approach is: If ink skipping is requested, clip
rectangles are calculated for the text intercepts. The vertical bounds
for those clip rectangles are computed from the extends of the underline
style: For example tight bounds of a Bezier curve are calculated for the
wavy underline style. Using these vertical boundaries, the text
intercepts are retrieved from Skia, via GraphicsContext. Skia computes
them from the glyph shapes and two distances, the vertical bounds that
were previously calculated. The rectangles are inflated in the baseline
direction by the underline thickness itself, same as WebKit. These clip
rectangles are then used to clip out painting an underline at the
calculated positions.

Known issue at this point: Due to the way we draw upright in vertical
text, currently there is no correct way of computing the ink skip
rectangles in this case, tracked in crbug.com/655154

[1] https://drafts.csswg.org/css-text-decor-3/#text-decoration-skip-property

BUG=649700, 581456
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/c0fd99aa9f08fd23c5cf35e4f29b2e650fcace92
Cr-Commit-Position: refs/heads/master@{#433415}

[drott, 2016-10-13 18:09:25]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[drott, 2016-10-13 18:09:26]

Description was changed from

==========
Introduce support for text-decoration-skip: ink

In addition to the "objects" default value, support value "ink" for
text-decoration-skip. The spec [1] describes this as:

    "Skip over where glyphs are drawn: interrupt the decoration line to
    let the shape of the text show through where the text decoration
    would otherwise cross over a glyph. The UA must skip a small
    distance to either side of the glyph outline."

This CL supports this for all underline, overline, and line-through and
all text-decoration-style styles: solid, double, dashed, dotted, wavy.

The implementation approach is: If ink skipping is requested, clip
rectangles are calculated for the text intercepts. The vertical bounds
for those clip rectangles are computed from the extends of the underline
style: For example tight bounds of a Bezier curve are calculated for the
wavy underline style. Using these vertical boundaries, the text
intercepts are retrieved from Skia, via GraphicsContext. Skia computes
them from the glyph shapes and two distances, the vertical bounds that
were previously calculated. The rectangles are inflated in the baseline
direction by the underline thickness itself, same as WebKit. These clip
rectangles are then used to clip out painting an underline at the
calculated positions.

Known issue at this point: Due to the way we draw upright in vertical
text, currently there is no correct way of computing the ink skip
rectangles in this case, tracked in crbug.com/655154

[1] https://drafts.csswg.org/css-text-decor-3/#text-decoration-skip-property

BUG=649700, 581456
==========

to

==========
Introduce support for text-decoration-skip: ink

In addition to the "objects" default value, support value "ink" for
text-decoration-skip. The spec [1] describes this as:

    "Skip over where glyphs are drawn: interrupt the decoration line to
    let the shape of the text show through where the text decoration
    would otherwise cross over a glyph. The UA must skip a small
    distance to either side of the glyph outline."

This CL supports this for all underline, overline, and line-through and
all text-decoration-style styles: solid, double, dashed, dotted, wavy.

The implementation approach is: If ink skipping is requested, clip
rectangles are calculated for the text intercepts. The vertical bounds
for those clip rectangles are computed from the extends of the underline
style: For example tight bounds of a Bezier curve are calculated for the
wavy underline style. Using these vertical boundaries, the text
intercepts are retrieved from Skia, via GraphicsContext. Skia computes
them from the glyph shapes and two distances, the vertical bounds that
were previously calculated. The rectangles are inflated in the baseline
direction by the underline thickness itself, same as WebKit. These clip
rectangles are then used to clip out painting an underline at the
calculated positions.

Known issue at this point: Due to the way we draw upright in vertical
text, currently there is no correct way of computing the ink skip
rectangles in this case, tracked in crbug.com/655154

[1] https://drafts.csswg.org/css-text-decor-3/#text-decoration-skip-property

BUG=649700, 581456
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[commit-bot: I haz the power, 2016-10-13 18:10:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[drott, 2016-10-13 18:18:40]

Reduce font size in test

[drott, 2016-10-13 18:18:41]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[eae, 2016-10-13 18:18:53]

eae@chromium.org changed reviewers:
+ eae@chromium.org

[eae, 2016-10-13 18:18:54]

This looks great!

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp (right):

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp:29: // pixel gap,
if underline is thick then use a bigger gap.
Is this CSS or device pixels? If CSS pixel we probably want to use a LayoutUnit
to allow for non-integer device-pixel-ratios.

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp:36: // Blink
odd line-wrapping.

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp:44: // FIXME: We
support only horizontal text for now.
Is this still accurate? The test case includes vertical text.

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp:276: // pixels
distance since
Odd line wrapping again here and below on line 280.

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/TextPainter.cpp (right):

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/TextPainter.cpp:270: for (int i = 0; i <
numIntervals; i += 2) {
Do we want to assert that the number of intervals is even?

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/TextPainter.cpp:276:
m_graphicsContext.setFillColor(Color(127, 0, 0, 127));
Hard coded color? This is temporary, right?

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/TextPainter.cpp:278:
paintClipRect.inflateX(clipRect.width() * -0.25);
Please add a comment explaining the 0.25 value.

[commit-bot: I haz the power, 2016-10-13 18:19:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[drott, 2016-10-13 18:35:42]

Adressing some of Emil's review comments

[drott, 2016-10-13 18:35:44]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 18:36:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[drott, 2016-10-13 18:39:29]

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp (right):

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp:44: // FIXME: We
support only horizontal text for now.
On 2016/10/13 at 18:18:54, eae wrote:
> Is this still accurate? The test case includes vertical text.

All good questions and remarks... This code is old, and I just moved it, see
below. I'll try to find answers.

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/TextPainter.cpp (right):

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/TextPainter.cpp:270: for (int i = 0; i <
numIntervals; i += 2) {
On 2016/10/13 at 18:18:54, eae wrote:
> Do we want to assert that the number of intervals is even?

Makes sense, adding an assert.

https://codereview.chromium.org/2416993002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/TextPainter.cpp:276:
m_graphicsContext.setFillColor(Color(127, 0, 0, 127));
On 2016/10/13 at 18:18:54, eae wrote:
> Hard coded color? This is temporary, right?

Yep, the whole paintRect can go, and no color setting is needed, this was debug
code.

[eae, 2016-10-13 18:52:54]

LGTM!

[drott, 2016-10-13 18:54:34]

Remove sketch comments

[drott, 2016-10-13 18:54:35]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-13 18:54:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-13 19:26:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-13 19:26:31]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[drott, 2016-11-16 14:30:30]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 14:30:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-16 14:30:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 14:30:55]

Dry run: CLs for remote refs other than refs/pending/heads/master must contain
NOTRY=true and NOPRESUBMIT=true in order for the CQ to process them

[drott, 2016-11-16 14:39:45]

Model dependency to multiple decorations painting

[drott, 2016-11-16 14:39:48]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-16 14:40:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[drott, 2016-11-16 14:43:25]

On 2016/10/13 at 18:52:54, eae wrote:
> LGTM!

Could you take another look, only TextPainter really changed after I analyzed
why I saw artifacts in vertical. I believe a dilation in Y direction is a
reasonable solution for the clipping rectangle, I added a comment for
explanations.

[commit-bot: I haz the power, 2016-11-16 16:17:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-16 16:17:43]

Dry run: Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_layout_te...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[eae, 2016-11-16 16:32:28]

OK, LGTM

[drott, 2016-11-18 16:43:04]

Fix wavy offset

[drott, 2016-11-18 16:43:33]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-18 16:44:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 18:34:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 18:34:15]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[drott, 2016-11-18 20:43:09]

Perhaps generic test expectations are enough?

[drott, 2016-11-18 20:43:15]

The CQ bit was checked by drott@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-18 20:44:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[drott, 2016-11-18 20:46:35]

Falling back to adding rebaseline expectation

[drott, 2016-11-18 20:46:42]

The CQ bit was checked by drott@chromium.org

[drott, 2016-11-18 20:46:43]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2416993002/#ps160001
(title: "Falling back to adding rebaseline expectation")

[commit-bot: I haz the power, 2016-11-18 20:47:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 22:50:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 22:50:09]

Try jobs failed on following builders:
  linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[drott, 2016-11-19 08:49:12]

The CQ bit was checked by drott@chromium.org

[commit-bot: I haz the power, 2016-11-19 08:49:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-19 08:54:50]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-11-19 09:02:28]

Description was changed from

==========
Introduce support for text-decoration-skip: ink

In addition to the "objects" default value, support value "ink" for
text-decoration-skip. The spec [1] describes this as:

    "Skip over where glyphs are drawn: interrupt the decoration line to
    let the shape of the text show through where the text decoration
    would otherwise cross over a glyph. The UA must skip a small
    distance to either side of the glyph outline."

This CL supports this for all underline, overline, and line-through and
all text-decoration-style styles: solid, double, dashed, dotted, wavy.

The implementation approach is: If ink skipping is requested, clip
rectangles are calculated for the text intercepts. The vertical bounds
for those clip rectangles are computed from the extends of the underline
style: For example tight bounds of a Bezier curve are calculated for the
wavy underline style. Using these vertical boundaries, the text
intercepts are retrieved from Skia, via GraphicsContext. Skia computes
them from the glyph shapes and two distances, the vertical bounds that
were previously calculated. The rectangles are inflated in the baseline
direction by the underline thickness itself, same as WebKit. These clip
rectangles are then used to clip out painting an underline at the
calculated positions.

Known issue at this point: Due to the way we draw upright in vertical
text, currently there is no correct way of computing the ink skip
rectangles in this case, tracked in crbug.com/655154

[1] https://drafts.csswg.org/css-text-decor-3/#text-decoration-skip-property

BUG=649700, 581456
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Introduce support for text-decoration-skip: ink

In addition to the "objects" default value, support value "ink" for
text-decoration-skip. The spec [1] describes this as:

    "Skip over where glyphs are drawn: interrupt the decoration line to
    let the shape of the text show through where the text decoration
    would otherwise cross over a glyph. The UA must skip a small
    distance to either side of the glyph outline."

This CL supports this for all underline, overline, and line-through and
all text-decoration-style styles: solid, double, dashed, dotted, wavy.

The implementation approach is: If ink skipping is requested, clip
rectangles are calculated for the text intercepts. The vertical bounds
for those clip rectangles are computed from the extends of the underline
style: For example tight bounds of a Bezier curve are calculated for the
wavy underline style. Using these vertical boundaries, the text
intercepts are retrieved from Skia, via GraphicsContext. Skia computes
them from the glyph shapes and two distances, the vertical bounds that
were previously calculated. The rectangles are inflated in the baseline
direction by the underline thickness itself, same as WebKit. These clip
rectangles are then used to clip out painting an underline at the
calculated positions.

Known issue at this point: Due to the way we draw upright in vertical
text, currently there is no correct way of computing the ink skip
rectangles in this case, tracked in crbug.com/655154

[1] https://drafts.csswg.org/css-text-decor-3/#text-decoration-skip-property

BUG=649700, 581456
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/c0fd99aa9f08fd23c5cf35e4f29b2e650fcace92
Cr-Commit-Position: refs/heads/master@{#433415}
==========

[commit-bot: I haz the power, 2016-11-19 09:02:30]

Patchset 9 (id:??) landed as
https://crrev.com/c0fd99aa9f08fd23c5cf35e4f29b2e650fcace92
Cr-Commit-Position: refs/heads/master@{#433415}