New issue
Advanced search Search tips

Issue 881110 link

Starred by 1 user
Status: Fixed
Owner:
e...@chromium.org
Closed: Dec 12
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Have all clients of ShapeResult use public API

Project Member Reported by e...@chromium.org, Sep 5 
Currently a number of clients, most notably ShapingLineBreaker and ShapeResultBloberizer, depend on internal state which makes it tricky to make changes to how ShapeResults are stored, shared, and accessed.
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 12 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9185a5f2e79084fef5ce3a737df69efa87ab975a

commit 9185a5f2e79084fef5ce3a737df69efa87ab975a
Author: Emil A Eklund <eae@chromium.org>
Date: Wed Sep 12 03:52:35 2018

Replace internal method with ShapeResult::ForEachGlyph API

ShapeResultBloberizer relied on an internal ShapeResult ForEachGlyph API
that exposed a lot of internal implementation details of ShapeResult and
that used a lambda expression making it tricky to use across compilation
units. This change removes the RunInfo::ForEachGlyph/ForEachGlyphInRange
methods and replaces them with public two ForEachGlyph methods, one full
and one taking a range, on ShapeResult. Both use a well-defined callback
function and invokes it for each glyph without exposing class internals.

Also changes the only caller, ShapeResultBloberizer, to use the new API.

This is a part of a series of changes aiming to provide an API for shape
data which in turn will allow us to change the internal representation.

Bug:  881110 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I675802eb3fd2bb9a6b54f727a1d9b1a587f69882
Reviewed-on: https://chromium-review.googlesource.com/1208728
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590592}
[modify] https://crrev.com/9185a5f2e79084fef5ce3a737df69efa87ab975a/third_party/blink/renderer/platform/fonts/shaping/shape_result.cc
[modify] https://crrev.com/9185a5f2e79084fef5ce3a737df69efa87ab975a/third_party/blink/renderer/platform/fonts/shaping/shape_result.h
[modify] https://crrev.com/9185a5f2e79084fef5ce3a737df69efa87ab975a/third_party/blink/renderer/platform/fonts/shaping/shape_result_bloberizer.cc
[modify] https://crrev.com/9185a5f2e79084fef5ce3a737df69efa87ab975a/third_party/blink/renderer/platform/fonts/shaping/shape_result_inline_headers.h
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 14 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ef8b65de690c3a2d83bd6b991763359a7c1f3d6

commit 2ef8b65de690c3a2d83bd6b991763359a7c1f3d6
Author: Emil A Eklund <eae@chromium.org>
Date: Fri Sep 14 02:18:27 2018

Add ShapeResult::ForEachGraphemeClusters API

Replace the use of internal ShapeResult notions in ShapeResultBloberizer
with the new ShapeResult::ForEachGraphemeClusters API, modeled after the
existing ForEachGlyph API.

Also fixes emphasis mark painting for LayoutNG and rebaselines the tests
for emphasis painting, some of which had incorrect expectations.

Bug:  881110 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ic147f68d98453fa32d1527b090bc1915f7af8ac1
Reviewed-on: https://chromium-review.googlesource.com/1225101
Reviewed-by: Koji Ishii <kojii@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591266}
[modify] https://crrev.com/2ef8b65de690c3a2d83bd6b991763359a7c1f3d6/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/text/emphasis-complex-expected.png
[modify] https://crrev.com/2ef8b65de690c3a2d83bd6b991763359a7c1f3d6/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/text/emphasis-ellipsis-complextext-expected.png
[modify] https://crrev.com/2ef8b65de690c3a2d83bd6b991763359a7c1f3d6/third_party/WebKit/LayoutTests/platform/linux/fast/text/emphasis-complex-expected.png
[modify] https://crrev.com/2ef8b65de690c3a2d83bd6b991763359a7c1f3d6/third_party/blink/renderer/platform/fonts/font.cc
[modify] https://crrev.com/2ef8b65de690c3a2d83bd6b991763359a7c1f3d6/third_party/blink/renderer/platform/fonts/shaping/shape_result.cc
[modify] https://crrev.com/2ef8b65de690c3a2d83bd6b991763359a7c1f3d6/third_party/blink/renderer/platform/fonts/shaping/shape_result.h
[modify] https://crrev.com/2ef8b65de690c3a2d83bd6b991763359a7c1f3d6/third_party/blink/renderer/platform/fonts/shaping/shape_result_bloberizer.cc
[modify] https://crrev.com/2ef8b65de690c3a2d83bd6b991763359a7c1f3d6/third_party/blink/renderer/platform/fonts/shaping/shape_result_bloberizer.h
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Oct 9 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/178dd7b4e40000
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Oct 9 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/16845d02e40000
Project Member

Comment 12 by 42576172...@developer.gserviceaccount.com, Oct 9 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/10a177c2e40000
Project Member

Comment 15 by 42576172...@developer.gserviceaccount.com, Oct 9 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/141c225ae40000
Project Member

Comment 16 by 42576172...@developer.gserviceaccount.com, Oct 9 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/16d67b8ce40000
Project Member

Comment 17 by 42576172...@developer.gserviceaccount.com, Oct 9 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/11bf5f9ae40000
Project Member

Comment 18 by 42576172...@developer.gserviceaccount.com, Oct 9 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/121c225ae40000
Project Member

Comment 21 by 42576172...@developer.gserviceaccount.com, Oct 10 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/168ce112e40000
Project Member

Comment 22 by 42576172...@developer.gserviceaccount.com, Oct 10 

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/11c1164ae40000

Comment 23 by e...@chromium.org, Dec 12

Status: Fixed (was: Assigned)
►

Sign in to add a comment