Bug 158866 - MathOperator: Improve alignment for vertical size variant
Summary: MathOperator: Improve alignment for vertical size variant
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords:
Depends on: 124827 152244
Blocks: 159689 159715
  Show dependency treegraph
 
Reported: 2016-06-16 23:04 PDT by Frédéric Wang (:fredw)
Modified: 2016-07-17 23:09 PDT (History)
10 users (show)

See Also:


Attachments
Patch (4.63 KB, patch)
2016-06-24 03:08 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Screenshot of MathML torture test 14, XITS fonts (5.98 KB, image/png)
2016-06-24 03:09 PDT, Frédéric Wang (:fredw)
no flags Details
Patch (101.98 KB, patch)
2016-07-13 23:25 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-yosemite (856.71 KB, application/zip)
2016-07-14 00:13 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (874.47 KB, application/zip)
2016-07-14 00:15 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-yosemite (1.49 MB, application/zip)
2016-07-14 00:21 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews125 for ios-simulator-wk2 (740.01 KB, application/zip)
2016-07-14 00:22 PDT, Build Bot
no flags Details
Patch (124.75 KB, patch)
2016-07-14 00:30 PDT, Frédéric Wang (:fredw)
bfulgham: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2016-06-16 23:04:22 PDT
Follow-up of bug 152244 comment 32.

Currently, MathOperator::stretchTo is called with the desired ascent/descent. When m_stretchType == StretchType::GlyphAssembly, this is used to set the desired vertical position of the operator. However, when m_stretchType == StretchType::SizeVariant the actual ascent/descent of the glyph is used for painting, and this may not correspond at all to the original MathOperator::stretchTo request.

One easy way to fix that is to make MathOperator::stretchTo calculate a parameter m_verticalOffset when m_stretchType != StretchType::GlyphAssembly which would measure the difference between targetCenter = (-ascent + descent) / 2 and glyphCenter = (-m_ascent + m_descent) / 2. Then that parameter can be used to apply a shift during MathOperator::paint.

Or maybe MathOperator::stretchTo should only take the target height as the parameter and it will be up to the MathOperator::paint caller to adjust the vertical offset (although that would probably mean that the shift will be stored in the caller's class).

There is a test from the MathML in HTML5 test suite but it does not work well in WebKit yet:
http://tests.mathml-association.org/mathml/presentation-markup/operators/mo-axis-height-1.html
Comment 1 Frédéric Wang (:fredw) 2016-06-24 03:08:13 PDT
Created attachment 281964 [details]
Patch

Here is an experimental patch showing the idea. Not sure it handles all the cases.
Comment 2 Frédéric Wang (:fredw) 2016-06-24 03:09:21 PDT
Created attachment 281965 [details]
Screenshot of MathML torture test 14, XITS fonts

This screenshot shows the improvement of the patch for MathML torture test 14, XITS fonts.
Comment 3 Frédéric Wang (:fredw) 2016-07-13 23:25:19 PDT
Created attachment 283605 [details]
Patch
Comment 4 Frédéric Wang (:fredw) 2016-07-13 23:29:08 PDT
So I'm not sure what happened with mo-axis-height-1.html. It seems that the previous import and test expectation was just wrong :-( Anyway, after bug 158884 on http://tests.mathml-association.org/mathml/presentation-markup/operators/mo-axis-height-1.html the size variant test fails and the glyph assembly test passes. With the uploaded patch, the size variant test should pass too ;-)

I still need to retrieve some pixel results for iOS/Mac, rebaseline and upload a new patch...
Comment 5 Build Bot 2016-07-14 00:13:09 PDT
Comment on attachment 283605 [details]
Patch

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

New failing tests:
mathml/opentype/opentype-stretchy.html
mathml/presentation/mo-stretch.html
Comment 6 Build Bot 2016-07-14 00:13:13 PDT
Created attachment 283610 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 7 Build Bot 2016-07-14 00:15:15 PDT
Comment on attachment 283605 [details]
Patch

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

New failing tests:
mathml/opentype/opentype-stretchy.html
mathml/presentation/mo-stretch.html
Comment 8 Build Bot 2016-07-14 00:15:19 PDT
Created attachment 283611 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 9 Build Bot 2016-07-14 00:21:52 PDT
Comment on attachment 283605 [details]
Patch

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

New failing tests:
mathml/opentype/opentype-stretchy.html
mathml/presentation/mo-stretch.html
Comment 10 Build Bot 2016-07-14 00:21:56 PDT
Created attachment 283614 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 11 Build Bot 2016-07-14 00:22:25 PDT
Comment on attachment 283605 [details]
Patch

Attachment 283605 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1678594

New failing tests:
mathml/opentype/opentype-stretchy.html
mathml/presentation/mo-stretch.html
Comment 12 Build Bot 2016-07-14 00:22:29 PDT
Created attachment 283615 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.5
Comment 13 Frédéric Wang (:fredw) 2016-07-14 00:30:42 PDT
Created attachment 283617 [details]
Patch
Comment 14 Frédéric Wang (:fredw) 2016-07-15 11:28:42 PDT
Can someone please review this patch?
Comment 15 Brent Fulgham 2016-07-15 11:40:58 PDT
Comment on attachment 283617 [details]
Patch

This change looks fine. r=me.
Comment 16 Frédéric Wang (:fredw) 2016-07-15 11:48:12 PDT
Committed r203289: <http://trac.webkit.org/changeset/203289>
Comment 17 Frédéric Wang (:fredw) 2016-07-17 23:09:05 PDT
Committed r203339: <http://trac.webkit.org/changeset/203339>