133567 – Use OpenType MATH constant AxisHeight for math tables

RESOLVED FIXED 133567

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 (:fredw)

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 (:fredw)	no flags	Details Formatted Diff Diff
Patch (9.48 KB, patch) 2016-04-26 09:03 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch (117.93 KB, patch) 2016-06-16 23:13 PDT, Frédéric Wang (:fredw)	no flags	Details Formatted Diff Diff
Patch (117.96 KB, patch) 2016-06-25 00:39 PDT, Frédéric Wang (:fredw)	bfulgham: review+	Details Formatted Diff Diff
Patch (118.26 KB, patch) 2016-07-08 00:52 PDT, Frédéric Wang (:fredw)	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 (:fredw)	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Frédéric Wang (:fredw)

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 (:fredw)

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

This is done in Alex's MathMLLayout branch.

Frédéric Wang (:fredw)

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 (:fredw)

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

Created attachment 277381 [details] Patch

Frédéric Wang (:fredw)

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 (:fredw)

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 (:fredw)

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 (:fredw)

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 (:fredw)

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 (:fredw)

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

Created attachment 283126 [details] Patch

Frédéric Wang (:fredw)

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.