Issue 111392 - Glyphs within a cluster (cell) are rendered on top of each other when drawn along an arc with FontWork
Summary: Glyphs within a cluster (cell) are rendered on top of each other when drawn a...
Status: CLOSED FIXED
Alias: None
Product: Draw
Classification: Application
Component: ui (show other issues)
Version: DEV300m73
Hardware: All All
: P3 Trivial (vote)
Target Milestone: OOo 3.3
Assignee: wolframgarten
QA Contact: issues@graphics
URL:
Keywords:
Depends on: 111460
Blocks:
  Show dependency tree
 
Reported: 2010-05-05 11:22 UTC by devel
Modified: 2017-05-20 10:23 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
Document showing bug (20.41 KB, application/vnd.oasis.opendocument.graphics)
2010-05-05 11:24 UTC, devel
no flags Details
bad layout of 2nd glyph in each cluster (25.34 KB, application/pdf)
2010-05-05 11:25 UTC, devel
no flags Details
Patch adjusting the DX array offset and scaling (3.01 KB, patch)
2010-05-05 11:31 UTC, devel
no flags Details | Diff
correct rendering of text on path (25.41 KB, application/pdf)
2010-05-05 11:32 UTC, devel
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description devel 2010-05-05 11:22:28 UTC
Text on a curve containing multiple spacing glyphs in a cell cluster has all the
glyphs within a cell drawn on top of each other. This is fine if the glyphs have
the same x-offset, but not if they are spacing marks.

The attached file shows an example using 2 different Myanmar fonts. One uses
Graphite for rendering, the other OpenType and both exhibit the same bug
indicating it is not a problem with the rendering engine. The text consists of 8
code points in 4 cells of 2 glyphs each.

Fonts are available from:

http://scripts.sil.org/Padauk
http://www.myanmarnlp.net.mm/

Further analysis suggests that it is due to an incorrectly scaled DXArray which
is rounded to have zero values as passed in from
drawinglayer/source/processor2d/vclprocessor2d.cxx 
VclProcessor2D::RenderTextSimpleOrDecoratedPortionPrimitive2D(). Indeed, the bug
goes away if the DX array is disabled. 

However, I think the real bug is in svx/source/svdraw/svdotextpathdecomposition.cxx 
where the dx array needs to have the cumulative offset from preceding clusters
subtracted and then be scaled by the font size and scale factor. Example patch
to follow.

I'm not sure where else svdotextpathdecomposition is called from - I hope this
fix won't upset other areas of the code.
Comment 1 devel 2010-05-05 11:24:03 UTC
Created attachment 69304 [details]
Document showing bug
Comment 2 devel 2010-05-05 11:25:25 UTC
Created attachment 69305 [details]
bad layout of 2nd glyph in each cluster
Comment 3 devel 2010-05-05 11:31:14 UTC
Created attachment 69306 [details]
Patch adjusting the DX array offset and scaling
Comment 4 devel 2010-05-05 11:32:31 UTC
Created attachment 69307 [details]
correct rendering of text on path
Comment 5 wolframgarten 2010-05-05 12:18:57 UTC
Reassigned. Please handle.
Comment 6 Armin Le Grand 2010-05-05 13:25:53 UTC
AW: The patch looks promising, and it's at the right place. Thanks to kstribley,
this is a good addition.
Unfortunately, i cannot reproduce to really check what happens and to see the
text in alive code. when i install the fonts on WIN32, a DEV300 m76 crashes as
soon as font is accessed (either tools/options/languageoptions or Draw/Impress
and scrolling through the font selection DropDown). Asked HDU to check if he
gets the fonts working...

BTW: At the place for the patch the aNewDXArray only would need to be prepared
once i opt to optimize this in the same change...
Comment 7 hdu@apache.org 2010-05-05 14:02:20 UTC
Wonderful analysis of the problem, thanks Keith! +1 for the patch from me. FWIW the problem also 
happens for OOO320.
AW: the crash you are seeing is unrelated. It is probably stacktid 811965 aka #161963#
Comment 8 Armin Le Grand 2010-05-05 14:28:28 UTC
AW->HDU: Please do not simply use 'AW:', this is thought to indicate what i
said, thanks. At least use @aw, or HDU->AW to make things clear.

Installed a DEV300m77 and the crash is not there. Can i have the fix to work
with my m76 to fix this task?

