Apache OpenOffice (AOO) Bugzilla – Issue 111054
formatting borked for expanded text with Graphite fonts (spacing, line width)
Last modified: 2017-05-20 11:42:04 UTC
(Feel free to come up with a more descriptive title.) A lot of my documents that were originally written using the 3.0 and 3.1 series have totally broken formatting when opened in 3.2. Specifically the line length seems to get incrementally shorter with every line in a paragraph down to about the middle, then it gets longer again. So the shape of the right border of each paragraph looks like this: < The effect carries over when printing or exporting as PDF. This doesn't happen with all documents but seems to be related to the Gentium family of fonts and / or the fact that I like to use wider spacing (+1/2 point). See attachments.
Created attachment 69036 [details] sample file that exhibits the problem (excerpt)
Created attachment 69037 [details] that's how it should (used to) render
Created attachment 69038 [details] that's how it renders in 3.2
I can reproduce the error with a new document and the font "Gentium Book Basic" in DEV300m76. It does not occur with other fonts, for example "Palatino Linotype", "Garamond", or "Dejavu Serif".
@Regina: Thank you for your quick analyze! :) @HDU: a consequence of issue 89682 implementation?
Some more testing: Gentium / GentiumAlt - OK Gentium Basic / Gentium Book Basic - FAIL The only fundamental difference between these two fonts seems to be that the latter do have Graphite tables while the former do not (as of yet). So I tried another Graphite font: Charis SIL - FAIL Since a lot of "plain" TTF and OTF fonts (both formats) are fine that would indicate there is either - something wrong with SIL's Graphite enabled fonts or - something wrong with 3.2's Graphite implementation. Does anyone know which font the Graphite support was tested against?
Setting SAL_DISABLE_GRAPHITE=1 helps. Personally I can live without Granite but it's still an ugly workaround.
adjusted summary
It seems to be the Spacing expanded by 0.5pt (on the Position tab) which is doing it. If Spacing is Normal it goes back to rendering correctly. To be honest, I don't think I ever tested this combination. I will look into in more detail, but it may take several days. I notice that if the font is changed to Padauk, which is another Graphite font, it does render sensibly. Padauk isn't a good font for German since it doesn't have umlauts, but perhaps it is related to Graphite's line breaking algorithm, which will depend on the rules in the font.
It seems to be the Spacing expanded by 0.5pt (on the Position tab) which is doing it. If Spacing is Normal it goes back to rendering correctly. To be honest, I don't think I ever tested this combination. I will look into in more detail, but it may take several days. I notice that if the font is changed to Padauk, which is another Graphite font, it does render sensibly. Padauk isn't a good font for German since it doesn't have umlauts, but perhaps it is related to Graphite's line breaking algorithm, which will depend on the rules in the individual font.
@kstribley: thanks for looking into this! Indeed, text breaking for expanded or condensed spacing already caused enough trouble for other layout engines in OOo. The severity of this issue is reduced by the small percentage of documents using such non-zero horizontal extra-spacing.
Created attachment 69132 [details] Line breaking patch for expanded condensed graphite text
The problem was in the GraphiteLayout::GetTextBreak method. It subtracted the extra character spacing for all glyphs in a run before computing where the Line Break point lay. If the number of characters being tested was much more than the number that could fit, this resulted in line breaking occurring in a much narrower width than it should have been. Originally, I was using Graphite's LineFillSegment object to calculate the line break point, however, this is not possible when extra spacing is being applied per character. I've therefore decided to record the break weights from the Graphite segment when it is first analyzed in the appendCluster method. These can then be used to compute an appropriate break point even when there is extra character spacing. The attached patch fixes the issue for me on Linux and I hope to test it some more on Windows later in the week. It also includes a fix for incorrect rendering of expanded/condensed Graphite text in the preview window of the Character dialog (the expansion/condensation wasn't being applied). PS: I think my previous comment about Padauk being OK was erroneous. Perhaps fallback was confusing the issue.
Thanks! The change looks quite tricky... if it solves both text breaking and displaying non-default spaced text all is well. FWIW, I had a similar problem with text breaking on the Aqua port in the method GetTextBreak() (in vcl/aqua/source/gdi/salatslayout.cxx) which uses native OSX text breaking methods to emulate the unique text breaking semantics expected by Writer and EditEngine. This was be solved by just adapting GetTextBreak(). At the root of the problem there is of course SW and EE having these very un-conventional expectations (issue 108684).
Created attachment 69248 [details] Revised patch fixing justification regression, includes fix for issue 111272
I've noticed that if expanded text is used with Arabic, then Kashida's are introduced for large expansions. This seems to affect both the Graphite code and the ICU code on Linux. Is that actually correct? I don't know any Arabic, but if I compare it with where they are inserted for justification they don't look to be the same positions. @hdu are you able to comment on this?
> if expanded text is used with Arabic, then Kashida's are introduced for large expansions Yes, for shaping engines that do not bother with topics such as justification OOo has a layer to emulate it on top of the non-justfied layout. This introduces kashidas for scripts that use that method for justification. Graphite does justification itself so the justification-emulation layer should become active. The concept of expanded/condensed spacing in WE/EE itself is a bit dubious and for cursive scripts it IMHO does not make any sense at all. WriterEngine/EditEngine should then either disallow it or they should accumulate the expanded space into the locations suggested by normal justification for this script, i.e. into spaces or into the kashida positions.
correction: Graphite does justification itself so the justification-emulation layer should NOT become active. I hope the issue tracker will allow to edit comments one day...
I split off issue 111336 for the problem I described above (with cursive scripts and non-zero spacing).
The patch looks good to me (for CWS graphite02), thanks! @kstribley: should I hgpush+commit the latest patch as it is or do you prefer to commit it yourself? CWS graphite02 should probably also disable the justification emulation layer (described in #desc18) for Graphite layouts if graphite prefers to do the justification itself. Due to Writer writer having unusual opinions on the division of responsibility between the code layers used for justification (issue 108684) this might become tricky though. Feel free to split off an issue for that topic.
Regarding the topic of the justification emulation layer I mentioned in #desc18 and #desc21: This can be disabled by overriding the AdjustLayout() method from class derived from SalLayout. I saw that GraphiteLayout::AdjustLayout() already does that, but it mostly copied the code from the generic SalLayout justification emulation layer. So the kashidas noticed in #desc17 come from GraphiteLayout::kashidaJustify() instead of the GenericSalLayout::KashidaJustify() which I suspected first. If graphite prefers to do the justification itself just forward the justification request in AdjustLayout() to e.g. gr::Segment::JustifiedSegment().
Applied the second patch into CWS graphite02.
@sba: please verify in CWS graphite02
*** Issue 111277 has been marked as a duplicate of this issue. ***
SBA->ES: Please take over, thx.
Verified in CWS graphite02