133567 – Use OpenType MATH constant AxisHeight for math tables

RESOLVED FIXED133567

Use OpenType MATH constant AxisHeight for math tables

https://bugs.webkit.org/show_bug.cgi?id=133567

Summary Use OpenType MATH constant AxisHeight for math tables

Frédéric Wang Nélar

Reported 2014-06-05 22:47:28 PDT

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.

Attachments
Patch (8.50 KB, patch) 2016-03-18 08:09 PDT, Frédéric Wang Nélar	no flags	Details Formatted Diff Diff
Patch (9.48 KB, patch) 2016-04-26 09:03 PDT, Frédéric Wang Nélar	no flags	Details Formatted Diff Diff
Patch (117.93 KB, patch) 2016-06-16 23:13 PDT, Frédéric Wang Nélar	no flags	Details Formatted Diff Diff
Patch (117.96 KB, patch) 2016-06-25 00:39 PDT, Frédéric Wang Nélar	bfulgham: review+	Details Formatted Diff Diff
Patch (118.26 KB, patch) 2016-07-08 00:52 PDT, Frédéric Wang Nélar	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews102 for mac-yosemite (920.72 KB, application/zip) 2016-07-08 01:39 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews104 for mac-yosemite-wk2 (788.68 KB, application/zip) 2016-07-08 01:44 PDT, Build Bot	no flags	Details
Patch (118.12 KB, patch) 2016-07-08 01:49 PDT, Frédéric Wang Nélar	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Frédéric Wang Nélar

Comment 1 2014-07-20 07:44:15 PDT

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 Nélar

Comment 2 2016-02-05 06:27:16 PST

This is done in Alex's MathMLLayout branch.

Frédéric Wang Nélar

Comment 3 2016-03-18 08:09:04 PDT

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 Nélar

Comment 4 2016-04-26 09:03:10 PDT

Created attachment 277381 [details] Patch

Frédéric Wang Nélar

Comment 5 2016-05-17 01:01:23 PDT

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 Nélar

Comment 6 2016-06-16 23:13:45 PDT

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 Nélar

Comment 7 2016-06-25 00:39:27 PDT

Created attachment 282055 [details] Patch

Brent Fulgham

Comment 8 2016-07-07 13:21:51 PDT

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 Nélar

Comment 9 2016-07-08 00:40:11 PDT

(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 Nélar

Comment 10 2016-07-08 00:52:57 PDT

Created attachment 283117 [details] Patch

Build Bot

Comment 11 2016-07-08 01:39:32 PDT

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

Comment 12 2016-07-08 01:39:36 PDT

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 13 2016-07-08 01:44:00 PDT

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

Comment 14 2016-07-08 01:44:03 PDT

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 Nélar

Comment 15 2016-07-08 01:49:05 PDT

Created attachment 283126 [details] Patch

Frédéric Wang Nélar

Comment 16 2016-07-08 02:38:42 PDT

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

Note You need to log in before you can comment on or make changes to this bug.