Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Teach TextBreakIterator about strict and loose line breaking behaviors #14162

Conversation

litherum
Copy link
Contributor

@litherum litherum commented May 21, 2023

99eac46

Teach TextBreakIterator about strict and loose line breaking behaviors
https://bugs.webkit.org/show_bug.cgi?id=257108
rdar://109634206

Reviewed by Cameron McCormack.

This is the first part of our line breaker refactoring. We currently have 2 totally separate classes,
both of which can perform line breaking: TextBreakIterator and LazyLineBreakIterator.

TextBreakIterator has multiple backends - it can be backed by either ICU or Core Foundation. It also
supports all kinds of text segmentation: caret positions, grapheme cluster segmentation, etc.

LazyLineBreakIterator only works with ICU, and can only do line breaking. However, it supports loose
and strict line breaking modes, and supports a "prior context."

It's kind of a shame that we have 2 classes which both do similar things, so I'm going to try to
unify them into a single class which can do everything. I'd like to improve TextBreakIterator to be
able to do everything that LazyLineBreakIterator can do, and then delete LazyLineBreakIterator,
because I think that's going to be the most straightforward way of doing it.

This patch teaches TextBreakIterator about the different line breaking behaviors, by turning the
"mode" enum into a variant, and giving the Line struct a behavior enum.

* Source/WTF/wtf/text/TextBreakIterator.cpp:
(WTF::mapModeToBackingIterator):
* Source/WTF/wtf/text/TextBreakIterator.h:
(WTF::TextBreakIterator::LineMode::operator== const):
(WTF::TextBreakIterator::CaretMode::operator== const):
(WTF::TextBreakIterator::DeleteMode::operator== const):
(WTF::TextBreakIterator::CharacterMode::operator== const):
(WTF::LazyLineBreakIterator::get):
(WTF::TextBreakIteratorCache::TextBreakIteratorCache): Deleted.
* Source/WTF/wtf/text/WTFString.h:
(WTF::StringLiterals::operator _str):
* Source/WTF/wtf/text/cocoa/TextBreakIteratorInternalICUCocoa.cpp:
(WTF::mapModeToBackingIterator):
* Source/WTF/wtf/text/icu/TextBreakIteratorICU.h:
(WTF::TextBreakIteratorICU::TextBreakIteratorICU):
(WTF::TextBreakIteratorICU::makeLocaleWithBreakKeyword):
* Source/WebCore/platform/graphics/ComplexTextController.cpp:
(WebCore::ComplexTextController::offsetForPosition):
(WebCore::ComplexTextController::collectComplexTextRuns):
* Source/WebCore/platform/graphics/ComposedCharacterClusterTextIterator.h:
(WebCore::ComposedCharacterClusterTextIterator::ComposedCharacterClusterTextIterator):
* Source/WebCore/rendering/RenderText.cpp:
(WebCore::RenderText::previousOffset const):
(WebCore::RenderText::previousOffsetForBackwardDeletion const):
(WebCore::RenderText::nextOffset const):
* Tools/TestWebKitAPI/Tests/WTF/TextBreakIterator.cpp:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/264376@main

1d81799

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe   πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2   πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt   πŸ§ͺ mac-wk1   πŸ§ͺ gtk-wk2
βœ… πŸ›  πŸ§ͺ jsc   πŸ§ͺ api-ios   πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ jsc-arm64 βœ… πŸ›  tv   πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ›  jsc-armv7
βœ… πŸ›  tv-sim βœ… πŸ§ͺ jsc-armv7-tests
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch βœ… πŸ›  jsc-mips
βœ… πŸ›  watch-sim βœ… πŸ§ͺ jsc-mips-tests

@litherum litherum self-assigned this May 21, 2023
@litherum litherum added the Text For bugs in text layout and rendering, including international text support. label May 21, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 21, 2023
@litherum litherum removed the merging-blocked Applied to prevent a change from being merged label May 21, 2023
@litherum litherum force-pushed the eng/Teach-TextBreakIterator-about-strict-and-loose-line-breaking-behaviors branch from dcbcad6 to e8e59a6 Compare May 21, 2023 23:25
@litherum litherum force-pushed the eng/Teach-TextBreakIterator-about-strict-and-loose-line-breaking-behaviors branch 2 times, most recently from 966b452 to 78a497a Compare May 21, 2023 23:27
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 21, 2023
@litherum litherum removed the merging-blocked Applied to prevent a change from being merged label May 21, 2023
@litherum litherum force-pushed the eng/Teach-TextBreakIterator-about-strict-and-loose-line-breaking-behaviors branch from 78a497a to 006a077 Compare May 21, 2023 23:33
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 21, 2023
@litherum litherum removed the merging-blocked Applied to prevent a change from being merged label May 21, 2023
@litherum litherum force-pushed the eng/Teach-TextBreakIterator-about-strict-and-loose-line-breaking-behaviors branch from 006a077 to c13879d Compare May 21, 2023 23:41
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 22, 2023
Copy link
Contributor

