Use OpenType MATH constant AxisHeight for math tables

• Component: MathML
• Status: RESOLVED FIXED

[Frédéric Wang (:fredw)]

Currently, the MathML code seems to use xHeight/2 as an approximation of the math axis (from the MathML spec: the axis of an equation is an alignment line used by typesetters. It is the line on which a minus sign typically lies):

fred@debian:~/webkit/WebKit/Source/WebCore/rendering/mathml$ grep "xHeight()" *
RenderMathMLBlock.cpp:        lengthValue = floatValue * style->fontMetrics().xHeight();
RenderMathMLBlock.cpp:    return (logicalHeight() + style().fontMetrics().xHeight()) / 2;
RenderMathMLFraction.cpp:        return denominatorWrapper->logicalTop() + static_cast<int>(lroundf((m_lineThickness + style().fontMetrics().xHeight()) / 2));
RenderMathMLOperator.cpp:        LayoutUnit axis = style().fontMetrics().xHeight() / 2;
RenderMathMLScripts.cpp:    LayoutUnit axis = style().fontMetrics().xHeight() / 2;

This should probably be handled in a separate "GetMathAxis" function to clarify that we use the math axis. Moreover when a MATH table is available, we should use OpenTypeMathData::AxisHeight to be more accurate.

[Frédéric Wang (:fredw)]

This, as well as using 
OpenTypeMathData::FractionRuleThickness as the default thickness in RenderMathMLFraction.cpp should now be relatively easy. See how it is done for  OpenTypeMathData::DisplayOperatorMinHeight in RenderMathMLOperator.cpp.

(see the desc of the MATH table in http://mpeg.chiariglione.org/standards/mpeg-4/open-font-format/text-isoiec-cd-14496-22-3rd-edition)

[Frédéric Wang (:fredw)]

This is done in Alex's MathMLLayout branch.

[Frédéric Wang (:fredw)]

Created attachment 274414 [details]
Patch

After the refactoring of RenderMathMLFraction and RenderMathMLOperator, the only remaining case to handle is RenderMathMLTable, which is addressed in this patch.

[Frédéric Wang (:fredw)]

Created attachment 277381 [details]
Patch

[Frédéric Wang (:fredw)]

The case of  RenderMathMLOperator has now removed from phase 1 of the refactoring and should be handled here too. Actually, I realized that symmetric stretching is not quite correct (at least now that we support size variant and not just glyph assembly) see bug 152244 comment 32 for suggestions.

[Frédéric Wang (:fredw)]

Created attachment 281536 [details]
Patch

Just uploading the patch again, as it seems it won't apply onto trunk for now.

[Frédéric Wang (:fredw)]

Created attachment 282055 [details]
Patch

[Brent Fulgham]

Comment on attachment 282055 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282055&action=review

This change looks good, but doesn't apply. I'll mark it r+ on the condition that you make sure the tests pass before you land it!  :-)

> Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:317
> +    return Optional<int>(logicalHeight() / 2 + axisHeight(style()));

It might be simpler to just say:

return Optional<int>(logicalHeight() / 2 + mathAxisHeight());

[Frédéric Wang (:fredw)]

(In reply to comment #8)
> Comment on attachment 282055 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=282055&action=review
> 
> This change looks good, but doesn't apply. I'll mark it r+ on the condition
> that you make sure the tests pass before you land it!  :-)
> 
> > Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp:317
> > +    return Optional<int>(logicalHeight() / 2 + axisHeight(style()));
> 
> It might be simpler to just say:
> 
> return Optional<int>(logicalHeight() / 2 + mathAxisHeight());

Yes, but again as in bug 133845 that's not possible because RenderMathMLTable does not inherit from RenderMathMLBlock. That's why I introduced this static axisHeight function to share a bit the logic... :-( I will add a comment here too.

[Frédéric Wang (:fredw)]

Created attachment 283117 [details]
Patch

[Build Bot]

Comment on attachment 283117 [details]
Patch

Attachment 283117 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1645611

New failing tests:
imported/mathml-in-html5/mathml/presentation-markup/operators/mo-axis-height-1.html

[Build Bot]

Created attachment 283124 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 283117 [details]
Patch

Attachment 283117 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1645621

New failing tests:
imported/mathml-in-html5/mathml/presentation-markup/operators/mo-axis-height-1.html

[Build Bot]

Created attachment 283125 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Frédéric Wang (:fredw)]

Created attachment 283126 [details]
Patch

[Frédéric Wang (:fredw)]

Committed r202973: <http://trac.webkit.org/changeset/202973>