AW: I cannot simply add this patch, i need to check if the also applied scaling
of the DXArray is correct or not. AFAIK the TextSimplePortionPrimitive2D takes
it's DXArray in scale-independent in unit coordinates; the scaling should be
part of the transformation.
Comment 9 Armin Le Grand 2010-05-06 14:25:04 UTC
AW: Looking in resynched version (m77) now, re-installing fonts, added to CWS
aw081...
Comment 10 Armin Le Grand 2010-05-06 14:34:39 UTC
AW: One argument for the offset being correct is that i do the same in
TextDecoratedPortionPrimitive2D::impSplitSingleWords where the words are split
in preparation of the decomposition of a TextDecoratedPortionPrimitive2D into
TextSimplePortionPrimitive2D's, so this seems correct. It did not show up in
western cases since here each character breaks to just one DXArray entry which
is then not really influencing the text position at all when rendering. It looks
different with two or more entries per character, though.
OTOH i think the scaling is wrong; it's already added to the transformation
building the base for the TextSimplePortionPrimitive2D.
Checking further...
Comment 11 Armin Le Grand 2010-05-06 14:44:59 UTC
AW->HDU: I am now on a DEV300m77 WIN32 nonpro, and when the fonts are installed,
office crashes when loading the bugdoc, i'll sent You a stack per eMail.
Removing the fonts allows document loading without a crash (as proof). What to
do...?
Comment 12 hdu@apache.org 2010-05-06 15:11:59 UTC
@aw: see issue 110806 and its patch: stlport_debug asserts about a dubious iterator manipulation. On a 
non-pro this iterator manipulation results in the same code as the patched code, though.
Comment 13 Armin Le Grand 2010-05-06 17:39:56 UTC
AW: Checked against #i100514# where i cleanedup FontScaling and DxArray
handling. The current description at TextSimplePortionPrimitive2D for maDXArray
is wrong; it does not contain FontScaling independent unit sizes, but logical
sizes. This is handled correctly in the renderers and from text primitive
creators throughout the office, but not from impDecomposePathTextPrimitive where
i seem to have forgotten to change it when cleaning up. This did not show up
since for western fonts always single chars are created where it has no
influence at all with the unified values only used as start values.

I need to change the description at TextSimplePortionPrimitive2D for maDXArray
and adapt DXArray handling in impDecomposePathTextPrimitive. Then the
translation part of the patch is okay and it is clear why the scaling part was
currently needed...
Comment 14 Armin Le Grand 2010-05-07 15:31:06 UTC
AW: To be able to work, i have temporarily commented out
vcl/source/glyphs/graphite_layout.cxx ln 248 where the non-pro DEV300 m77 still
crashes.

AW: Have now sorted out the problem. The FontSize (part of transformation in
text primitive) and the DXArray are independent as described in #i100514#.
Adapted description in TextSimplePortionPrimitive2D accordingly and extensive to
explain that.
To get impDecomposePathTextPrimitive working two things are needed to adapt the
DXArray from the flat layouted text coming from EditEngine in DrawPortionInfos.
First, when splitting in text portions, the right part of the original needs to
be extracted and translated to the start offset of the portion (as in the
patch). Second, if XFT_AUTOSIZE is used to scale the text to the curce length,
this has to be applied to the DXArray, too, but not the FontScaling tself as
described in the patch.
It is also necessary to correct impPathTextPortion::impPathTextPortion where
(overlooked from #i100514#) the scaling of theDXArray was resetted to unit
coordinates.
All of this did not show up in western languages since the portion length is
alwas 1 there; thus neither a wrong offset nor a wrong scaling in a DXArray took
any effect. A DXArray only makes any effect when the portion is more than 1
character.

Thanks go to kstribley for finding the problem at all. Seems like not too many
people use CTL and the old FontWork (better called: Text on curve).
Comment 15 Armin Le Grand 2010-05-07 15:39:37 UTC
AW: Added two more changes:
- Create no DXArray at all for 1 == GlyphLengths
- Create it only once for shadow and/or text on curve

Checking in changes, done.
Comment 16 Armin Le Grand 2010-05-27 13:33:19 UTC
AW: Adapted target
Comment 17 Armin Le Grand 2010-05-28 11:38:38 UTC
AW: Checked in CWS aw081, works as expected.
AW->WG: Please verify.
Comment 18 wolframgarten 2010-06-02 10:18:04 UTC
Verified in CWS.