Chromium Code Reviews Chromium Code Reviews
Issue
104813005:
Explicitly set text direction for TextRuns (Closed)
DescriptionExplicitly set text direction for TextRuns
Currently we do test measurement on TextRuns without directionality
information, by assuming that it is left to right. For some scripts and
character combinations the measurements are different in rtl and ltr.
To handle that we currently have HarfBuzz guess the direction based on
the script and content. This is unreliable and fails in many cases.
With this patch the directionality information from the BidiResolver is
plumbed through and used to correctly set the text direction thereby
eliminating the need to for HarfBuzz to guess. This fixes a whole slew
of subtle repainting and selection bidi bugs on all platforms using
HarfBuzz for text shaping (win, linux, chromeos, android).
* Source/core/rendering/InlineTextBox.cpp:
Pass direction to RenderText::width method based on element
directionality.
* Source/core/rendering/RenderBR.h:
Add TextDirection argument to overriden width method.
* Source/core/rendering/RenderBlockFlow.cpp,
Source/core/rendering/RenderBlockFlow.h:
Add direction argument to constructTextRun method.
* Source/core/rendering/RenderBlockLineLayout.cpp:
Pass directionality to RenderText::width method based on bidi run
direction.
* Source/core/rendering/RenderCombineText.cpp,
Source/core/rendering/RenderCombineText.h:
Pass direction through.
* Source/core/rendering/RenderText.cpp,
Source/core/rendering/RenderText.h:
(RenderText::widthFromCache):
Add textDirection argument and pass it through to constructTextRun.
(RenderText::trimmedPrefWidths):
Assume LTR in trimmedPrefWidths for now, this matches the current
behavior.
(RenderText::computePreferredLogicalWidths):
As preferred widths are computed before layout we do not yet have
the bidi runs for the text (nor the line boxes). Use the
BidiResolver to construct bidi runs for the entire text content and
use the direction information for those runs when computing the
width by passing it to widthFromCache.
(RenderText::width):
Change width methods to take a text direction argument.
* Source/core/rendering/line/BreakingContextInlineHeaders.h:
Add text direction argument to textWidth method and change the
handleText method to pass the direction based on the BidiResolver.
* Source/platform/fonts/harfbuzz/HarfBuzzShaper.cpp,
Source/platform/fonts/harfbuzz/HarfBuzzShaper.h:
(HarfBuzzShaper::shape):
Remove tge shouldSetDirection logic as it is no longer needed (see
shapeHarfBuzzRuns).
(HarfBuzzShaper::shapeHarfBuzzRuns):
Remove shouldSetDirection argument and always set the direction
as the direction information is now avalible when calculating
widths.
* Source/platform/text/BidiResolver.h:
(BidiResolver::createBidiRunsForLine):
Add reorderRuns argument (defaults to true) to allow the reorder
in visual order to be bypassed as the width calculation logic
RenderText::computePreferredLogicalWidths requires the runs to be in
logical order.
R=eseidel@chromium.org, behdad@chromium.org, leviw@chromium.org
BUG=327465
TEST=fast/text/international/mixed-directionality-selection.html
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=164788
Patch Set 1 #
Total comments: 3
Patch Set 2 : addressing leviws comments #Patch Set 3 : Use BidiResolver in RenderText #
Total comments: 4
Patch Set 4 : Patch for landing #Messages
Total messages: 17 (0 generated)
|
