[Libreoffice] Horizontal glyph adjustments are ignored with ICU layout

[Michael Meeks]

Hi Khaled,

On Wed, 2011-01-05 at 15:54 +0200, Khaled Hosny wrote:
> This my first message to the list, hopping it will be a fruitful
> experience :)

	Drat; we took ages to reply. I suspect Thorsten might be able to help,
but failing that the best text expert I'm aware of is Tim (CC'd) who may
be able to give some advice. SIL's Graphite integration would
undoubtedly have fallen over the same sort of problems, and no doubt he
can give some pointers to how to work around it - Tim ? :-)

	HTH,

		Michael.

> I was trying to debug fdo#31016[1] since it breaks almost all my fonts,
> it toke me few months but I think I finally got the root of it (I
> started working on it few weeks before libreoffice was announced, what a
> nice code base ;).
> 
> Anyway, it turned out that the issue is not specific to kerning nor
> Arabic, but affects all horizontal glyph positioning in the ICU layout
> path; the problem does not show on Windows nor with Graphite fonts and
> of course not with con-CTL.
> 
> The X adjustment of glyph widths is simply ignored, and glyphs are drawn
> stacked after each other baased on their original width, which results in
> kerning being ignored as well as other forms of glyph positioning like
> cursive anchors, however vertical glyph positions (in the Y access) are
> OK.
> 
> In source/glyphs/gcach_layout.cxx, ICU's layoutChars() is called and new
> glyph indices and positions are returned, which is then fed into
> SalLayout in the form of GlyphItem's. Though GlyphItem has maLinearPos
> which holds its absolute position, many places in the code re-calculate
> glyph position from its mnOrigWidth (original glyph width) and mnNewWidth
> (width after adjustments), but the ICU path simply sets mnNewWidth to
> mnOrigWidth since ICU layout engine does not return individual glyph
> widths, resulting in failure of glyph position re-calculation which
> manifests in this bug.
> 
> As a prove of concept, the attached patch trays to set mnNewWidth in a
> very hackish way by subtracting current glyph position from the of next
> non-diacritic (+ve) glyph. It does indeed fix the problem (see the
> attached screenshots), but it looks very unreliable to me. May be a
> cleaner solution is to re-implement all the broken virtual methods to
> eliminate the re-calculation part.
> 
> Hope this helps, and sorry for the broken terminology as I'm not really
> a developer and less of C++ one (I can only do much of C++ that looks
> like C, which I don't really "speak" either).
> 
> [1] https://bugs.freedesktop.org/show_bug.cgi?id=31016

-- 
 michael.meeks at novell.com  <><, Pseudo Engineer, itinerant idiot