74821 - Implement GetBoundingMetrics() on the Mac

Status: RESOLVED FIXED

(Core :: MathML, defect)

Description

[rbs]

Currently GetBoundingMetrics() is only a stub on the Mac, returning a failure
exit status. While the MathML code is compiling fine, and is gracefully handling 
this error condition, the ultimate effect is that the quality of the rendering
leaves a lot to be desired. E.g., stretchy characters don't stretch.
GetBoundingMetrics() is a vital piece of MathML rendering and has to be 
implemented for the rendering to work as expected.

The definition of GetBoundingMetrics() is given below:
http://lxr.mozilla.org/seamonkey/source/gfx/public/nsIRenderingContext.h#737

The structure that GetBoundingMetrics() computes is given below:
http://lxr.mozilla.org/seamonkey/source/gfx/public/nsIRenderingContext.h#830

Comment 1

[rbs]

Cc:ing peterv who started something but had troubles with the implementation.
Peter, feel free to take the bug and attach any patch on your progress so far.

Comment 2

[rbs]

Re-assigning from myself to nobody. I don't have a Mac. 

Comment 3

[Peter Van der Beken]

Unfortunately, I seem to have lost the implementation I had in my move. I did
find back some of the notes I made while implementing, so I'll try to reconstruct.

Comment 4

[Steve Dagley]

Removing Blocker severity since it hasn't been updated since April

Comment 5

[Steve Dagley]

Is this Mac OS X specific?

Comment 6

[rbs]

No, all flavors of Mac need this.

Comment 7

[rbs]

Peter, were you able to dig the patch of your earlier attempt? You might want to 
attach it to the bug in the case where someone is interested to try it out.

Comment 8

[Simon Fraser [no longer active]]

This should be doable with ATSUI, and maybe doable with some newish font manager
APIs.

Comment 9

[Daniel S. Rogers]

There appear to be two functions for measuring typographic bounding boxes of
unicode text in the ATSU.

ATSUMeasureText:
http://developer.apple.com/techpubs/macosx/Carbon/text/ATSUI/Apple_Type_S_code_Imaging/Functions/Measuring_Ty_Image_Bounds.html#//apple_ref/C/func/ATSUMeasureText
is used to measure bounding boxes before final line layout.

ATSUGetGlyphBounds:
http://developer.apple.com/techpubs/macosx/Carbon/text/ATSUI/Apple_Type_S_code_Imaging/Functions/Measuring_Ty_Image_Bounds.html#//apple_ref/C/func/ATSUGetGlyphBounds
is used to measure bounding boxes after final line layout.

Both return ascent and descent and the amount that the line extends before and
after the origin of the line in the current graphics port.  (I have a feeling
though that ATSUMeasureText is more of what we are interested it).

I am still working backwards through the code to determine how to acquire all
the arguments needed for one of the functions (not to mention which one is the
one to use).  I can't guarantee that I will get an implementation anytime as
school takes priority, but I will work on it.

Comment 10

[Greg K.]

Should this bug be marked dependent on bug 121540?

Comment 11

[rob raguet-schofield]

Attachment: patch to get bounding metrics

here is a patch to get bounding metrics on mac platforms.  it works on osx, but
i didn't try on classic.  the documentation states that it should work back to
8.5.

this only handles the unicode case, but everything seems to be working ok.  i
could implement a non-unicode case using OutlineMetrics(), but i'm not sure
there's a need for it right now and it would be a bit uglier.

next i'll try to determine why the mathematica fonts don't seem to be working
on for me osx.	if i can get that figured out mathml should be good to go...

Comment 12

[rbs]

Great contribution of rob (from Wolfram Research -- makers of Mathematica) on a
long drawn out problem...

Mac people (sfraser, peterv, sdagley, hsivonen, etc) care to chime in & give the
patch a swirl...

> this only handles the unicode case, but everything seems to be working ok.  i
> could implement a non-unicode

This would be desirable in order to support the symbolic fonts such as TeX fonts
or Mathematica fonts. Do the APIs in comment #9 not appropriate for this case?

Comment 13

[Greg K.]

