Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(13)

Issue 1479003002: Remove Simple Text Path (Closed)

Created:
5 years ago by drott
Modified:
4 years, 1 month ago
CC:
apavlov+blink_chromium.org, behdad, blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, caseq+blink_chromium.org, chromium-reviews, danakj, devtools-reviews_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, eae+blinkwatch, ebrahimgnu, f(malita), fs, gyuyoung2, jbroman, jchaffraix+rendering, Justin Novosad, kojii, kouhei+svg_chromium.org, kozyatinskiy+blink_chromium.org, leviw+renderwatch, lushnikov+blink_chromium.org, pdr+renderingwatchlist_chromium.org, pdr+graphicswatchlist_chromium.org, pdr+svgwatchlist_chromium.org, pfeldman+blink_chromium.org, rwlbuis, Stephen Chennney, sergeyv+blink_chromium.org, szager+layoutwatch_chromium.org, vmpstr+blinkwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove Simple Text Path Concluding a 3 year effort which unified the shaping logic across all platforms, improved the performance of the complex text path through a word cache, further improved the correctness of the complex text path and removed all remaining dependencies on simple text path constructs from SimpleFontData, SVG text and InlineTextBox, fixing tricky memory regressions on Android we are now ready to remove the simple text code path. This means we now only have one code path for text processing removing a class of bugs related to discrepancies between the code paths and get rid of the legacy maintenance burden that was the simple text code path. All our text processing now treats complex scripts as a first class citizen and supports advanced typography at no additional cost. BUG=561099 Committed: https://crrev.com/054a96857c077c00cb8b9c71e0a13a9051caa4f7 Cr-Commit-Position: refs/heads/master@{#427685}

Patch Set 1 #

Patch Set 2 : no default copy constructor needed #

Patch Set 3 : Rebased on top of fmalita's font changes and other dependency fixes #

Patch Set 4 : Rebased on top of 1555013002 #

Patch Set 5 : Rebased on top of codepath choice removal, SVG text and emphasis mark fix #

Patch Set 6 : Rebased, UTF16Iterator offset removed, characterRangeCodePath, TextPath.h removed #

Patch Set 7 : Rebased again #

Patch Set 8 : Rebase #

Patch Set 9 : Remove TextPath.h from gypi file #

Patch Set 10 : Rebased #

Patch Set 11 : Fix CharacterTest #

Patch Set 12 : Rebased #

Patch Set 13 : Rebased #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -2420 lines) Patch
M third_party/WebKit/Source/core/inspector/InspectorCSSAgent.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutText.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/LayoutText.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -4 lines 0 comments Download
M third_party/WebKit/Source/core/layout/api/LineLayoutText.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +0 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/layout/line/InlineTextBox.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +0 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/Font.h View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +0 lines, -22 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/Font.cpp View 1 2 3 4 5 6 7 8 9 10 11 7 chunks +10 lines, -375 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/GlyphBufferTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -8 lines 0 comments Download
D third_party/WebKit/Source/platform/fonts/GlyphPage.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -232 lines 0 comments Download
D third_party/WebKit/Source/platform/fonts/GlyphPageTreeNode.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -219 lines 0 comments Download
D third_party/WebKit/Source/platform/fonts/GlyphPageTreeNode.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -485 lines 0 comments Download
D third_party/WebKit/Source/platform/fonts/GlyphPageTreeNodeTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +0 lines, -290 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/SegmentedFontData.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +3 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/SegmentedFontData.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/SimpleFontData.h View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +11 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/SimpleFontData.cpp View 1 2 3 4 5 6 7 8 9 10 11 8 chunks +6 lines, -59 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/UTF16TextIterator.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/UTF16TextIterator.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -11 lines 0 comments Download
M third_party/WebKit/Source/platform/fonts/opentype/OpenTypeVerticalData.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaperTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/fonts/shaping/Shaper.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -1 line 0 comments Download
D third_party/WebKit/Source/platform/fonts/shaping/SimpleShaper.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -81 lines 0 comments Download
D third_party/WebKit/Source/platform/fonts/shaping/SimpleShaper.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -254 lines 0 comments Download
M third_party/WebKit/Source/platform/text/Character.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +0 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/platform/text/Character.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -136 lines 0 comments Download
M third_party/WebKit/Source/platform/text/CharacterTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +0 lines, -143 lines 0 comments Download
D third_party/WebKit/Source/platform/text/TextPath.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +0 lines, -39 lines 0 comments Download
M third_party/WebKit/Source/platform/text/TextRun.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -2 lines 0 comments Download
M third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +0 lines, -8 lines 0 comments Download

Messages

