Skip to content
Permalink
Browse files
Teach InlineTextBox::clampOffset() about combined text and hyphenation
https://bugs.webkit.org/show_bug.cgi?id=178032

Reviewed by Zalan Bujtas.

Treat combined text and the last character of a word halve plus hyphen as single units.

With regards to combined text, ideally we would allow arbitrary selection inside combined
text. Currently we do not support selection of combined text. To simplify the process of
adding support for selecting combined text we treat combined text as a single unit. Once
we are confident that we correctly implemented such support we can re-evaluate allowing
arbitrary selection of combined text.

With regards to treating the last character of a word halve plus hyphen as a single unit.
This patch extends the targeted fix made for document markers in r223013 to all code that
makes use of clamped offsets as a result the selection rect for inline boxes more accurately
reflect the rectangle(s) that make up the painted selection. This is a step towards reconciling
the difference between the computation of the rectangle that represents an arbitrary
selection and the code that paints the active selection as part of <https://bugs.webkit.org/show_bug.cgi?id=138913>.

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::localSelectionRect const): Compute text run, including combined text
or hyphens due to line wrapping now that specified start and end positions are clamped with
respect to combined text and hyphens (computed earlier in this function). Only measure the
text represented by the selection if the start position > 0 or the end position is not equal
to the length of the run.
(WebCore::InlineTextBox::paint): Remove unnecessary code to fix up the selection start and
end positions based on the truncation offset as this is done by clampedOffset(), called by
selectionStartEnd().
(WebCore::InlineTextBox::clampedOffset const): Modified to adjust the clamped offset with
respect to truncation as well as treat combined text or a trailing word halve plus hyphen
as single units. Assert that we are not fully truncated because it does not make sense to
be computing the clamped offset in such a situation since nothing should be painted.
(WebCore::InlineTextBox::selectionStartEnd const): Modified to compute the end of an inside
selection using clampedOffset() to account for truncation, combined text or a hyphen. We
already are using clampedOffset() when computing the start and end position for all other
selection states.
(WebCore::InlineTextBox::paintSelection): Compute text run, including combined text
or hyphens due to line wrapping now that specified start and end positions are clamped with
respect to combined text and hyphens (computed earlier in this function). Remove unnecessary
code to adjust selection end point with respect to truncation, combined text, or an added
hyphen now that selectionStartEnd() takes care of this (via clampedOffset()).
(WebCore::InlineTextBox::paintTextSubrangeBackground): Compute text run, including combined
text or hyphens due to line wrapping now that specified start and end positions are clamped
with respect to combined text and hyphens (computed earlier in this function).
(WebCore::InlineTextBox::paintDocumentMarker): Compute text run, including combined text now
that specified start and end positions are clamped with respect to combined text (computed earlier in this function).
Also remove unnecessary code to adjust end offset of the marker with respect to truncation
and length of the text run as clampedOffset() now does this for us.

Canonical link: https://commits.webkit.org/194476@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223259 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
dydz committed Oct 12, 2017
1 parent feb6543 commit 698ba85d01f68e23bb86727207fffd7d488cc75e
Showing 2 changed files with 78 additions and 31 deletions.
@@ -1,3 +1,55 @@
2017-10-12 Daniel Bates <dabates@apple.com>

Teach InlineTextBox::clampOffset() about combined text and hyphenation
https://bugs.webkit.org/show_bug.cgi?id=178032

Reviewed by Zalan Bujtas.

Treat combined text and the last character of a word halve plus hyphen as single units.

With regards to combined text, ideally we would allow arbitrary selection inside combined
text. Currently we do not support selection of combined text. To simplify the process of
adding support for selecting combined text we treat combined text as a single unit. Once
we are confident that we correctly implemented such support we can re-evaluate allowing
arbitrary selection of combined text.