Adding patch and review keywords. MathML for Mac Mozilla (bug 155703) needs bug
107146 fixed in addition to this one.

Comment 14

[Simon Fraser [no longer active]]

Comment on attachment 91155 [details] [diff] [review]
patch to get bounding metrics

All that font fallback rendering code is a real mess, and needs cleaning up,
but you seemed to be able to find your way around it OK.

The code looks fine. Some of it appears to have tabs, which should be removed.
Fix that, and sr=sfraser

Comment 15

[rbs]

Not sure if this is Mac quirk, but when I click the link to view the patch on 
Win2K, it appears as truncated, e.g., it starts with:

#7AA23.diff((err = ATSUMeasureTextImage(aTxtLayout, 
+               kATSUFromTextBeginning, kATSUToTextEnd, 0, 0, &rect)) != noErr)

Comment 16

[rbs]

Attachment: same patch - cvs diff'ed with a Win2K box

Comment 17

[rbs]

> > this only handles the unicode case, but everything seems to be working ok.
> > i could implement a non-unicode
>
> This would be desirable in order to support the symbolic fonts

ignore this comment... we are not talking about the same thing. You were 
referring to the |char*| version of GetBoundingMetrics... Indeed, nobody uses 
that (as yet) since MathML with its &alpha, β, etc, is essentially Unicode.
Moreover, if someone really wants the |char*| version, they can recover it by 
first copying their |char*| to a |PRUnichar*|. In other words, there is no 
particular urgency for the |char*| version at the moment, and after the long 
wait, it is already encouraging to see a working |PRUnichar*| version at all.

So yes, what you have now is sufficient for today's needs.

> next i'll try to determine why the mathematica fonts don't seem to be working
> on for me osx.  if i can get that figured out mathml should be good to go...

This is where ucvmath comes into play, and that's bug 107146 which is the other 
aspect needed to allow recognizing & special-casing the symbolic fonts.
http://www.mozilla.org/projects/mathml/enable.html

Comment 18

[rob raguet-schofield]

Attachment: original patch w/o tabs

the tabs should be gone now, sorry about that.

Comment 19

[Simon Fraser [no longer active]]

The diff was probably attached with Mac IE, which uses AppleSingle format.
Luckly, on Mac, I can still see all the text (so my review is valid ;)

Comment 20

[rbs]

That was indeed what I suspected it -- that you folks on the Mac were seeing
things okay, whereas those of us on other platforms could be seeing funny things.

Patch looks okay to me. I am going to give r=rbs, and request a= to see if
drivers buy it. With this code (and attachment 91293 [details] [diff] [review] on bug 155703), MathML will
be close to go on the Mac. Subscripts, superscrips, fractions, etc, will display
properly. However, stretchy characters (e.g., radical symbols, large
parentheses, etc, http://www.mozilla.org/projects/mathml/screenshots/sqrt.gif)
won't work until bug 107146 is fixed. While fixing that bug, there may be some
contingent changes back in the bounding metrics (e.g., to support symbolic
fonts), but these may arise as a matter of necessity and shouldn't block this
patch. If I was close to rob, fixing that bug could be much easier for him, but
since I don't have a mac, I can only hope you the best, and provide feedback as
needed.

Something that you didn't do was to wrap the newly added helper functions into
#ifdef MOZ_MATHML, but that can be trivially added since there aren't much
entanglements. In fact, I already went ahead and just did it (tm) to facilitate
the checkin. I am going to attach the patch that is ready to be checked in once
clearance is given (either by drivers's approval or after 1.1 is branched).

-> re-assigning to rob who fixed the bug.

Comment 21

[rbs]

Attachment: patch to be checked in

Comment 22

[rbs]

Comment on attachment 91452 [details] [diff] [review]
patch to be checked in

flagging with r=rbs & sr=sfraser

Comment 23

[rbs]

Attachment: slight change -- first time must init so that the left-bearing is set

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 91483 [details] [diff] [review]
slight change  -- first time must init so that the left-bearing is set

a=asa (on behalf of drivers) for checkin to 1.1

Comment 25

[rbs]

Fixed on the trunk.