diff options
author | Khaled Hosny <khaled@aliftype.com> | 2022-08-26 22:20:55 +0200 |
---|---|---|
committer | Miklos Vajna <vmiklos@collabora.com> | 2022-08-31 08:29:49 +0200 |
commit | 8cb4db941f91cc234dd18c61f8b1e51f65360d1f (patch) | |
tree | 4637ae5e0d509302f7b7a1b12f580a1749e4e36e /vcl/source/gdi | |
parent | d325edf88c8dbcdaf324b8e3585a314c9351cf60 (diff) |
tdf#30731: Improve caret travelling in Writer
Previously, when measuring caret position, Writer would measure the
width of the substring before the caret (i.e. layout it independent of
the text after the caret and measure its width).
This is incorrect, though. It assumes cutting the string laying it out
would result in the same width as when laid out as part of a bigger
string, which is invalid assumption when e.g. cutting inside a ligature
or between letters that have different shapes when next to each other,
etc.
This appears to work when the width of the substring laid out alone is
close enough to its width when laid out with the full text. But in cases
where is widths are largely different, like the extreme case in the bug
report, the caret will be jumping around as it is positioned based on
the unligated glyphs not the ligated, rendered glyphs.
This change introduces a special mode of measuring text width for caret
positioning, that will layout the whole string that return the width of
the requested substring.
Fields and small caps text are trickier to handle, so old behaviour is
retained for them. Now one will probably notice but if they do, it can
be dealt with then.
This also tries to be conservative and keep other pleases using the
existing behaviour which might be desirable (e.g. when measuring text
width for line breaking, we want the unligated width), but there might
be other places that should use the new behaviour.
To handle caret inside ligatures, the grapheme clusters in the ligature
are counted and the width of the whole ligature is distributed on them
evenly. A further improvement would be using HarfBuzz API to get
ligature caret positions for fonts that provide them, which helps when
the ligature components have different widths.
Change-Id: I02062e2e2e1b1a35c8f84307c0a8f5d743059ab5
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/138889
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Diffstat (limited to 'vcl/source/gdi')
-rw-r--r-- | vcl/source/gdi/CommonSalLayout.cxx | 50 | ||||
-rw-r--r-- | vcl/source/gdi/sallayout.cxx | 18 |
2 files changed, 55 insertions, 13 deletions
diff --git a/vcl/source/gdi/CommonSalLayout.cxx b/vcl/source/gdi/CommonSalLayout.cxx index fc28200ff98a..f75dc12dedd0 100644 --- a/vcl/source/gdi/CommonSalLayout.cxx +++ b/vcl/source/gdi/CommonSalLayout.cxx @@ -26,6 +26,7 @@ #include <vcl/unohelp.hxx> #include <vcl/font/Feature.hxx> #include <vcl/font/FeatureParser.hxx> +#include <vcl/svapp.hxx> #include <ImplLayoutArgs.hxx> #include <TextLayoutCache.hxx> @@ -638,19 +639,58 @@ bool GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay return true; } -void GenericSalLayout::GetCharWidths(std::vector<DeviceCoordinate>& rCharWidths) const +void GenericSalLayout::GetCharWidths(std::vector<DeviceCoordinate>& rCharWidths, const OUString& rStr) const { const int nCharCount = mnEndCharPos - mnMinCharPos; rCharWidths.clear(); rCharWidths.resize(nCharCount, 0); + css::uno::Reference<css::i18n::XBreakIterator> xBreak; + auto aLocale(maLanguageTag.getLocale()); + for (auto const& aGlyphItem : m_GlyphItems) { - const int nIndex = aGlyphItem.charPos() - mnMinCharPos; - if (nIndex >= nCharCount) + if (aGlyphItem.charPos() >= mnEndCharPos) continue; - rCharWidths[nIndex] += aGlyphItem.newWidth(); + if (aGlyphItem.charCount() > 1 && aGlyphItem.newWidth() != 0 && !rStr.isEmpty()) + { + // We are calculating DX array for cursor positions and this is a + // ligature, we want to distribute the glyph width over the + // ligature components. + if (!xBreak.is()) + xBreak = mxBreak.is() ? mxBreak : vcl::unohelper::CreateBreakIterator(); + + sal_Int32 nDone; + sal_Int32 nPos = aGlyphItem.charPos(); + unsigned int nGraphemeCount = 0; + + // Count grapheme clusters in the ligatures. + while (nPos < aGlyphItem.charPos() + aGlyphItem.charCount()) + { + nPos = xBreak->nextCharacters(rStr, nPos, aLocale, + css::i18n::CharacterIteratorMode::SKIPCELL, 1, nDone); + nGraphemeCount++; + } + + // Set the width of each grapheme cluster. + nPos = aGlyphItem.charPos(); + auto nWidth = aGlyphItem.newWidth() / nGraphemeCount; + // rounding difference + auto nDiff = aGlyphItem.newWidth() - (nWidth * nGraphemeCount); + for (unsigned int i = 0; i < nGraphemeCount; i++) + { + rCharWidths[nPos - mnMinCharPos] += nWidth; + // add rounding difference to last component to maintain + // ligature width. + if (i == nGraphemeCount - 1) + rCharWidths[nPos - mnMinCharPos] += nDiff; + nPos = xBreak->nextCharacters(rStr, nPos, aLocale, + css::i18n::CharacterIteratorMode::SKIPCELL, 1, nDone); + } + } + else + rCharWidths[aGlyphItem.charPos() - mnMinCharPos] += aGlyphItem.newWidth(); } } @@ -665,7 +705,7 @@ void GenericSalLayout::ApplyDXArray(const double* pDXArray, const sal_Bool* pKas std::unique_ptr<double[]> const pNewCharWidths(new double[nCharCount]); // Get the natural character widths (i.e. before applying DX adjustments). - GetCharWidths(aOldCharWidths); + GetCharWidths(aOldCharWidths, {}); // Calculate the character widths after DX adjustments. for (int i = 0; i < nCharCount; ++i) diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx index eca07148ed53..6d22b302b6c5 100644 --- a/vcl/source/gdi/sallayout.cxx +++ b/vcl/source/gdi/sallayout.cxx @@ -133,6 +133,7 @@ sal_UCS4 GetLocalizedChar( sal_UCS4 nChar, LanguageType eLang ) SalLayout::SalLayout() : mnMinCharPos( -1 ), mnEndCharPos( -1 ), + maLanguageTag( LANGUAGE_DONTKNOW ), mnUnitsPerPixel( 1 ), mnOrientation( 0 ), maDrawOffset( 0, 0 ), @@ -147,6 +148,7 @@ void SalLayout::AdjustLayout( vcl::text::ImplLayoutArgs& rArgs ) mnMinCharPos = rArgs.mnMinCharPos; mnEndCharPos = rArgs.mnEndCharPos; mnOrientation = rArgs.mnOrientation; + maLanguageTag = rArgs.maLanguageTag; } DevicePoint SalLayout::GetDrawPosition(const DevicePoint& rRelative) const @@ -263,10 +265,10 @@ SalLayoutGlyphs SalLayout::GetGlyphs() const return SalLayoutGlyphs(); // invalid } -DeviceCoordinate GenericSalLayout::FillDXArray( std::vector<DeviceCoordinate>* pCharWidths ) const +DeviceCoordinate GenericSalLayout::FillDXArray( std::vector<DeviceCoordinate>* pCharWidths, const OUString& rStr ) const { if (pCharWidths) - GetCharWidths(*pCharWidths); + GetCharWidths(*pCharWidths, rStr); return GetTextWidth(); } @@ -494,7 +496,7 @@ void GenericSalLayout::GetCaretPositions( int nMaxIndex, sal_Int32* pCaretXArray sal_Int32 GenericSalLayout::GetTextBreak( DeviceCoordinate nMaxWidth, DeviceCoordinate nCharExtra, int nFactor ) const { std::vector<DeviceCoordinate> aCharWidths; - GetCharWidths(aCharWidths); + GetCharWidths(aCharWidths, {}); DeviceCoordinate nWidth = 0; for( int i = mnMinCharPos; i < mnEndCharPos; ++i ) @@ -675,7 +677,7 @@ void MultiSalLayout::AdjustLayout( vcl::text::ImplLayoutArgs& rArgs ) mpLayouts[n]->SalLayout::AdjustLayout( aMultiArgs ); // then we can measure the unmodified metrics int nCharCount = rArgs.mnEndCharPos - rArgs.mnMinCharPos; - FillDXArray( &aJustificationArray ); + FillDXArray( &aJustificationArray, {} ); // #i17359# multilayout is not simplified yet, so calculating the // unjustified width needs handholding; also count the number of // stretchable virtual char widths @@ -1039,12 +1041,12 @@ sal_Int32 MultiSalLayout::GetTextBreak( DeviceCoordinate nMaxWidth, DeviceCoordi int nCharCount = mnEndCharPos - mnMinCharPos; std::vector<DeviceCoordinate> aCharWidths; std::vector<DeviceCoordinate> aFallbackCharWidths; - mpLayouts[0]->FillDXArray( &aCharWidths ); + mpLayouts[0]->FillDXArray( &aCharWidths, {} ); for( int n = 1; n < mnLevel; ++n ) { SalLayout& rLayout = *mpLayouts[ n ]; - rLayout.FillDXArray( &aFallbackCharWidths ); + rLayout.FillDXArray( &aFallbackCharWidths, {} ); double fUnitMul = mnUnitsPerPixel; fUnitMul /= rLayout.GetUnitsPerPixel(); for( int i = 0; i < nCharCount; ++i ) @@ -1070,7 +1072,7 @@ sal_Int32 MultiSalLayout::GetTextBreak( DeviceCoordinate nMaxWidth, DeviceCoordi return -1; } -DeviceCoordinate MultiSalLayout::FillDXArray( std::vector<DeviceCoordinate>* pCharWidths ) const +DeviceCoordinate MultiSalLayout::FillDXArray( std::vector<DeviceCoordinate>* pCharWidths, const OUString& rStr ) const { DeviceCoordinate nMaxWidth = 0; @@ -1086,7 +1088,7 @@ DeviceCoordinate MultiSalLayout::FillDXArray( std::vector<DeviceCoordinate>* pCh for( int n = mnLevel; --n >= 0; ) { // query every fallback level - DeviceCoordinate nTextWidth = mpLayouts[n]->FillDXArray( &aTempWidths ); + DeviceCoordinate nTextWidth = mpLayouts[n]->FillDXArray( &aTempWidths, rStr ); if( !nTextWidth ) continue; // merge results from current level |