From dccd8a928eef856e5fce490ec24a8a483a841ad5 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Mon, 3 Apr 2023 06:41:22 +0000 Subject: [PATCH] Bug 1825983 - Refactor handling of trailing trimmable whitespace. r=emilio This slightly simplifies gfxTextRun::BreakAndMeasureText, and should make it less confusing to reason about trailing whitespace in nsTextFrame/nsLineLayout. Differential Revision: https://phabricator.services.mozilla.com/D174377 --- gfx/thebes/gfxTextRun.cpp | 25 +++++------------- gfx/thebes/gfxTextRun.h | 32 ++++++++++------------- layout/generic/nsTextFrame.cpp | 47 ++++++++++++++++------------------ 3 files changed, 41 insertions(+), 63 deletions(-) diff --git a/gfx/thebes/gfxTextRun.cpp b/gfx/thebes/gfxTextRun.cpp index f187fbf10d3f..9333eb545751 100644 --- a/gfx/thebes/gfxTextRun.cpp +++ b/gfx/thebes/gfxTextRun.cpp @@ -924,7 +924,7 @@ void gfxTextRun::ClassifyAutoHyphenations(uint32_t aStart, Range aRange, uint32_t gfxTextRun::BreakAndMeasureText( uint32_t aStart, uint32_t aMaxLength, bool aLineBreakBefore, gfxFloat aWidth, PropertyProvider* aProvider, SuppressBreak aSuppressBreak, - gfxFloat* aTrimWhitespace, bool aWhitespaceCanHang, Metrics* aMetrics, + gfxFloat* aTrimmableWhitespace, Metrics* aMetrics, gfxFont::BoundingBoxType aBoundingBoxType, DrawTarget* aRefDrawTarget, bool* aUsedHyphenation, uint32_t* aLastBreak, bool aCanWordWrap, bool aCanWhitespaceWrap, gfxBreakPriority* aBreakPriority) { @@ -1126,7 +1126,7 @@ uint32_t gfxTextRun::BreakAndMeasureText( } advance += charAdvance; - if (aTrimWhitespace || aWhitespaceCanHang) { + if (aTrimmableWhitespace) { if (mCharacterGlyphs[i].CharIsSpace()) { ++trimmableChars; trimmableAdvance += charAdvance; @@ -1169,26 +1169,13 @@ uint32_t gfxTextRun::BreakAndMeasureText( if (aMetrics) { auto fitEnd = aStart + charsFit; - // Initially, measure everything, so that our bounding box includes - // any trimmable or hanging whitespace. + // Get the overall metrics if requested (this includes any potentially + // trimmable or hanging whitespace). *aMetrics = MeasureText(Range(aStart, fitEnd), aBoundingBoxType, aRefDrawTarget, aProvider); - if (aTrimWhitespace || aWhitespaceCanHang) { - // Measure trailing whitespace that is to be trimmed/hung. - Metrics trimOrHangMetrics = - MeasureText(Range(fitEnd - trimmableChars, fitEnd), aBoundingBoxType, - aRefDrawTarget, aProvider); - if (aTrimWhitespace) { - aMetrics->mAdvanceWidth -= trimOrHangMetrics.mAdvanceWidth; - } else if (aMetrics->mAdvanceWidth > aWidth) { - // Restrict width of hanging whitespace so it doesn't overflow. - aMetrics->mAdvanceWidth = std::max( - aWidth, aMetrics->mAdvanceWidth - trimOrHangMetrics.mAdvanceWidth); - } - } } - if (aTrimWhitespace) { - *aTrimWhitespace = trimmableAdvance; + if (aTrimmableWhitespace) { + *aTrimmableWhitespace = trimmableAdvance; } if (aUsedHyphenation) { *aUsedHyphenation = usedHyphenation; diff --git a/gfx/thebes/gfxTextRun.h b/gfx/thebes/gfxTextRun.h index 779fd37dbd01..6fc1ee148f01 100644 --- a/gfx/thebes/gfxTextRun.h +++ b/gfx/thebes/gfxTextRun.h @@ -415,14 +415,11 @@ class gfxTextRun : public gfxShapedText { * @param aLineBreakBefore set to true if and only if there is an actual * line break at the start of this string. * @param aSuppressBreak what break should be suppressed. - * @param aTrimWhitespace if non-null, then we allow a trailing run of - * spaces to be trimmed; the width of the space(s) will not be included in - * the measured string width for comparison with the limit aWidth, and - * trimmed spaces will not be included in returned metrics. The width - * of the trimmed spaces will be returned in aTrimWhitespace. - * Trimmed spaces are still counted in the "characters fit" result. - * @param aHangWhitespace true if we allow whitespace to overflow the - * container at a soft-wrap + * @param aTrimmableWhitespace if non-null, returns the advance of any + * run of trailing spaces that might be trimmed if the run ends up at + * end-of-line. + * Trimmable spaces are still counted in the "characters fit" result, and + * contribute to the returned Metrics values. * @param aMetrics if non-null, we fill this in for the returned substring. * If a hyphenation break was used, the hyphen is NOT included in the returned * metrics. @@ -453,17 +450,14 @@ class gfxTextRun : public gfxShapedText { * Note that negative advance widths are possible especially if negative * spacing is provided. */ - uint32_t BreakAndMeasureText(uint32_t aStart, uint32_t aMaxLength, - bool aLineBreakBefore, gfxFloat aWidth, - PropertyProvider* aProvider, - SuppressBreak aSuppressBreak, - gfxFloat* aTrimWhitespace, bool aHangWhitespace, - Metrics* aMetrics, - gfxFont::BoundingBoxType aBoundingBoxType, - DrawTarget* aDrawTargetForTightBoundingBox, - bool* aUsedHyphenation, uint32_t* aLastBreak, - bool aCanWordWrap, bool aCanWhitespaceWrap, - gfxBreakPriority* aBreakPriority); + uint32_t BreakAndMeasureText( + uint32_t aStart, uint32_t aMaxLength, bool aLineBreakBefore, + gfxFloat aWidth, PropertyProvider* aProvider, + SuppressBreak aSuppressBreak, gfxFloat* aTrimmableWhitespace, + Metrics* aMetrics, gfxFont::BoundingBoxType aBoundingBoxType, + DrawTarget* aDrawTargetForTightBoundingBox, bool* aUsedHyphenation, + uint32_t* aLastBreak, bool aCanWordWrap, bool aCanWhitespaceWrap, + gfxBreakPriority* aBreakPriority); // Utility getters diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index 34da4d9ced98..b79d47fe95f0 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -9140,7 +9140,7 @@ void nsTextFrame::ReflowText(nsLineLayout& aLineLayout, nscoord aAvailableWidth, } uint32_t transformedLastBreak = 0; bool usedHyphenation; - gfxFloat trimmedWidth = 0; + gfxFloat trimmableWidth = 0; gfxFloat availWidth = aAvailableWidth; if (Style()->IsTextCombined()) { // If text-combine-upright is 'all', we would compress whatever long @@ -9164,7 +9164,8 @@ void nsTextFrame::ReflowText(nsLineLayout& aLineLayout, nscoord aAvailableWidth, uint32_t transformedCharsFit = mTextRun->BreakAndMeasureText( transformedOffset, transformedLength, HasAnyStateBits(TEXT_START_OF_LINE), availWidth, &provider, suppressBreak, - canTrimTrailingWhitespace ? &trimmedWidth : nullptr, whitespaceCanHang, + canTrimTrailingWhitespace || whitespaceCanHang ? &trimmableWidth + : nullptr, &textMetrics, boundingBoxType, aDrawTarget, &usedHyphenation, &transformedLastBreak, textStyle->WordCanWrap(this), isBreakSpaces, &breakPriority); @@ -9224,31 +9225,27 @@ void nsTextFrame::ReflowText(nsLineLayout& aLineLayout, nscoord aAvailableWidth, AddStateBits(TEXT_NO_RENDERED_GLYPHS); } - gfxFloat trimmableWidth = 0; bool brokeText = forceBreak >= 0 || transformedCharsFit < transformedLength; - if (canTrimTrailingWhitespace) { - // Optimization: if we trimmed trailing whitespace, and we can be sure - // this frame will be at the end of the line, then leave it trimmed off. - // Otherwise we have to undo the trimming, in case we're not at the end of - // the line. (If we actually do end up at the end of the line, we'll have - // to trim it off again in TrimTrailingWhiteSpace, and we'd like to avoid - // having to re-do it.) - if (brokeText || HasAnyStateBits(TEXT_IS_IN_TOKEN_MATHML)) { - // We're definitely going to break so our trailing whitespace should - // definitely be trimmed. Record that we've already done it. - AddStateBits(TEXT_TRIMMED_TRAILING_WHITESPACE); - } else if (!HasAnyStateBits(TEXT_IS_IN_TOKEN_MATHML)) { - // We might not be at the end of the line. (Note that even if this frame - // ends in breakable whitespace, it might not be at the end of the line - // because it might be followed by breakable, but preformatted, - // whitespace.) Undo the trimming. - textMetrics.mAdvanceWidth += trimmedWidth; - trimmableWidth = trimmedWidth; - if (mTextRun->IsRightToLeft()) { - // Space comes before text, so the bounding box is moved to the - // right by trimmdWidth - textMetrics.mBoundingBox.MoveBy(gfxPoint(trimmedWidth, 0)); + if (trimmableWidth > 0.0) { + if (canTrimTrailingWhitespace) { + // Optimization: if we we can be sure this frame will be at end of line, + // then trim the whitespace now. + if (brokeText || HasAnyStateBits(TEXT_IS_IN_TOKEN_MATHML)) { + // We're definitely going to break so our trailing whitespace should + // definitely be trimmed. Record that we've already done it. + AddStateBits(TEXT_TRIMMED_TRAILING_WHITESPACE); + textMetrics.mAdvanceWidth -= trimmableWidth; + trimmableWidth = 0.0; } + } else if (whitespaceCanHang) { + // Figure out how much will hang. + // XXX This probably needs to be passed down and handled by nsLineLayout + // rather than here, e.g. for bug 1712703 and 1253840. + gfxFloat hang = + std::min(std::max(0.0, textMetrics.mAdvanceWidth - availWidth), + trimmableWidth); + textMetrics.mAdvanceWidth -= hang; + trimmableWidth = 0.0; } }