427666 - stretchy operators built from only glue (inc OverBar) are of incorrect length and off-center

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

[Karl Tomlinson (:karlt)]

Attachment: snapshot of testcase in url

Seems dependent on fonts.  Happens with STIX fonts.

Comment 1

[Damon Sicore (:damons)]

This is something we could take in a dot release.  MathML seems to be at a point where we could ship this and fix the smaller issues as soon as possible after we release.

Comment 2

[Karl Tomlinson (:karlt)]

There are two different bugs in the testcase.
Filed bug 428863 for the UnderBar.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: clip parts of stretchy chars more carefully

The problem was that some of the parts of stretchy characters where not being clipped carefully enough when near joins.  This tidies up the clipping to fix this bug and also Bug 349907.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is your AppUnitsPerDevPixel really important? I guess you're trying to handle a scale in the CTM? Why does that matter?

Comment 5

[Karl Tomlinson (:karlt)]

(In reply to comment #4)
> I guess you're trying to handle a scale in the CTM?

Yes.

> Why does that matter?

The oneDevPixel is used in calculating the positions of the origins for the glue glyphs.  If this is not (an integer multiple of) a cairo dev pixel, then the glue glyphs will be snapped to unexpected positions.  (Bounding metrics are already rounded to cairo dev pixels.)

Also it determines how much error might be in the bounding metrics due to cairo rounding out the glyph extents to (its) dev pixels, so we know how much to trim off.

Do we know there is no scale?  If so, then this is not necessary.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The only time we'll have a scale in the CTM right now is in SVG <foreignobject> situations. But then we might also have rotation or shearing in which case this approach isn't going to work anyway and I think we'll just have to live with the bug for these cases.

Comment 7

[Karl Tomlinson (:karlt)]

(In reply to comment #6)
> The only time we'll have a scale in the CTM right now is in SVG <foreignobject>
> situations. But then we might also have rotation or shearing in which case this
> approach isn't going to work anyway and I think we'll just have to live with
> the bug for these cases.

Yes, we'll just have to live with some glyph snapping issues if there is non-right-angle rotation, but AppUnitsPerDevPixel at least makes an estimate of the errors involved.  (For shearing it might work OK - I'm not certain.)
But we don't _need_ to live with problems when there is only scaling and right angle rotations.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

There's a code complexity cost here that I don't think is justified by improved cosmetic results on very obscure testcases, especially when there are slightly more obscure testcases that won't be fixed.

I suggest posting to the cairo list asking how this should/could be solved for the completely general case.

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: clip parts of stretchy chars more carefully without code for non-translation transformations

(In reply to comment #8)

> There's a code complexity cost here that I don't think is justified by
> improved cosmetic results on very obscure testcases, especially when there
> are slightly more obscure testcases that won't be fixed.

OK.  Removed the AppUnitsPerDevPixel function to reduce the complexity and skip
trying to do a best effort for non-translation transformations.

I've left SnapToDevPixels.  It feels just as easy to do the complete
transformation as it is to just pull out the translation in the appropriate
direction.

> I suggest posting to the cairo list asking how this should/could be solved for
> the completely general case.

I don't really want to start such a discussion now as I don't expect to
implement any significant change to the algorithm at this stage.  This patch
merely tweaks the existing algorithm to get better results.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

For posterity, the fully-general way to do this would be to draw the glyphs to a path and then fill the path. This would avoid all issues at glyph joins (assuming we can figure out how to position/overlap the glyphs correctly), although with current cairo you'd lose subpixel-AA and it could be a bit slow. So it's worth keeping the current code for the normal case.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 316729 [details] [diff] [review]
clip parts of stretchy chars more carefully without code for non-translation transformations

+      // it is, to trim off ends and get a solid edge for the join.

"so"

Would be nice if we could share code for horizontal and vertical drawing here. Not sure how feasible that is. Think about it.

Comment 13

[Karl Tomlinson (:karlt)]

(In reply to comment #12)
> (From update of attachment 316729 [details] [diff] [review])
> +      // it is, to trim off ends and get a solid edge for the join.
> 
> "so"

I'm not sure exactly where you're suggesting the "so".  How about:

+      // To protect against gaps, pretend the glue is smaller than it is,
+      // in order to trim off ends and thus get a solid edge for the join.

> Would be nice if we could share code for horizontal and vertical drawing here.
> Not sure how feasible that is. Think about it.

Yes.  Would be nice.  In fact these were once one function with many condition blocks.  Maybe something could be done by setting up common references to the appropriate variables.  It may make things harder to understand - not sure.  I'd rather try that in a separate patch without making other changes at the same time though.

Comment 14

[Karl Tomlinson (:karlt)]

Comment on attachment 316729 [details] [diff] [review]
clip parts of stretchy chars more carefully without code for non-translation transformations

Requesting approval1.9.

The effects of this are limited to MathML stretchy operators.  To test these
effects I've visually checked for regressions (and for fixing bug 349907)
on these sites

http://www.maths-informatique-jeux.com/international/mathml_test/accents.xml
http://www.mozilla.org/projects/mathml/start.xhtml
http://www.mozilla.org/projects/mathml/demo/texvsmml.xhtml
attachment 316720 [details]

in these situations

* on Linux with full hinting and without hinting
* on Windows with ClearType and standard antialiasing
* on Mac with light and strong font smoothing

I also have a reftest to land and so ensure that this bug doesn't regress.

Comment 15

[Damon Sicore (:damons)]

Comment on attachment 316729 [details] [diff] [review]
clip parts of stretchy chars more carefully without code for non-translation transformations

a1.9+=damons

Comment 16

[Karl Tomlinson (:karlt)]

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1208920203&maxdate=1208920385&who=karlt%2B%25karlt.net