With regards to treating the last character of a word halve plus hyphen as a single unit.
This patch extends the targeted fix made for document markers in r223013 to all code that
makes use of clamped offsets as a result the selection rect for inline boxes more accurately
reflect the rectangle(s) that make up the painted selection. This is a step towards reconciling
the difference between the computation of the rectangle that represents an arbitrary
selection and the code that paints the active selection as part of <https://bugs.webkit.org/show_bug.cgi?id=138913>.

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::localSelectionRect const): Compute text run, including combined text
or hyphens due to line wrapping now that specified start and end positions are clamped with
respect to combined text and hyphens (computed earlier in this function). Only measure the
text represented by the selection if the start position > 0 or the end position is not equal
to the length of the run.
(WebCore::InlineTextBox::paint): Remove unnecessary code to fix up the selection start and
end positions based on the truncation offset as this is done by clampedOffset(), called by
selectionStartEnd().
(WebCore::InlineTextBox::clampedOffset const): Modified to adjust the clamped offset with
respect to truncation as well as treat combined text or a trailing word halve plus hyphen
as single units. Assert that we are not fully truncated because it does not make sense to
be computing the clamped offset in such a situation since nothing should be painted.
(WebCore::InlineTextBox::selectionStartEnd const): Modified to compute the end of an inside
selection using clampedOffset() to account for truncation, combined text or a hyphen. We
already are using clampedOffset() when computing the start and end position for all other
selection states.
(WebCore::InlineTextBox::paintSelection): Compute text run, including combined text
or hyphens due to line wrapping now that specified start and end positions are clamped with
respect to combined text and hyphens (computed earlier in this function). Remove unnecessary
code to adjust selection end point with respect to truncation, combined text, or an added
hyphen now that selectionStartEnd() takes care of this (via clampedOffset()).
(WebCore::InlineTextBox::paintTextSubrangeBackground): Compute text run, including combined
text or hyphens due to line wrapping now that specified start and end positions are clamped
with respect to combined text and hyphens (computed earlier in this function).
(WebCore::InlineTextBox::paintDocumentMarker): Compute text run, including combined text now
that specified start and end positions are clamped with respect to combined text (computed earlier in this function).
Also remove unnecessary code to adjust end offset of the marker with respect to truncation
and length of the text run as clampedOffset() now does this for us.

2017-10-11 Simon Fraser <simon.fraser@apple.com>

Don't assert if mix-blend-mode is set to a non-separable blend mode on a composited layer
@@ -197,16 +197,12 @@ LayoutRect InlineTextBox::localSelectionRect(unsigned startPos, unsigned endPos)
LayoutUnit selectionTop = this->selectionTop();
LayoutUnit selectionHeight = this->selectionHeight();

// FIXME: Adjust selection rect with respect to combined text.
bool ignoreCombinedText = true;
auto text = this->text(ignoreCombinedText);
auto text = this->text();
TextRun textRun = createTextRun(text);
// FIXME: Shouldn't we adjust ePos to textRun.length() if ePos == (m_truncation != cNoTruncation ? m_truncation : m_len)
// so that the selection spans the hypen/combined text?

LayoutRect selectionRect = LayoutRect(LayoutPoint(logicalLeft(), selectionTop), LayoutSize(m_logicalWidth, selectionHeight));
// Avoid computing the font width when the entire line box is selected as an optimization.
if (sPos || ePos != m_len)
if (sPos || ePos != textRun.length())
lineFont().adjustSelectionRectForText(textRun, selectionRect, sPos, ePos);
// FIXME: The computation of the snapped selection rect differs from the computation of this rect
// in paintSelection(). See <https://bugs.webkit.org/show_bug.cgi?id=138913>.
@@ -501,11 +497,6 @@ void InlineTextBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset,
if (haveSelection && (paintSelectedTextOnly || paintSelectedTextSeparately))
std::tie(selectionStart, selectionEnd) = selectionStartEnd();

if (m_truncation != cNoTruncation) {
selectionStart = std::min(selectionStart, static_cast<unsigned>(m_truncation));
selectionEnd = std::min(selectionEnd, static_cast<unsigned>(m_truncation));
}

float emphasisMarkOffset = 0;
bool emphasisMarkAbove;
bool hasTextEmphasis = emphasisMarkExistsAndIsAbove(lineStyle, emphasisMarkAbove);
@@ -598,14 +589,27 @@ void InlineTextBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset,

unsigned InlineTextBox::clampedOffset(unsigned x) const
{
return std::max(std::min(x, start() + len()), start()) - start();
ASSERT(m_truncation != cFullTruncation);
unsigned offset = std::max(std::min(x, m_start + m_len), m_start) - m_start;
if (m_truncation != cNoTruncation)
offset = std::min<unsigned>(offset, m_truncation);
else if (offset == m_len) {
// Fix up the offset if we are combined text or have a hyphen because we manage these embellishments.
// That is, they are not reflected in renderer().text(). We treat combined text as a single unit.
// We also treat the last codepoint in this box and the hyphen as a single unit.
if (auto* combinedText = this->combinedText())
offset = combinedText->combinedStringForRendering().length();
else if (hasHyphen())
offset += lineStyle().hyphenString().length();
}
return offset;
}

