From 7625f21a72ea613ccedb43e37352fb6c51cae4f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= Date: Thu, 31 Oct 2013 10:59:18 +0000 Subject: [PATCH] Resolves: rhbz#1025201 Incorrect rendering of Devanagari short i --- ...ect-rendering-of-Devanagari-short-i-.patch | 98 +++++++++++++++++++ libreoffice.spec | 5 +- 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 0001-fdo-70968-Incorrect-rendering-of-Devanagari-short-i-.patch diff --git a/0001-fdo-70968-Incorrect-rendering-of-Devanagari-short-i-.patch b/0001-fdo-70968-Incorrect-rendering-of-Devanagari-short-i-.patch new file mode 100644 index 0000000..391227f --- /dev/null +++ b/0001-fdo-70968-Incorrect-rendering-of-Devanagari-short-i-.patch @@ -0,0 +1,98 @@ +From 71077148d442b3bbfeefd9a572942946c6a95823 Mon Sep 17 00:00:00 2001 +From: Khaled Hosny +Date: Wed, 30 Oct 2013 09:34:38 +0200 +Subject: [PATCH] fdo#70968: Incorrect rendering of Devanagari short 'i' vowel + +It seems that some Indic fonts assign 'mark' glyph class to combining +spacing marks (spacing not non spacing) so my reliance on the glyph +class to set the IS_DIACRITIC flags broke those fonts. This is a bandaid +to get around the issue, plus some long rant! (at this rate, I'll be +writing "The VCL haters handbook" pretty soon). + +Change-Id: I3ff892acf746d50182573f94e7e8c3c6f9464ae0 +--- + vcl/generic/glyphs/gcach_layout.cxx | 26 +++++++++++++++++++++----- + vcl/source/gdi/sallayout.cxx | 21 +++++++++++++++++++++ + 2 files changed, 42 insertions(+), 5 deletions(-) + +diff --git a/vcl/generic/glyphs/gcach_layout.cxx b/vcl/generic/glyphs/gcach_layout.cxx +index 4673931..7a8bfc9 100644 +--- a/vcl/generic/glyphs/gcach_layout.cxx ++++ b/vcl/generic/glyphs/gcach_layout.cxx +@@ -456,20 +456,36 @@ bool HbLayoutEngine::layout(ServerFontLayout& rLayout, ImplLayoutArgs& rArgs) + if (bInCluster) + nGlyphFlags |= GlyphItem::IS_IN_CLUSTER; + ++ // The whole IS_DIACRITIC concept is a stupid hack that was ++ // introduced ages ago to work around the utter brokenness of the ++ // way justification adjustments are applied (the DXArray fiasco). ++ // Since it is such a stupid hack, there is no sane way to directly ++ // map to concepts of the "outside" world, so we do some rather ++ // ugly hacks: ++ // * If the font has a GDEF table, we check for glyphs with mark ++ // glyph class which is sensible, except that some fonts ++ // (fdo#70968) assign mark class to spacing marks (which is wrong ++ // but usually harmless), so we try to sniff what HarfBuzz thinks ++ // about this glyph by checking if it gives it a zero advance ++ // width. ++ // * If the font has no GDEF table, we just check if the glyph has ++ // zero advance width, but this is stupid and can be wrong. A ++ // better way would to check the character's Unicode combining ++ // class, but unfortunately glyph gives combining marks the ++ // cluster value of its base character, so nCharPos will be ++ // pointing to the wrong character (but HarfBuzz might change ++ // this in the future). + bool bDiacritic = false; + if (hb_ot_layout_has_glyph_classes(mpHbFace)) + { + // the font has GDEF table +- if (hb_ot_layout_get_glyph_class(mpHbFace, nGlyphIndex) == HB_OT_LAYOUT_GLYPH_CLASS_MARK) ++ bool bMark = hb_ot_layout_get_glyph_class(mpHbFace, nGlyphIndex) == HB_OT_LAYOUT_GLYPH_CLASS_MARK; ++ if (bMark && pHbPositions[i].x_advance == 0) + bDiacritic = true; + } + else + { + // the font lacks GDEF table +- // HACK: if the resolved glyph advance is zero assume it is a +- // combining mark. The whole IS_DIACRITIC concept is a hack to +- // fix the other hacks we use to second-guess glyph advances in +- // ApplyDXArray and the likes and it needs to die + if (pHbPositions[i].x_advance == 0) + bDiacritic = true; + } +diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx +index f395936..450ec232 100644 +--- a/vcl/source/gdi/sallayout.cxx ++++ b/vcl/source/gdi/sallayout.cxx +@@ -1018,6 +1018,27 @@ void GenericSalLayout::AdjustLayout( ImplLayoutArgs& rArgs ) + + // ----------------------------------------------------------------------- + ++// This DXArray thing is one of the stupidest ideas I have ever seen (I've been ++// told that it probably a one-to-one mapping of some Windows 3.1 API, which is ++// telling). To justify a text string, Writer calls OutputDevice::GetTextArray() ++// to get an array that maps input characters (not glyphs) to their absolute ++// position, GetTextArray() in turn calls SalLayout::FillDXArray() to get an ++// array of character widths that it converts to absolute positions. ++// ++// Writer would then apply justification adjustments to that array of absolute ++// character positions and return to OutputDevice, which eventually calls ++// ApplyDXArray(), which needs to extract the individual adjustments for each ++// character to apply it to corresponding glyphs, and since that information is ++// already lost it tries to do some heuristics to guess it again. Those ++// heuristics often fail, and have always been a source of all sorts of weird ++// text layout bugs, and instead of fixing the broken design a hack after hack ++// have been applied on top of it, making it a complete mess that nobody ++// understands. ++// ++// As you can see by now, this is utterly stupid, why Writer does not just send ++// us directly the advance width transformations it wants to apply to each ++// character instead of this whole mess? ++ + void GenericSalLayout::ApplyDXArray( ImplLayoutArgs& rArgs ) + { + if( m_GlyphItems.empty()) +-- +1.8.3.1 + diff --git a/libreoffice.spec b/libreoffice.spec index e2df7fa..ca3d128 100644 --- a/libreoffice.spec +++ b/libreoffice.spec @@ -265,6 +265,7 @@ Patch24: 0001-Related-rhbz-1020712-wrong-default-font-shown-in-edi.patch Patch25: 0001-Related-rhbz-919070-display-1-means-span-all-display.patch Patch26: 0001-fdo-67725-unoidl-AggregatingCursor-must-wrap-modules.patch Patch27: 0001-Resolves-rhbz-1021915-force-menubar-menus-to-be-up-d.patch +Patch28: 0001-fdo-70968-Incorrect-rendering-of-Devanagari-short-i-.patch %define instdir %{_libdir} %define baseinstdir %{instdir}/libreoffice @@ -1024,6 +1025,7 @@ mv -f redhat.soc extras/source/palettes/standard.soc %patch25 -p1 -b .rhbz-919070-display-1-means-span-all-display.patch %patch26 -p1 -b .fdo-67725-unoidl-AggregatingCursor-must-wrap-modules.patch %patch27 -p1 -b .rhbz-1021915-force-menubar-menus-to-be-up-d.patch +%patch28 -p1 -b .fdo-70968-Incorrect-rendering-of-Devanagari-short-i-.patch # TODO: check this # these are horribly incomplete--empty translations and copied english @@ -2115,9 +2117,10 @@ update-desktop-database %{_datadir}/applications &> /dev/null || : %endif %changelog -* Tue Oct 29 2013 Stephan Bergmann - 1:4.1.3.2-2-UNBUILT +* Thu Oct 31 2013 Stephan Bergmann - 1:4.1.3.2-2 - Resolves: fdo#67725 unoidl::AggregatingCursor must wrap modules for aggregation - Resolves: rhbz#1021915 force menubar menus to be up/down only +- Resolves: rhbz#1025201 Incorrect rendering of Devanagari short i * Wed Oct 23 2013 David Tardon - 1:4.1.3.2-1 - 4.1.3 rc2