You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
99 lines
5.2 KiB
99 lines
5.2 KiB
From 71077148d442b3bbfeefd9a572942946c6a95823 Mon Sep 17 00:00:00 2001
|
|
From: Khaled Hosny <khaledhosny@eglug.org>
|
|
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
|
|
|