Currently we verify Kashida insertion positions with HarfBuzz to drop positions where it is not safe to insert kashida without re-shaping (as we insert Kashida after shaping). This works fine for OpenType fonts, but HarfBuzz does not (and can not) do any such validation for AAT fonts, so all Kashida insertion positions are considered invalid for such fonts and we get no Kashida insertion at all. AAT Arabic fonts are rather common on macOS (all Arabic fonts shipped by Apple seem to be AAT fonts). A suggested fix is to not do any Kashida position validation for AAT fonts, so basically revert to the old behavior. Almost all Arabic AAT fonts are rather simple ones that do not do any complex substitutions, so the validation is superfluous and no validation is better than no Kashida justification. For testing, if access to macOS system fonts is not possible, http://wiki.irmug.com/index.php/X_Series_2 fonts can be used as they are hyprid OpenType/AAT fonts (and HarfBuzz will use the AAT layout table so OpenType layout tables are effectively ignored).
> Almost all Arabic AAT fonts are rather simple Is it possible to distinguish rather-simple from not-simple fonts?
(In reply to Eyal Rozenberg from comment #1) > > Almost all Arabic AAT fonts are rather simple > > Is it possible to distinguish rather-simple from not-simple fonts? Not to my knowledge. Unlike OpenType where the shaping is split between the font and the shaping engine, AAT font do everything themselves (using a state machine) and the shaping engine executes it without much knowledge about the writing system, so simple and complex fonts all have a state machine that does something.
-> NEW
Khaled: I thought about a first part to create OutputDevice::HasAatFeatures that would be then called in editeng/source/editeng/impedit3.cxx Here's the patch: diff --git a/include/vcl/outdev.hxx b/include/vcl/outdev.hxx index 3f534c50b29a..4a76bdaa18bc 100644 --- a/include/vcl/outdev.hxx +++ b/include/vcl/outdev.hxx @@ -1159,6 +1159,8 @@ public: sal_Int32 HasGlyphs( const vcl::Font& rFont, std::u16string_view rStr, sal_Int32 nIndex = 0, sal_Int32 nLen = -1 ) const; + bool HasAatFeatures() const; + tools::Long GetMinKashida() const; // i60594 diff --git a/vcl/inc/font/FeatureCollector.hxx b/vcl/inc/font/FeatureCollector.hxx index 1b71d021599d..d4c6fe4160cd 100644 --- a/vcl/inc/font/FeatureCollector.hxx +++ b/vcl/inc/font/FeatureCollector.hxx @@ -12,6 +12,7 @@ #include <vcl/font/Feature.hxx> #include <hb.h> +#include <hb-aat.h> #include <i18nlangtag/languagetag.hxx> #include <font/PhysicalFontFace.hxx> @@ -42,6 +43,7 @@ private: public: bool collect(); + bool hasAatFeatures() { return hb_aat_layout_has_substitution(m_pHbFace); }; }; } // namespace vcl::font diff --git a/vcl/source/outdev/font.cxx b/vcl/source/outdev/font.cxx index a2f32327f72c..75ad8b24f6cf 100644 --- a/vcl/source/outdev/font.cxx +++ b/vcl/source/outdev/font.cxx @@ -171,6 +171,22 @@ bool OutputDevice::GetFontFeatures(std::vector<vcl::font::Feature>& rFontFeature return true; } +bool OutputDevice::HasAatFeatures() const +{ + if (!ImplNewFont()) + return false; + + LogicalFontInstance* pFontInstance = mpFontInstance.get(); + if (!pFontInstance) + return false; + + const LanguageTag& rOfficeLanguage = Application::GetSettings().GetUILanguageTag(); + + std::vector<vcl::font::Feature> vFontFeatures; + vcl::font::FeatureCollector aFeatureCollector(pFontInstance->GetFontFace(), vFontFeatures, rOfficeLanguage); + return aFeatureCollector.hasAatFeatures(); +} + FontMetric OutputDevice::GetFontMetric() const { FontMetric aMetric; Does it seem reasonable or is there a simpler way? (of course, I may clean the duplicated code in OutputDevice::HasAatFeatures with a lcl function but it's just for the idea).
I’m not very up to date with the Kashida code, but I was thinking of something simpler, like in https://git.libreoffice.org/core/+/refs/heads/master/vcl/source/gdi/CommonSalLayout.cxx#687 always set GlyphItemFlags::IS_SAFE_TO_INSERT_KASHIDA flag if the font has AAT substitutions, so that down stream code does not require any changes. Since Jonathan Clark is actively working on this code, he is more suited to give an opinion here.
(In reply to خالد حسني from comment #5) > I’m not very up to date with the Kashida code, but I was thinking of > something simpler, like in > https://git.libreoffice.org/core/+/refs/heads/master/vcl/source/gdi/ > CommonSalLayout.cxx#687 always set GlyphItemFlags::IS_SAFE_TO_INSERT_KASHIDA > flag if the font has AAT substitutions, so that down stream code does not > require any changes. Since Jonathan Clark is actively working on this code, > he is more suited to give an opinion here. In my opinion: - Flag all glyphs as safe to insert kashida, as suggested here. - Update OutputDevice::GetWordKashidaPositions (vcl/source/outdev/font.cxx:1218) to leave the position vector empty for AAT fonts. - Update the fallback rule 8 (i18nutil/source/utility/kashida.cxx:297) to require a non-empty pValidPositions vector. We can't rely on VCL to filter out invalid positions for AAT fonts, so this fallback is not a safe default anymore. It shouldn't be necessary to change any code in sw or editeng.