std::pair<unsigned, unsigned> InlineTextBox::selectionStartEnd() const
{
auto selectionState = renderer().selectionState();
if (selectionState == RenderObject::SelectionInside)
return { 0, m_len };
return { 0, clampedOffset(m_start + m_len) };

auto start = renderer().view().selection().startPosition();
auto end = renderer().view().selection().endPosition();
@@ -643,14 +647,8 @@ void InlineTextBox::paintSelection(GraphicsContext& context, const FloatPoint& b

// Note that if the text is truncated, we let the thing being painted in the truncation
// draw its own highlight.

// FIXME: Adjust text run for combined text.
bool ignoreCombinedText = true;
auto text = this->text(ignoreCombinedText);
auto text = this->text();
TextRun textRun = createTextRun(text);
unsigned endOfLineIgnoringHyphenAndCombinedText = m_truncation != cNoTruncation ? m_truncation : m_len;
if (selectionEnd == endOfLineIgnoringHyphenAndCombinedText)
selectionEnd = textRun.length(); // Extend selection to include hyphen/combined text.

const RootInlineBox& rootBox = root();
LayoutUnit selectionBottom = rootBox.selectionBottom();
@@ -661,6 +659,8 @@ void InlineTextBox::paintSelection(GraphicsContext& context, const FloatPoint& b

LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, m_logicalWidth, selectionHeight);
lineFont().adjustSelectionRectForText(textRun, selectionRect, selectionStart, selectionEnd);

// FIXME: Support painting combined text.
context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), textRun.ltr()), c);
#else
UNUSED_PARAM(context);
@@ -684,12 +684,11 @@ inline void InlineTextBox::paintTextSubrangeBackground(GraphicsContext& context,
LayoutUnit deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, 0, selectionHeight());

// FIXME: Adjust text run for combined text and hyphenation.
bool ignoreCombinedText = true;
bool ignoreHyphen = true;
auto text = this->text(ignoreCombinedText, ignoreHyphen);
auto text = this->text();
TextRun textRun = createTextRun(text);
lineFont().adjustSelectionRectForText(textRun, selectionRect, startOffset, endOffset);

// FIXME: Support painting combined text.
context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), textRun.ltr()), color);
}

@@ -781,20 +780,15 @@ void InlineTextBox::paintDocumentMarker(GraphicsContext& context, const FloatPoi
unsigned startPosition = clampedOffset(subrange.startOffset);
unsigned endPosition = clampedOffset(subrange.endOffset);

if (m_truncation != cNoTruncation)
endPosition = std::min(endPosition, static_cast<unsigned>(m_truncation));

// Calculate start & width
// FIXME: Adjust text run for combined text.
bool ignoreCombinedText = true;
int deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
int selHeight = selectionHeight();
FloatPoint startPoint(boxOrigin.x(), boxOrigin.y() - deltaY);
auto text = this->text(ignoreCombinedText);
auto text = this->text();
TextRun run = createTextRun(text);

LayoutRect selectionRect = LayoutRect(startPoint, FloatSize(0, selHeight));
lineFont().adjustSelectionRectForText(run, selectionRect, startPosition, endPosition >= len() ? run.length() : endPosition);
lineFont().adjustSelectionRectForText(run, selectionRect, startPosition, endPosition);
IntRect markerRect = enclosingIntRect(selectionRect);
start = markerRect.x() - startPoint.x();
width = markerRect.width();
@@ -838,6 +832,7 @@ void InlineTextBox::paintDocumentMarker(GraphicsContext& context, const FloatPoi
// In larger fonts, though, place the underline up near the baseline to prevent a big gap.
underlineOffset = baseline + 2;
}
// FIXME: Support painting combined text.
context.drawLineForDocumentMarker(FloatPoint(boxOrigin.x() + start, boxOrigin.y() + underlineOffset), width, lineStyleForSubrangeType(subrange.type));
}

0 comments on commit 698ba85

Please sign in to comment.