include/vcl/vcllayout.hxx | 6 +++---
vcl/source/gdi/CommonSalLayout.cxx | 15 ++++++++-------
vcl/source/gdi/pdfwriter_impl.cxx | 6 ++++--
vcl/source/gdi/sallayout.cxx | 8 +++-----
vcl/source/outdev/text.cxx | 11 +++++++----
5 files changed, 25 insertions(+), 21 deletions(-)
New commits:
commit 560e5406f3c6e8e6ed0f1c251b6910465266ed8d
Author: Jonathan Clark <jonat...@libreoffice.org>
AuthorDate: Wed Aug 21 16:54:44 2024 -0600
Commit: Jonathan Clark <jonat...@libreoffice.org>
CommitDate: Thu Aug 22 02:57:52 2024 +0200
tdf#153966 vcl: Fix precision loss laying out right-aligned text
Previously, characters would appear to 'fidget' while typing
right-aligned right-to-left text.
This fix contains the following changes:
- Removed a floating point truncate during right-align positioning.
- Removed unnecessary repeated multiply-add from the main layout loop.
Change-Id: I2f070efb6a1387db1595b41023bd0f076064afda
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/172230
Tested-by: Jenkins
Reviewed-by: Jonathan Clark <jonat...@libreoffice.org>
diff --git a/include/vcl/vcllayout.hxx b/include/vcl/vcllayout.hxx
index aa80e815a843..b36341ccc7a7 100644
--- a/include/vcl/vcllayout.hxx
+++ b/include/vcl/vcllayout.hxx
@@ -72,8 +72,8 @@ public:
// used by upper layers
basegfx::B2DPoint& DrawBase() { return
maDrawBase; }
const basegfx::B2DPoint& DrawBase() const { return
maDrawBase; }
- Point& DrawOffset() { return
maDrawOffset; }
- const Point& DrawOffset() const { return
maDrawOffset; }
+ basegfx::B2DPoint& DrawOffset() { return maDrawOffset; }
+ const basegfx::B2DPoint& DrawOffset() const { return maDrawOffset; }
basegfx::B2DPoint GetDrawPosition( const basegfx::B2DPoint& rRelative =
basegfx::B2DPoint(0,0) ) const;
virtual bool LayoutText( vcl::text::ImplLayoutArgs&, const
SalLayoutGlyphsImpl* ) = 0; // first step of layouting
@@ -136,7 +136,7 @@ protected:
Degree10 mnOrientation;
- mutable Point maDrawOffset;
+ basegfx::B2DPoint maDrawOffset;
basegfx::B2DPoint maDrawBase;
bool mbSubpixelPositioning;
diff --git a/vcl/source/gdi/CommonSalLayout.cxx
b/vcl/source/gdi/CommonSalLayout.cxx
index 0425b5794bca..769929f76058 100644
--- a/vcl/source/gdi/CommonSalLayout.cxx
+++ b/vcl/source/gdi/CommonSalLayout.cxx
@@ -436,7 +436,7 @@ bool
GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
double nYScale = 0;
GetFont().GetScale(&nXScale, &nYScale);
- basegfx::B2DPoint aCurrPos(0, 0);
+ hb_position_t nCurrX = 0;
while (true)
{
int nBidiMinRunPos, nBidiEndRunPos;
@@ -683,7 +683,8 @@ bool
GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
if (hb_glyph_info_get_glyph_flags(&pHbGlyphInfos[i]) &
HB_GLYPH_FLAG_SAFE_TO_INSERT_TATWEEL)
nGlyphFlags |= GlyphItemFlags::IS_SAFE_TO_INSERT_KASHIDA;
- double nAdvance, nXOffset, nYOffset;
+ hb_position_t nAdvance;
+ double nXOffset, nYOffset;
if (aSubRun.maDirection == HB_DIRECTION_TTB)
{
nGlyphFlags |= GlyphItemFlags::IS_VERTICAL;
@@ -712,19 +713,19 @@ bool
GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
nYOffset = -pHbPositions[i].y_offset;
}
- nAdvance = nAdvance * nXScale;
+ double nScaledAdvance = nAdvance * nXScale;
nXOffset = nXOffset * nXScale;
nYOffset = nYOffset * nYScale;
if (!GetSubpixelPositioning())
{
- nAdvance = std::round(nAdvance);
+ nScaledAdvance = std::round(nScaledAdvance);
nXOffset = std::round(nXOffset);
nYOffset = std::round(nYOffset);
}
- basegfx::B2DPoint aNewPos(aCurrPos.getX() + nXOffset,
aCurrPos.getY() + nYOffset);
+ basegfx::B2DPoint aNewPos(nCurrX * nXScale + nXOffset,
nYOffset);
const GlyphItem aGI(nCharPos, nCharCount, nGlyphIndex,
aNewPos, nGlyphFlags,
- nAdvance, nXOffset, nYOffset,
nOrigCharPos);
+ nScaledAdvance, nXOffset, nYOffset,
nOrigCharPos);
if (aGI.origCharPos() >= rArgs.mnDrawMinCharPos
&& aGI.origCharPos() < rArgs.mnDrawEndCharPos)
@@ -735,7 +736,7 @@ bool
GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
if (aGI.origCharPos() >= rArgs.mnDrawOriginCluster
&& aGI.origCharPos() < rArgs.mnDrawEndCharPos)
{
- aCurrPos.adjustX(nAdvance);
+ nCurrX += nAdvance;
}
}
}
diff --git a/vcl/source/gdi/pdfwriter_impl.cxx
b/vcl/source/gdi/pdfwriter_impl.cxx
index d46cbf0da80e..434156491703 100644
--- a/vcl/source/gdi/pdfwriter_impl.cxx
+++ b/vcl/source/gdi/pdfwriter_impl.cxx
@@ -6520,11 +6520,13 @@ void PDFWriterImpl::drawRelief( SalLayout& rLayout,
const OUString& rText, bool
if( eRelief == FontRelief::Engraved )
nOff = -nOff;
- rLayout.DrawOffset() += Point( nOff, nOff );
+ auto aPrevOffset = rLayout.DrawOffset();
+ rLayout.DrawOffset()
+ += basegfx::B2DPoint{ static_cast<double>(nOff),
static_cast<double>(nOff) };
updateGraphicsState();
drawLayout( rLayout, rText, bTextLines );
- rLayout.DrawOffset() -= Point( nOff, nOff );
+ rLayout.DrawOffset() = aPrevOffset;
setTextLineColor( aTextLineColor );
setOverlineColor( aOverlineColor );
aSetFont.SetColor( aTextColor );
diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx
index 1a63504ecc88..4d2d4fe68475 100644
--- a/vcl/source/gdi/sallayout.cxx
+++ b/vcl/source/gdi/sallayout.cxx
@@ -143,9 +143,8 @@ void SalLayout::AdjustLayout( vcl::text::ImplLayoutArgs&
rArgs )
basegfx::B2DPoint SalLayout::GetDrawPosition(const basegfx::B2DPoint&
rRelative) const
{
- basegfx::B2DPoint aPos(maDrawBase);
- basegfx::B2DPoint aOfs(rRelative.getX() + maDrawOffset.X(),
- rRelative.getY() + maDrawOffset.Y());
+ basegfx::B2DPoint aPos{maDrawBase};
+ basegfx::B2DPoint aOfs = rRelative + maDrawOffset;
if( mnOrientation == 0_deg10 )
aPos += aOfs;
@@ -1220,8 +1219,7 @@ bool MultiSalLayout::GetNextGlyph(const GlyphItem**
pGlyph,
{
int nFontTag = nLevel << GF_FONTSHIFT;
nStart |= nFontTag;
- rPos.adjustX(maDrawBase.getX() + maDrawOffset.X());
- rPos.adjustY(maDrawBase.getY() + maDrawOffset.Y());
+ rPos += maDrawBase + maDrawOffset;
return true;
}
}
diff --git a/vcl/source/outdev/text.cxx b/vcl/source/outdev/text.cxx
index d1528fb68101..f327f56c9024 100644
--- a/vcl/source/outdev/text.cxx
+++ b/vcl/source/outdev/text.cxx
@@ -214,7 +214,7 @@ bool OutputDevice::ImplDrawRotateText( SalLayout&
rSalLayout )
tools::Rectangle aBoundRect;
rSalLayout.DrawBase() = basegfx::B2DPoint( 0, 0 );
- rSalLayout.DrawOffset() = Point( 0, 0 );
+ rSalLayout.DrawOffset() = basegfx::B2DPoint{ 0, 0 };
if (basegfx::B2DRectangle r; rSalLayout.GetBoundRect(r))
{
aBoundRect = SalLayout::BoundRect2Rectangle(r);
@@ -368,9 +368,12 @@ void OutputDevice::ImplDrawSpecialText( SalLayout&
rSalLayout )
if ( eRelief == FontRelief::Engraved )
nOff = -nOff;
- rSalLayout.DrawOffset() += Point( nOff, nOff);
- ImplDrawTextDirect( rSalLayout, mbTextLines );
- rSalLayout.DrawOffset() -= Point( nOff, nOff);
+
+ auto aPrevOffset = rSalLayout.DrawOffset();
+ rSalLayout.DrawOffset()
+ += basegfx::B2DPoint{ static_cast<double>(nOff),
static_cast<double>(nOff) };
+ ImplDrawTextDirect(rSalLayout, mbTextLines);
+ rSalLayout.DrawOffset() = aPrevOffset;
SetTextLineColor( aTextLineColor );
SetOverlineColor( aOverlineColor );