Total messages: 96 (46 generated)
drott
no default copy constructor needed
5 years ago (2015-11-26 13:38:18 UTC) #2
drott
Rebased on top of fmalita's font changes and other dependency fixes
4 years, 11 months ago (2016-01-04 08:09:40 UTC) #3
drott
This is ready for review now, though it contains some redundancies and needs to be ...
4 years, 11 months ago (2016-01-04 09:09:34 UTC) #6
eae
This is awesome but ideally we'd wait until the next branch point (late next week) ...
4 years, 11 months ago (2016-01-04 18:03:45 UTC) #7
drott
On 2016/01/04 18:03:45, eae wrote: > This is awesome but ideally we'd wait until the ...
4 years, 11 months ago (2016-01-04 18:19:35 UTC) #8
eae
Thank you. I don't expect much to change in the platform or line-layout code before ...
4 years, 11 months ago (2016-01-04 18:20:30 UTC) #9
drott
Rebased on top of codepath choice removal, SVG text and emphasis mark fix
4 years, 11 months ago (2016-01-05 08:52:03 UTC) #10
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1479003002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1479003002/80001
4 years, 11 months ago (2016-01-05 14:20:04 UTC) #12
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 11 months ago (2016-01-05 15:50:38 UTC) #14
eae
Awesome! LGTM but please wait until branch point as discussed.
4 years, 11 months ago (2016-01-05 16:22:19 UTC) #15
behdad
lgtm Amazing!
4 years, 11 months ago (2016-01-05 16:27:46 UTC) #17
drott
TODO: Remove TextPath.h and characterRangeCodePath functions in Character.h
4 years, 11 months ago (2016-01-08 13:45:39 UTC) #18
drott
TODO, remove offset/endoffset from UTF16TextIterator
4 years, 11 months ago (2016-01-21 13:54:32 UTC) #19
drott
TODO, remove TypeSettingFeatures
4 years, 7 months ago (2016-05-10 11:16:02 UTC) #20
drott
Rebased, UTF16Iterator offset removed, characterRangeCodePath, TextPath.h removed
4 years, 5 months ago (2016-07-01 16:11:43 UTC) #22
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1479003002/100001
4 years, 5 months ago (2016-07-01 16:11:51 UTC) #23
drott
$ git diff origin/master.. --stat [...] 30 files changed, 35 insertions(+), 2226 deletions(-) ;-)
4 years, 5 months ago (2016-07-01 16:12:35 UTC) #24
eae
Awesome!
4 years, 5 months ago (2016-07-01 16:14:33 UTC) #25
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/238166)
4 years, 5 months ago (2016-07-01 16:15:35 UTC) #27
drott
Rebased again
4 years, 5 months ago (2016-07-01 16:22:36 UTC) #29
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1479003002/120001
4 years, 5 months ago (2016-07-01 16:22:48 UTC) #30
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/97428)
4 years, 5 months ago (2016-07-01 16:26:10 UTC) #32
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1479003002/120001
4 years, 5 months ago (2016-07-01 16:51:33 UTC) #34
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/97435) mac_chromium_compile_dbg_ng on ...
4 years, 5 months ago (2016-07-01 16:54:53 UTC) #36
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1479003002/120001
4 years, 5 months ago (2016-07-04 07:58:05 UTC) #38
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm64_dbg_recipe/builds/90994) linux_android_rel_ng on ...
4 years, 5 months ago (2016-07-04 08:01:28 UTC) #40
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1479003002/120001
4 years, 5 months ago (2016-07-04 13:09:46 UTC) #42
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linux/builds/185753) chromeos_amd64-generic_chromium_compile_only_ng on ...
4 years, 5 months ago (2016-07-04 13:12:39 UTC) #44
drott
Rebase
4 years, 5 months ago (2016-07-04 13:41:45 UTC) #46
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1479003002/140001
4 years, 5 months ago (2016-07-04 13:41:57 UTC) #47
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/98150)
4 years, 5 months ago (2016-07-04 13:45:37 UTC) #49
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1479003002/160001
4 years, 5 months ago (2016-07-04 13:50:14 UTC) #51
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 5 months ago (2016-07-04 15:11:39 UTC) #53
drott
Rebased
4 years, 3 months ago (2016-08-29 08:05:12 UTC) #54
drott
Fix CharacterTest
4 years, 3 months ago (2016-08-29 08:34:06 UTC) #59
eae
Nuke it from orbit, it's the only way to be sure.
4 years, 3 months ago (2016-09-20 08:07:13 UTC) #64
drott
On 2016/09/20 at 08:07:13, eae wrote: > Nuke it from orbit, it's the only way ...
4 years, 3 months ago (2016-09-21 08:08:31 UTC) #65
eae
We do.
4 years, 3 months ago (2016-09-21 09:08:06 UTC) #66
drott
Rebased
4 years, 2 months ago (2016-10-18 12:21:35 UTC) #67
eae
M54 has hit stable, might be time to rebase it again :)
4 years, 1 month ago (2016-10-25 17:51:05 UTC) #74
drott
Rebased
4 years, 1 month ago (2016-10-26 01:18:24 UTC) #75
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1479003002/240001
4 years, 1 month ago (2016-10-26 01:24:37 UTC) #79
eae
On 2016/10/26 01:24:37, commit-bot: I haz the power wrote: > CQ is trying da patch. ...
4 years, 1 month ago (2016-10-26 01:28:22 UTC) #80
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/289460)
4 years, 1 month ago (2016-10-26 01:34:00 UTC) #82
drott
bokan@, dcheng@, eae@, jochen@, pdr@ - could you please review third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp to unblock landing this? ...
4 years, 1 month ago (2016-10-26 14:00:55 UTC) #89
bokan
FrameLoaderClientImpl lgtm
4 years, 1 month ago (2016-10-26 14:12:00 UTC) #90
drott
On 2016/10/26 at 14:12:00, bokan wrote: > FrameLoaderClientImpl lgtm Thanks for the quick review.
4 years, 1 month ago (2016-10-26 14:17:33 UTC) #92
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1479003002/240001
4 years, 1 month ago (2016-10-26 14:17:36 UTC) #93
commit-bot: I haz the power
Committed patchset #13 (id:240001)
4 years, 1 month ago (2016-10-26 14:23:09 UTC) #94
commit-bot: I haz the power
4 years, 1 month ago (2016-10-26 14:25:57 UTC) #96
Message was sent while issue was closed.
Patchset 13 (id:??) landed as
https://crrev.com/054a96857c077c00cb8b9c71e0a13a9051caa4f7
Cr-Commit-Position: refs/heads/master@{#427685}

Powered by Google App Engine
This is Rietveld 408576698