Caolán McNamara
11 years ago
parent
5ed5bf4e52
commit
7625f21a72
@ -0,0 +1,98 @@
|
||||
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
|
||||
|
||||
Loading…
Reference in new issue