@heycam heycam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the commit message:

LazyLineBreakIterator only work with ICU, and only can do line breaking.

s/only work/only works/
s/only can do/can only do/

Source/WTF/wtf/text/TextBreakIterator.h Outdated Show resolved Hide resolved
Source/WTF/wtf/text/TextBreakIterator.h Show resolved Hide resolved
Source/WTF/wtf/text/WTFString.h Show resolved Hide resolved
Tools/TestWebKitAPI/Tests/WTF/TextBreakIterator.cpp Outdated Show resolved Hide resolved
@litherum litherum removed the merging-blocked Applied to prevent a change from being merged label May 22, 2023
@litherum litherum force-pushed the eng/Teach-TextBreakIterator-about-strict-and-loose-line-breaking-behaviors branch from c13879d to 1d81799 Compare May 22, 2023 19:54
@litherum litherum added the merge-queue Applied to send a pull request to merge-queue label May 22, 2023
https://bugs.webkit.org/show_bug.cgi?id=257108
rdar://109634206

Reviewed by Cameron McCormack.

This is the first part of our line breaker refactoring. We currently have 2 totally separate classes,
both of which can perform line breaking: TextBreakIterator and LazyLineBreakIterator.

TextBreakIterator has multiple backends - it can be backed by either ICU or Core Foundation. It also
supports all kinds of text segmentation: caret positions, grapheme cluster segmentation, etc.

LazyLineBreakIterator only works with ICU, and can only do line breaking. However, it supports loose
and strict line breaking modes, and supports a "prior context."

It's kind of a shame that we have 2 classes which both do similar things, so I'm going to try to
unify them into a single class which can do everything. I'd like to improve TextBreakIterator to be
able to do everything that LazyLineBreakIterator can do, and then delete LazyLineBreakIterator,
because I think that's going to be the most straightforward way of doing it.

This patch teaches TextBreakIterator about the different line breaking behaviors, by turning the
"mode" enum into a variant, and giving the Line struct a behavior enum.

* Source/WTF/wtf/text/TextBreakIterator.cpp:
(WTF::mapModeToBackingIterator):
* Source/WTF/wtf/text/TextBreakIterator.h:
(WTF::TextBreakIterator::LineMode::operator== const):
(WTF::TextBreakIterator::CaretMode::operator== const):
(WTF::TextBreakIterator::DeleteMode::operator== const):
(WTF::TextBreakIterator::CharacterMode::operator== const):
(WTF::LazyLineBreakIterator::get):
(WTF::TextBreakIteratorCache::TextBreakIteratorCache): Deleted.
* Source/WTF/wtf/text/WTFString.h:
(WTF::StringLiterals::operator _str):
* Source/WTF/wtf/text/cocoa/TextBreakIteratorInternalICUCocoa.cpp:
(WTF::mapModeToBackingIterator):
* Source/WTF/wtf/text/icu/TextBreakIteratorICU.h:
(WTF::TextBreakIteratorICU::TextBreakIteratorICU):
(WTF::TextBreakIteratorICU::makeLocaleWithBreakKeyword):
* Source/WebCore/platform/graphics/ComplexTextController.cpp:
(WebCore::ComplexTextController::offsetForPosition):
(WebCore::ComplexTextController::collectComplexTextRuns):
* Source/WebCore/platform/graphics/ComposedCharacterClusterTextIterator.h:
(WebCore::ComposedCharacterClusterTextIterator::ComposedCharacterClusterTextIterator):
* Source/WebCore/rendering/RenderText.cpp:
(WebCore::RenderText::previousOffset const):
(WebCore::RenderText::previousOffsetForBackwardDeletion const):
(WebCore::RenderText::nextOffset const):
* Tools/TestWebKitAPI/Tests/WTF/TextBreakIterator.cpp:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/264376@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Teach-TextBreakIterator-about-strict-and-loose-line-breaking-behaviors branch from 1d81799 to 99eac46 Compare May 22, 2023 21:12
@webkit-commit-queue webkit-commit-queue merged commit 99eac46 into WebKit:main May 22, 2023
@webkit-commit-queue
Copy link
Collaborator

Committed 264376@main (99eac46): https://commits.webkit.org/264376@main

Reviewed commits have been landed. Closing PR #14162 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Text For bugs in text layout and rendering, including international text support.
Projects
None yet
5 participants