Apache OpenOffice (AOO) Bugzilla – Issue 113301
hyphenation within Graphite ligatures can freeze OpenOffice.org
Last modified: 2010-09-07 11:58:44 UTC
I have tested the latest development version of OOo 3.3, and the hyphenation works within ligatures, too. I have found only a frequent positional problem here, see the attached screenshot. I was be able to fix it by modifying the character formatting of the related words, but now OpenOffice.org freezes, when I modify the character formatting the red characters only in the end of the line ("puff"). The attached test file uses the following OpenOffice.org extension: http://extensions.services.openoffice.org/en/node/3879 (Hungarian dictionaries, an older version with hyphenation within ffj) and the following Graphite font with the "ffj" and "ff" ligatures: http://www.numbertext.org/linux/
Created attachment 70728 [details] Hyphenation within the ffj ligatures with positional/kerning problem
Created attachment 70729 [details] test file
It would be interesting to know whether the problem still happens in the latest Graphite CWS (it is "approved by QA, but not integrated yet): http://eis.services.openoffice.org/EIS2/cws.ShowCWS?Path=DEV300%2Fgraphite03
Unfortunately the problem does seem to be present in CWS graphite03. Looking at it in the debugger, OutputDevice::GetTextWidth is being called with a zero length at a position in the middle of the paragraph. This seems to cause Graphite to try to use an out of bounds array index, which causes a crash, when I test it, rather than a freeze. The solution is probably to trap this case and avoid calling graphite, though I need to be careful that the rest of the GraphiteLayout can handle the consequences of not having a segment. The positioning issue with the hyphenated word being rendered assuming the width as if the ligature spanning the hyphenation point is still present will probably be much harder to fix.
Created attachment 70800 [details] patch to avoid calling graphite with zero length and avoid using cached segments stradling ligature
The patch prevents the crash by avoiding calling graphite when mnMinCharPos == mnEndCharPos. On windows the example document didn't crash when I changed the format of the highlighted text, but that seems to be because it happened to use an existing longer segment from the cache. The positioning bug appears to be due to the cache of Graphite Segments held by OOo. Normally, it is alright to use a cached segment which is for a longer string than was requested, in the CTL case such context is often necessary to get correct positions. However, in this case the cached segment had the ligature, whereas the requested text does not due to the hyphen break. The patch adds code to detect this case by querying the cached segment to determine if the character at mnEndCharPos-1 maps to a glyph and if so whether that glyph spans characters outside the requested range. If there is no glyph mapped for that character or the glyph is a ligature crossing the run boundary, then the cached segment is ignored and a new segment created with just the requested text. There seems to be a remaining problem with the hyphenation highlighted in red, because the top right hand end of the ff ligature is clipped by the rendering of the background for the hyphen. I haven't yet tracked down where the background is drawn from to see if it is possible to prevent that happening.
kstribley: I have tested your patch, and it works fine. Thanks, László Clipping by background color is a problem at character format boundaries, when I use white background colors for paragraphs to solve other typographical issues (bad repeating of left-top background image of a paragraph, when there is a page break within the paragraph).
SwTxtPainter::DrawTextLine calls the Paint method on each portion in the line in turn. sw/source/core/text/itrpaint.cxx line 421 For a hyphenated word, the text before the hyphen is an SwTxtPortion and so the paint is SwTxtPortion::Paint. sw/source/core/text/portxt.cxx line 575 The hyphen itself is derived from an SwExpandPortion, whose paint method is: sw/source/core/text/porexp.cxx line 108 SwExpandPortion::Paint Both of these paint methods call rInf.DrawBackBrush() before they paint the text. The way to avoid the clipping, would presumably be to separate the painting of the background (DrawBackBrush) and the painting of the text (DrawText). This would require 2 loops over the portions in the line, one for the background and one for the actual text. However, this might have a performance impact. This isn't graphite specific, so I suspect this would be better handled as a separate issue. @hdu are you aware of an existing issue for the clipping problem?
@md or ul: I suggest a OOo33 target for this issue. Ok? @kstribley: Thanks for analyzing the hang/crash and providing the patch to fix it. When the target is known I'll create a CWS graphite04 for it, feel free to push the fix there. Regarding the clipping problem this is probably already known as issue 56015 or issue 94493.
@hdu: From my point of view this is worth to be a stopper for 3.3 so I'm adding it to the meta issue and adjust it's target.
Good. As soon as OOO330_m2 (which will contain CWS graphite03) is released the CWS graphite04 will be created.
CWS graphite04 is created and its repository will show up in the next hour or so on http://hg.services.openoffice.org/cws/graphite04
@kstribley: do you prefer to commit+push the fix yourself or should I do it?
Applied in CWS graphite04
@hdu: thanks for applying the patch. I had problems with my connection today, so still waiting for the pull.
@sba: please verify in CWS graphite04
@es: please verify in CWS graphite04
@HDU: Cannot reproduce a crash or freeze in the master. Please verify per code review.
Ok in CWS graphite04.
Got into OOO330_m5 => closing