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 <jonathan@libreoffice.org>
This commit is contained in:
Jonathan Clark 2024-10-18 06:12:39 -06:00
parent 3cc367f426
commit 224fae69b2
8 changed files with 96 additions and 4 deletions

View file

@ -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)

View file

@ -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>
{
};
}

View file

@ -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

View file

@ -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

View file

@ -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: */

View file

@ -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
{

View file

@ -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

View file

@ -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)
{