Bug 119038 - Draw radicals with glyphs for better rendering
: Draw radicals with glyphs for better rendering
Status: UNCONFIRMED
: WebKit
MathML
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
: 130322
: 84019 99623 106866 115789 126516 130353
  Show dependency treegraph
 
Reported: 2013-07-24 01:09 PST by
Modified: 2014-03-27 01:42 PST (History)


Attachments
testcase (516 bytes, text/html)
2013-07-24 01:09 PST, Frédéric Wang
no flags Details
screenshot of testcase in Gecko and WebKit (4.08 KB, image/png)
2013-07-24 01:09 PST, Frédéric Wang
no flags Details
Patch V1 (29.85 KB, patch)
2014-03-24 08:33 PST, Frédéric Wang
no flags Review Patch | Details | Formatted Diff | Diff
Patch V2 (34.14 KB, patch)
2014-03-25 11:09 PST, Frédéric Wang
no flags Review Patch | Details | Formatted Diff | Diff
Current Screenshot of roots.xhtml (37.37 KB, image/png)
2014-03-25 11:09 PST, Frédéric Wang
no flags Details
Screenshot of roots.xhtml with the patch and Latin Modern Math (29.89 KB, image/png)
2014-03-25 11:10 PST, Frédéric Wang
no flags Details
Patch V3 (58.95 KB, patch)
2014-03-27 01:42 PST, Frédéric Wang
no flags Review Patch | Details | Formatted Diff | Diff


Note

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


Description From 2013-07-24 01:09:12 PST
Created an attachment (id=207380) [details]
testcase

Currently, the radicals are drawn with lines and this gives a poor rendering. See the attached testcase and the screenshot comparing Gecko (left) and WebKit (right).
------- Comment #1 From 2013-07-24 01:09:42 PST -------
Created an attachment (id=207381) [details]
screenshot of testcase in Gecko and WebKit
------- Comment #2 From 2014-03-18 03:18:11 PST -------
I think this is also the opportunity to refactor the whole RenderMathMLRoot code so that it behaves correctly with dynamic changes (I wonder whether it one the cause of bug 130353) and does not use CSS styles causing bug 126516.

@Martin: you said you wanted to work on this at some point. Did you make any progress?

The relevant MATH constants are:

RadicalVerticalGap

Space between the (ink) top of the expression and the bar over it.
Suggested: 11⁄4 default rule thickness.

RadicalDisplayStyleVertical

Space between the (ink) top of the expression and the bar over it.
Suggested: default rule thickness + 1⁄4 x-height.

RadicalRuleThickness

Thickness of the radical rule. This is the thickness of the rule in designed or constructed radical signs.
Suggested: default rule thickness.

RadicalExtraAscender

Extra white space reserved above the radical.
Suggested: RadicalRuleThickness.

RadicalKernBeforeDegree

Extra horizontal kern before the degree of a radical, if such is present.
Suggested: 5/18 of em.

RadicalKernAfterDegree

Negative kern after the degree of a radical, if such is present.
Suggested: −10/18 of em.

RadicalDegreeBottomRaise

Height of the bottom of the radical Percent degree, if such is present, in proportion to the ascender of the
radical sign.
Suggested: 60%.
------- Comment #3 From 2014-03-18 06:51:43 PST -------
(In reply to comment #2)

> @Martin: you said you wanted to work on this at some point. Did you make any progress?

Please go ahead. I did not get very far.
------- Comment #4 From 2014-03-20 02:23:37 PST -------
(In reply to comment #3)
> (In reply to comment #2)
> 
> > @Martin: you said you wanted to work on this at some point. Did you make any progress?
> 
> Please go ahead. I did not get very far.

So I tried adding the constuction

{ 0x221A, 0x20d3, 0x20d3, 0x23b7, 0x0    }, // radical

which seems to work with Asana Math and Cambria Math. However, for the old STIX set, the constructions must be done with different characters and fonts. So I'm not sure it's a good idea to do that until we can use the new fonts (bug 130322).

RenderMathMLRoot::addChild has child insertion that depends on the newChild CSS position, which can leads to weird rendering issues with msqrt/mroot if one forces the CSS style. However, I have not been able to produce a clear bug or security issue.

(I tried this:

- append absolute child c2 to an empty msqrt (this inserts the child outside the RenderMathMLRow wrapper)
- append a child c3 (this inserts it inside the RenderMathMLRow wrapper)
- insert c1 before c2 (as I read the code, this should append it to the RenderMathMLRow wrapper)
- remove c2

Then I expected the order of the renderer children to be c3, c1 instead of the DOM order c1, c3 ; but that does not seem to be the case)
------- Comment #5 From 2014-03-24 08:33:15 PST -------
Created an attachment (id=227647) [details]
Patch V1

Here is a first patch that applies on top of bug 130322.
------- Comment #6 From 2014-03-25 11:09:19 PST -------
Created an attachment (id=227768) [details]
Patch V2
------- Comment #7 From 2014-03-25 11:09:54 PST -------
Created an attachment (id=227769) [details]
Current Screenshot of roots.xhtml
------- Comment #8 From 2014-03-25 11:10:29 PST -------
Created an attachment (id=227770) [details]
Screenshot of roots.xhtml with the patch and Latin Modern Math
------- Comment #9 From 2014-03-25 11:17:21 PST -------
So I think I'm essentially done with the rewriting of the root code to draw the radical with a stretchy RenderMathMLOperator. The code uses all the radical parameters from the MATH table. I still need to write tests.

Some issues:

- Because of bug 130326, the spacing after the radical sign is sometimes too large.

- The alignment/thickness of the bar and radical sign may not match exactly in some cases. I think it's a limitation with pixel rounding etc, so I don't think that can be solved easily.

- Currently, the patch only works for fonts with a MATH table and breaks the support for radicals with other fonts. Some options: either ensure installation of MATH fonts or use some kind of fallback (scale transform, keeping the graphical drawing etc).
------- Comment #10 From 2014-03-27 01:42:30 PST -------
Created an attachment (id=227933) [details]
Patch V3

Now with complete tests for add/remove child.