i18nutil/source/utility/kashida.cxx | 7 ++-
include/vcl/rendercontext/SalLayoutFlags.hxx | 3 +
include/vcl/vcllayout.hxx | 2 +
vcl/inc/sallayout.hxx | 3 +
vcl/qa/cppunit/complextext.cxx | 51 +++++++++++++++++++++++++++
vcl/source/gdi/CommonSalLayout.cxx | 15 +++++++
vcl/source/gdi/sallayout.cxx | 15 +++++++
vcl/source/outdev/font.cxx | 4 ++
8 files changed, 96 insertions(+), 4 deletions(-)
New commits:
commit 224fae69b224d28a1664c48117e77265ed67a136
Author: Jonathan Clark <jonat...@libreoffice.org>
AuthorDate: Fri Oct 18 06:12:39 2024 -0600
Commit: Jonathan Clark <jonat...@libreoffice.org>
CommitDate: Fri Oct 18 21:49:03 2024 +0200
tdf#163215: Enable kashida justification for AAT fonts
Currently, we use HarfBuzz-provided kashida insertion position
information to decide on positions to insert kashida. This data is used
both while ranking kashida insertion positions, and to avoid inserting
kashida in positions that would break shaping on a per-font basis.
Unfortunately, HarfBuzz cannot validate kashida insertion positions for
AAT fonts. As a result, kashida were previously not inserted for text
using AAT fonts.
This change updates kashida justification to skip validation against
HarfBuzz when AAT fonts are used.
Change-Id: If0d31512b1db0f1f8155963f9b1031eb01bacc45
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/175165
Tested-by: Jenkins
Reviewed-by: Jonathan Clark <jonat...@libreoffice.org>
diff --git a/i18nutil/source/utility/kashida.cxx
b/i18nutil/source/utility/kashida.cxx
index aff667033a11..5181f099d92c 100644
--- a/i18nutil/source/utility/kashida.cxx
+++ b/i18nutil/source/utility/kashida.cxx
@@ -136,10 +136,13 @@ bool CanConnectToPrev(sal_Unicode cCh, sal_Unicode
cPrevCh)
bool isSyriacChar(sal_Unicode cCh)
{
- return (cCh >= 0x700 && cCh <= 0x74F) || (cCh >= 0x860 && cCh <= 0x86A);
+ return u_getIntPropertyValue(cCh, UCHAR_SCRIPT) == USCRIPT_SYRIAC;
}
-bool isArabicChar(sal_Unicode cCh) { return cCh >= 0x60C && cCh <= 0x6FE; }
+bool isArabicChar(sal_Unicode cCh)
+{
+ return u_getIntPropertyValue(cCh, UCHAR_SCRIPT) == USCRIPT_ARABIC;
+}
std::optional<i18nutil::KashidaPosition>
GetWordKashidaPositionArabic(const OUString& rWord, const std::vector<bool>&
pValidPositions)
diff --git a/include/vcl/rendercontext/SalLayoutFlags.hxx
b/include/vcl/rendercontext/SalLayoutFlags.hxx
index 3424fa2a1de4..be7e493e49b8 100644
--- a/include/vcl/rendercontext/SalLayoutFlags.hxx
+++ b/include/vcl/rendercontext/SalLayoutFlags.hxx
@@ -31,13 +31,14 @@ enum class SalLayoutFlags
KerningAsian = 0x0020,
Vertical = 0x0040,
DisableLigatures = 0x0200,
+ DisableKashidaValidation = 0x0400,
ForFallback = 0x2000,
GlyphItemsOnly = 0x4000,
UnclusteredGlyphs = 0x8000,
};
namespace o3tl
{
-template <> struct typed_flags<SalLayoutFlags> :
is_typed_flags<SalLayoutFlags, 0xE277>
+template <> struct typed_flags<SalLayoutFlags> :
is_typed_flags<SalLayoutFlags, 0xE677>
{
};
}
diff --git a/include/vcl/vcllayout.hxx b/include/vcl/vcllayout.hxx
index b36341ccc7a7..e6b743f626d5 100644
--- a/include/vcl/vcllayout.hxx
+++ b/include/vcl/vcllayout.hxx
@@ -107,6 +107,8 @@ public:
}
virtual void GetCaretPositions( std::vector<double>& rCaretPositions,
const OUString& rStr ) const = 0;
+
+ virtual bool HasFontKashidaPositions() const = 0;
virtual bool IsKashidaPosValid ( int /*nCharPos*/, int /*nNextCharPos*/
) const = 0; // i60594
// methods using glyph indexing
diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index ed539f058402..a8c0de8438c2 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -69,6 +69,7 @@ public:
SAL_DLLPRIVATE bool GetNextGlyph(const GlyphItem** pGlyph,
basegfx::B2DPoint& rPos, int& nStart,
const LogicalFontInstance** ppGlyphFont =
nullptr) const override;
SAL_DLLPRIVATE bool GetOutline(basegfx::B2DPolyPolygonVector&)
const override;
+ SAL_DLLPRIVATE bool HasFontKashidaPositions() const override;
SAL_DLLPRIVATE bool IsKashidaPosValid(int nCharPos, int
nNextCharPos) const override;
SAL_DLLPRIVATE SalLayoutGlyphs GetGlyphs() const final override;
@@ -117,6 +118,7 @@ public:
void DrawText(SalGraphics&) const final override;
SalLayoutGlyphs GetGlyphs() const final override;
+ bool HasFontKashidaPositions() const final override;
bool IsKashidaPosValid(int nCharPos, int nNextCharPos) const
final override;
// used by upper layers
@@ -171,6 +173,7 @@ private:
hb_set_t* mpVertGlyphs;
const bool mbFuzzing;
+ bool m_bHasFontKashidaPositions = false;
};
#endif // INCLUDED_VCL_INC_SALLAYOUT_HXX
diff --git a/vcl/qa/cppunit/complextext.cxx b/vcl/qa/cppunit/complextext.cxx
index 62b8ac2777dc..a3d033e7adba 100644
--- a/vcl/qa/cppunit/complextext.cxx
+++ b/vcl/qa/cppunit/complextext.cxx
@@ -697,4 +697,55 @@ CPPUNIT_TEST_FIXTURE(VclComplexTextTest,
testPartialArabicComposition)
CPPUNIT_ASSERT_DOUBLES_EQUAL(nCompleteWidth, nPartialWidth, /*delta*/
0.01);
}
+CPPUNIT_TEST_FIXTURE(VclComplexTextTest, testTdf163215)
+{
+ OUString aStr = u"ببببب"_ustr;
+ vcl::Font aFont(u"DejaVu Sans"_ustr, u"Book"_ustr, Size(0, 2048));
+
+ ScopedVclPtrInstance<VirtualDevice> pOutDev;
+ pOutDev->SetFont(aFont);
+
+ // Characteristic case with kashida position validation
+ auto pLayout1 = pOutDev->ImplLayout(aStr, 0, aStr.getLength(), Point(), 0,
{}, {},
+ SalLayoutFlags::GlyphItemsOnly);
+ CPPUNIT_ASSERT(pLayout1->HasFontKashidaPositions());
+
+ SalLayoutGlyphs aGlyphs1 = pLayout1->GetGlyphs();
+
+ std::vector<bool> aFoundPositions1;
+ for (const auto& stGlyph : *aGlyphs1.Impl(0))
+ {
+ aFoundPositions1.push_back(stGlyph.IsSafeToInsertKashida());
+ }
+
+ CPPUNIT_ASSERT_EQUAL(size_t(5), aFoundPositions1.size());
+ CPPUNIT_ASSERT(aFoundPositions1.at(0));
+ CPPUNIT_ASSERT(aFoundPositions1.at(1));
+ CPPUNIT_ASSERT(aFoundPositions1.at(2));
+ CPPUNIT_ASSERT(aFoundPositions1.at(3));
+ CPPUNIT_ASSERT(!aFoundPositions1.at(4));
+
+ // Case with kashida position validation disabled
+ auto pLayout2 = pOutDev->ImplLayout(aStr, 0, aStr.getLength(), Point(), 0,
{}, {},
+ SalLayoutFlags::GlyphItemsOnly
+ |
SalLayoutFlags::DisableKashidaValidation);
+ CPPUNIT_ASSERT(!pLayout2->HasFontKashidaPositions());
+
+ SalLayoutGlyphs aGlyphs2 = pLayout2->GetGlyphs();
+
+ std::vector<bool> aFoundPositions2;
+ for (const auto& stGlyph : *aGlyphs2.Impl(0))
+ {
+ aFoundPositions2.push_back(stGlyph.IsSafeToInsertKashida());
+ }
+
+ // With position validation disabled, all positions must be marked as valid
+ CPPUNIT_ASSERT_EQUAL(size_t(5), aFoundPositions2.size());
+ CPPUNIT_ASSERT(aFoundPositions2.at(0));
+ CPPUNIT_ASSERT(aFoundPositions2.at(1));
+ CPPUNIT_ASSERT(aFoundPositions2.at(2));
+ CPPUNIT_ASSERT(aFoundPositions2.at(3));
+ CPPUNIT_ASSERT(aFoundPositions2.at(4));
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/vcl/source/gdi/CommonSalLayout.cxx
b/vcl/source/gdi/CommonSalLayout.cxx
index a932dd54b16f..26347233d65f 100644
--- a/vcl/source/gdi/CommonSalLayout.cxx
+++ b/vcl/source/gdi/CommonSalLayout.cxx
@@ -40,6 +40,7 @@
#include <hb-ot.h>
#include <hb-graphite2.h>
#include <hb-icu.h>
+#include <hb-aat.h>
#include <map>
#include <memory>
@@ -376,6 +377,14 @@ bool
GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
hb_font_t *pHbFont = GetFont().GetHbFont();
bool isGraphite = GetFont().IsGraphiteFont();
+ // tdf#163215: Identify layouts that don't have strict kashida position
validation.
+ m_bHasFontKashidaPositions = false;
+ if (!(rArgs.mnFlags & SalLayoutFlags::DisableKashidaValidation))
+ {
+ hb_face_t* pHbFace = hb_font_get_face(pHbFont);
+ m_bHasFontKashidaPositions = !hb_aat_layout_has_substitution(pHbFace);
+ }
+
int nGlyphCapacity = 2 * (rArgs.mnEndCharPos - rArgs.mnMinCharPos);
m_GlyphItems.reserve(nGlyphCapacity);
@@ -683,7 +692,9 @@ bool
GenericSalLayout::LayoutText(vcl::text::ImplLayoutArgs& rArgs, const SalLay
if (hb_glyph_info_get_glyph_flags(&pHbGlyphInfos[i]) &
HB_GLYPH_FLAG_UNSAFE_TO_BREAK)
nGlyphFlags |= GlyphItemFlags::IS_UNSAFE_TO_BREAK;
- if (hb_glyph_info_get_glyph_flags(&pHbGlyphInfos[i]) &
HB_GLYPH_FLAG_SAFE_TO_INSERT_TATWEEL)
+ if (!m_bHasFontKashidaPositions
+ || (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;
@@ -1010,6 +1021,8 @@ void GenericSalLayout::ApplyJustificationData(const
JustificationData& rstJustif
}
}
+bool GenericSalLayout::HasFontKashidaPositions() const { return
m_bHasFontKashidaPositions; }
+
// Kashida will be inserted between nCharPos and nNextCharPos.
bool GenericSalLayout::IsKashidaPosValid(int nCharPos, int nNextCharPos) const
{
diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx
index 4d2d4fe68475..e8dc3999e216 100644
--- a/vcl/source/gdi/sallayout.cxx
+++ b/vcl/source/gdi/sallayout.cxx
@@ -1243,6 +1243,21 @@ bool
MultiSalLayout::GetOutline(basegfx::B2DPolyPolygonVector& rPPV) const
return bRet;
}
+bool MultiSalLayout::HasFontKashidaPositions() const
+{
+ // tdf#163215: VCL cannot suggest valid kashida positions for certain
fonts (e.g. AAT).
+ // In order to strictly validate kashida positions, all fallback fonts
must allow it.
+ for (int n = 0; n < mnLevel; ++n)
+ {
+ if (!mpLayouts[n]->HasFontKashidaPositions())
+ {
+ return false;
+ }
+ }
+
+ return true;
+}
+
bool MultiSalLayout::IsKashidaPosValid(int nCharPos, int nNextCharPos) const
{
// Check the base layout
diff --git a/vcl/source/outdev/font.cxx b/vcl/source/outdev/font.cxx
index a2f32327f72c..334f6a311eb0 100644
--- a/vcl/source/outdev/font.cxx
+++ b/vcl/source/outdev/font.cxx
@@ -1227,6 +1227,10 @@ void OutputDevice::GetWordKashidaPositions(const
OUString& rText,
if (!pSalLayout)
return;
+ // tdf#163215: VCL cannot suggest valid kashida positions for certain
fonts (e.g. AAT).
+ if (!pSalLayout->HasFontKashidaPositions())
+ return;
+
pOutMap->resize(nEnd, false);
for (sal_Int32 i = 0; i < nEnd; ++i)
{
