Last Comment Bug 414277 - (scale-stretchy) scale stretchy operators when there is no glyph of suitable size
(scale-stretchy)
: scale stretchy operators when there is no glyph of suitable size
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: x86 All
: -- normal with 1 vote (vote)
: mozilla2.0b5
Assigned To: Frédéric Wang (:fredw)
:
Mentors:
: 517774 538895 603927 613676 (view as bug list)
Depends on: 518172 nsStretchDirection mathml-operator-dict 542605 562347 LookupOperator 569195 605605
Blocks: 464592 stretchy-diagonal 208309 416065 416140 454155 491370 516292 518330
  Show dependency treegraph
 
Reported: 2008-01-27 16:28 PST by Karl Tomlinson (:karlt)
Modified: 2010-11-19 21:35 PST (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
First patch (5.01 KB, patch)
2009-09-08 11:26 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
Screenshot of MathML Torture Test with/without the patch applied (27.16 KB, image/png)
2009-09-08 11:27 PDT, Frédéric Wang (:fredw)
no flags Details
Testcase that demonstrates some problems (3.60 KB, application/xhtml+xml)
2009-09-10 01:39 PDT, Frédéric Wang (:fredw)
no flags Details
Screenshot of testcase: STIX fonts and patch not applied (39.01 KB, image/png)
2009-09-10 01:40 PDT, Frédéric Wang (:fredw)
no flags Details
Screenshot of testcase: No font and patch applied (43.24 KB, image/png)
2009-09-10 01:40 PDT, Frédéric Wang (:fredw)
no flags Details
stretching integral symbols (10.27 KB, image/png)
2009-09-10 06:53 PDT, Frédéric Wang (:fredw)
no flags Details
Stretch only for vertical/horizontal + add integral symbols in mathfont.properties (19.29 KB, patch)
2009-09-14 11:19 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
testcase (3.77 KB, application/xhtml+xml)
2009-09-14 11:24 PDT, Frédéric Wang (:fredw)
no flags Details
Screenshot of the testcase (no font + patch applied) (53.76 KB, image/png)
2009-09-14 11:25 PDT, Frédéric Wang (:fredw)
no flags Details
Ascent/Descent for vertical scale (19.73 KB, patch)
2009-09-14 14:22 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
Screenshot of the testcase (no font + patch applied) (52.27 KB, image/png)
2009-09-14 14:23 PDT, Frédéric Wang (:fredw)
no flags Details
Fix rendering of tests 3 & 4 (22.48 KB, patch)
2009-09-15 04:19 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
testcase largeop (3.29 KB, application/xhtml+xml)
2009-09-20 14:23 PDT, Frédéric Wang (:fredw)
no flags Details
Screenshot testcase largeop (84.96 KB, image/png)
2009-09-20 14:23 PDT, Frédéric Wang (:fredw)
no flags Details
patch (5th version) (16.31 KB, patch)
2010-01-21 03:13 PST, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
Testcase largeop (3.68 KB, application/xhtml+xml)
2010-01-21 03:17 PST, Frédéric Wang (:fredw)
no flags Details
Screenshot of MathML Torture Test with/without the patch applied (28.75 KB, image/png)
2010-01-21 03:22 PST, Frédéric Wang (:fredw)
no flags Details
patch (6th version) (21.61 KB, patch)
2010-01-24 11:44 PST, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
Testcase largeop (3.73 KB, application/xhtml+xml)
2010-01-26 09:09 PST, Frédéric Wang (:fredw)
no flags Details
Testcase integral (2.30 KB, application/xhtml+xml)
2010-01-26 09:09 PST, Frédéric Wang (:fredw)
no flags Details
Testcase root (4.35 KB, application/xhtml+xml)
2010-01-26 09:10 PST, Frédéric Wang (:fredw)
no flags Details
Screenshot largeop (9.85 KB, image/png)
2010-01-26 09:11 PST, Frédéric Wang (:fredw)
no flags Details
Screenshot integral (7.30 KB, image/png)
2010-01-26 09:11 PST, Frédéric Wang (:fredw)
no flags Details
Screenshot root (25.68 KB, image/png)
2010-01-26 09:12 PST, Frédéric Wang (:fredw)
no flags Details
patch (7th version) (24.28 KB, patch)
2010-01-26 09:13 PST, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
bug parenthesis (782 bytes, application/xhtml+xml)
2010-01-26 14:58 PST, Frédéric Wang (:fredw)
no flags Details
screenshot bug parenthesis (3.43 KB, image/png)
2010-01-26 14:58 PST, Frédéric Wang (:fredw)
no flags Details
patch (8th version) (18.84 KB, patch)
2010-04-29 11:56 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
Patch (9th version) (18.55 KB, patch)
2010-04-30 10:12 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
screen shot from https://eyeasme.com/Joe/MathML/MathML_browser_test_next (235.81 KB, image/png)
2010-05-13 11:23 PDT, Joe Java
no flags Details
Bug tilde (1.48 KB, application/xhtml+xml)
2010-05-17 12:31 PDT, Frédéric Wang (:fredw)
no flags Details
OverBar touches overscript (25.71 KB, image/png)
2010-05-17 13:41 PDT, Joe Java
no flags Details
strange gaps in uparrow (23.30 KB, image/png)
2010-05-17 13:45 PDT, Joe Java
no flags Details
Patch v. 10 (23.50 KB, patch)
2010-05-24 00:01 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
Patch v. 11 (24.09 KB, patch)
2010-05-24 12:33 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
Triple integral looks strange (55.17 KB, image/png)
2010-05-30 12:48 PDT, Joe Java
no flags Details
Patch v12 (24.39 KB, patch)
2010-06-10 14:12 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
notice the triple integral in last colum is not correct size (55.41 KB, image/png)
2010-06-11 19:54 PDT, Joe Java
no flags Details
MathML try build of 6_11_10 (401.37 KB, image/png)
2010-06-12 00:16 PDT, Joe Java
no flags Details
Patch v13 (24.15 KB, patch)
2010-08-01 14:07 PDT, Frédéric Wang (:fredw)
karlt: review+
dbaron: approval2.0+
Details | Diff | Splinter Review
add reftests (8.07 KB, patch)
2010-08-11 08:09 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
add reftests (11.52 KB, patch)
2010-08-11 23:41 PDT, Frédéric Wang (:fredw)
karlt: review+
Details | Diff | Splinter Review

Description Karl Tomlinson (:karlt) 2008-01-27 16:28:23 PST
For some stretchy grouping operators, the meaning (context) of the symbol depends on the size of the symbol.  If the font doesn't provide a glyph of suitable size, then we should scale the nearest-sized glyph.

This could be done either by changing the font-size, scaling by the same factor in each direction), which would work well for ∑, or by changing the transformation matrix to scale less in the non-stretchy direction, which might look better for parentheses and maybe radicals.

Doing this would improve MathML rendering for those that don't have STIX fonts installed.
Comment 1 distler 2008-01-27 17:49:06 PST
With respect to stretching "large operators" like ∑, the MathML Spec,

   http://www.w3.org/TR/2003/REC-MathML2-20031021/chapter3.html#id.3.2.5.8.2

says that these operators are supposed to have stretchy="true". As pointed out in the comments to bug 363240, TeX doesn't do that.  But the default size of these "largeop" operators in TeX is considerably larger (at least for display equations) than the current behaviour in Gecko.

For most purposes, the TeX behaviour works well enough. (It should be remembered that TeX's behaviour was determined in an era when scalable fonts didn't exist. Stretching a parenthesis with an extender glyph was one thing; stretching a summation sign would have required running Metafont on the fly.)

I'd be happy enough if the largeop operators were rendered in a larger font-size (as TeX does) in display equations.
Comment 2 Frédéric Wang (:fredw) 2009-09-08 11:26:39 PDT
Created attachment 399257 [details] [diff] [review]
First patch
Comment 3 Frédéric Wang (:fredw) 2009-09-08 11:27:19 PDT
Created attachment 399259 [details]
Screenshot of MathML Torture Test with/without the patch applied
Comment 4 Frédéric Wang (:fredw) 2009-09-08 11:28:47 PDT
	Attachment 399257 [details] [diff] is a simple patch that forces a scale transform when the Stretch function is not able to find a glyph / set of glyphs of suitable size. However I don't know exactly when this scale is actually needed and I should study more carefuly how the Stretch function works.

	Attachement 399259 is a screenshot of the MathML Torture Test with the font.mathfont-family empty (right hand side = patch applied). In this case, one can see that the patch improves stretching of braces, radicals and integrals.
Comment 5 Karl Tomlinson (:karlt) 2009-09-09 16:20:35 PDT
Comment on attachment 399259 [details]
Screenshot of MathML Torture Test with/without the patch applied

This is looking promising.
Comment 6 Frédéric Wang (:fredw) 2009-09-10 01:39:45 PDT
Created attachment 399688 [details]
Testcase that demonstrates some problems
Comment 7 Frédéric Wang (:fredw) 2009-09-10 01:40:31 PDT
Created attachment 399689 [details]
Screenshot of testcase: STIX fonts and patch not applied
Comment 8 Frédéric Wang (:fredw) 2009-09-10 01:40:57 PDT
Created attachment 399690 [details]
Screenshot of testcase: No font and patch applied
Comment 9 Jeroen Ruigrok van der Werven 2009-09-10 01:45:45 PDT
Definitely looking promising Frédéric!
Comment 10 Karl Tomlinson (:karlt) 2009-09-10 02:18:06 PDT
The operator dictionary (in mathfont.properties) specifies whether each operator stretches by default and whether that is in a horizontal or vertical direction.
(Really the direction and default stretchiness properties should probably be orthogonal, though it looks like direction can currently only be specified when stretchy at the moment.)
Apparently the operator dictionary is not being considered before stretching.
This would prevent ',' from stretching at all (unless explicitly specified), and would constrain the direction of stretch for arrows and union.

(The broken arrows are an unrelated issue.  It could probably be improved by checking that all glyphs come from the same font but that's difficult to determine until we switch from nsIRenderingContext to gfxFontGroup at least.)
Comment 11 Frédéric Wang (:fredw) 2009-09-10 02:30:32 PDT
> Apparently the operator dictionary is not being considered before stretching.
> This would prevent ',' from stretching at all (unless explicitly specified),
> and would constrain the direction of stretch for arrows and union.

For the comma, I think the problem comes from nsMathMLmfenceFrame.cpp that explicitly specify a vertical stretch. I have not check yet what happens for horizontal arrows and large operators.
Comment 12 Frédéric Wang (:fredw) 2009-09-10 06:53:11 PDT
Created attachment 399717 [details]
stretching integral symbols
Comment 13 Frédéric Wang (:fredw) 2009-09-10 06:53:39 PDT
	I easily fixed case 5 (large op stretched horizontally) by applying scale only when mDirection has been set to vertical/horizontal by the Stretch function (I thought it was always the case).
	I added other integral symbols in case 2 and got a weird effect: the symbols are not stretched (see Attachment 399717 [details]) but we can see the scale applied when we select them.
Comment 14 Frédéric Wang (:fredw) 2009-09-14 11:19:49 PDT
Created attachment 400542 [details] [diff] [review]
Stretch only for vertical/horizontal + add integral symbols in mathfont.properties
Comment 15 Frédéric Wang (:fredw) 2009-09-14 11:24:33 PDT
Created attachment 400544 [details]
testcase
Comment 16 Frédéric Wang (:fredw) 2009-09-14 11:25:40 PDT
Created attachment 400545 [details]
Screenshot of the testcase (no font + patch applied)
Comment 17 Frédéric Wang (:fredw) 2009-09-14 11:39:24 PDT
I've tried the patch with current trunk and it seems the weird effect described in comment 13 disappeared. Attachment 400542 [details] [diff] fixes the problem of large op stretched horizontally (test 5) as indicated in this comment. I also added some integral symbols in mathfont.properties, so that all these symbols are now stretched vertically (test 2).

Apart from the issue of symbols that should not be stretched vertically, it seems that the vertical position is not correct (test 2).
Comment 18 Frédéric Wang (:fredw) 2009-09-14 14:22:47 PDT
Created attachment 400585 [details] [diff] [review]
Ascent/Descent for vertical scale
Comment 19 Frédéric Wang (:fredw) 2009-09-14 14:23:21 PDT
Created attachment 400587 [details]
Screenshot of the testcase (no font + patch applied)
Comment 20 Frédéric Wang (:fredw) 2009-09-14 14:24:29 PDT
	I updated the patch so that when a MathMLChar is scaled vertically, its ascent/descent is set to those of aContainerSize. This fixes the problem of vertical alignment in test 2. The next step is to investigate why symbols are stretched vertically in tests 3 and 4...
	
	BTW, it seems that the current patch solves issues in other bugs:
- Bug 416065: tilde and check. https://bug416065.bugzilla.mozilla.org/attachment.cgi?id=301824
- Bug 301919: underbrace and overbrace. https://bug416140.bugzilla.mozilla.org/attachment.cgi?id=301919
- Bug 301824: angle brackets. https://eyeasme.com/Joe/MathML/angle_brackets_bug.html
Comment 21 Frédéric Wang (:fredw) 2009-09-15 04:19:32 PDT
Created attachment 400736 [details] [diff] [review]
Fix rendering of tests 3 & 4
Comment 22 Jo Hermans 2009-09-20 10:31:36 PDT
*** Bug 517774 has been marked as a duplicate of this bug. ***
Comment 23 Frédéric Wang (:fredw) 2009-09-20 14:23:00 PDT
Created attachment 401745 [details]
testcase largeop
Comment 24 Frédéric Wang (:fredw) 2009-09-20 14:23:35 PDT
Created attachment 401746 [details]
Screenshot testcase largeop
Comment 25 Frédéric Wang (:fredw) 2009-09-20 14:25:02 PDT
Now the problems of the testcase are fixed, let's turn our attention to the case of largeop. I've attached a new testcase and a screenshot with three versions (from left to right):

1) no patch applied
2) patch applied
3) patch applied + preserve aspect ratio

I think the third case is good but I don't know how to determine when aspect ratio should be preserved for a largeop (for instance, it should not be preserved for integral).
Comment 26 Frédéric Wang (:fredw) 2009-09-23 07:07:41 PDT
(In reply to comment #1)
> With respect to stretching "large operators" like ∑, the MathML Spec,
> 
>    http://www.w3.org/TR/2003/REC-MathML2-20031021/chapter3.html#id.3.2.5.8.2
> 
> says that these operators are supposed to have stretchy="true". As pointed out
> in the comments to bug 363240, TeX doesn't do that.  But the default size of
> these "largeop" operators in TeX is considerably larger (at least for display
> equations) than the current behaviour in Gecko.
> 
> For most purposes, the TeX behaviour works well enough. (It should be
> remembered that TeX's behaviour was determined in an era when scalable fonts
> didn't exist. Stretching a parenthesis with an extender glyph was one thing;
> stretching a summation sign would have required running Metafont on the fly.)
> 
> I'd be happy enough if the largeop operators were rendered in a larger
> font-size (as TeX does) in display equations.

I opened bug 518330 for this issue.
Comment 27 Frédéric Wang (:fredw) 2009-09-23 11:51:36 PDT
It seems that the Math WG is going to remove the requirement that large operators stretch vertically, so we can make symbols like sum nonstretchy:

http://lists.w3.org/Archives/Public/www-math/2009Sep/

However, we still need to deal with this case when the user explicitly ask to stretch the symbol. I agree with what is indicated in comment 10 : direction and default strechiness properties should be orthogonal. Hence I suggest to change mathfonts.properties in this way:

- "strechy" takes values true or false
- add a new attribute "direction" that takes at least values "vertical" and "horizontal".

	Then I would put the default value "both" to "direction". This would be the case where we want to preserve aspect ratio (sum, union...). We can even add a value "diagonal" for instance for diagonal arrows (BTW, this could be implemented using vertical/horizontal streching + rotation matrix). Finally, I think the syntax could be simplified for boolean properties: rather than "stretchy:true accent:true", just write "stretchy accent" (this is the syntax used in the MathML 3 REC). I guess it's worth opening a new bug to fix these issues first.
Comment 28 Frédéric Wang (:fredw) 2009-09-27 13:48:31 PDT
The WG has not really answered yet. However I've already prepared an improved syntax for mathfont.properties, that it is worth doing anyway (see bug 519126)
Comment 29 Karl Tomlinson (:karlt) 2009-09-28 00:33:59 PDT
(In reply to comment #27)
> However, we still need to deal with this case when the user explicitly ask to
> stretch the symbol. I agree with what is indicated in comment 10 : direction
> and default strechiness properties should be orthogonal. Hence I suggest to
> change mathfonts.properties in this way:
> 
> - "strechy" takes values true or false
> - add a new attribute "direction" that takes at least values "vertical" and
> "horizontal".

Sounds good.  It's not what the spec says as you pointed out very clearly,
thank you: http://lists.w3.org/Archives/Public/www-math/2009Sep/0021.html

Though these changes to mathfonts.properties seem to be appropriate for
producing the behavior that I think the spec authors expected.  There is a
hint of that in "but some renderers may be able to stretch *certain*
characters, such as diagonal arrows, in both directions independently"
(emphasis added).

> Then I would put the default value "both" to "direction". This would be the
> case where we want to preserve aspect ratio (sum, union...).

I'm happy with a default value "both" for stretching in *either* direction.
(We don't do mtd for both at once yet.)

However, "both" isn't really what's needed for sum and union.
These look best if they preserve aspect ratio, but they shouldn't stretch
horizontally just because their limits are wide, for example.
I'd prefer that "both" meant that they stretch horizontally and/or vertically
depending on their parent element.

Another tricky issue is making GetMaxWidth() work with operators that expand
to arbitrary sizes, while preserving aspect ratio.  (Really we need a
different layout algorithm, but I don't have a suitable algorithm in mind, so
let's not try to do that now.)

How about this for operators with largeop?:

* When only stretched as a largeop, increase the font size by a factor.
  Would a factor of 2 be suitable?  Or less maybe.  I haven't looked at
  textbooks for fonts to see what they do.

  This would preserve aspect ratio.  I think font size may work better with
  measuring and font hinting than a transformation, but if I'm wrong and a
  transformation works just as well then that's fine too.

* When the largeop is also stretchy, first increase font size (or symmetric
  transformation) by the largeop factor, then stretch by (additional)
  transformation only in the direction of the stretch.

This could be a reasonable compromise for having sum and union looking
reasonable in their largeop default non-stretchy state.  (If stretchy="true"
is specified and they stretch considerably, then they won't look their best, but there will be some extra width to balance things out a bit.)  Hopefully largeop integrals wouldn't look too bad with only a fixed extra thickness.
Comment 30 Frédéric Wang (:fredw) 2009-09-28 09:04:13 PDT
> I'm happy with a default value "both" for stretching in *either* direction.
> (We don't do mtd for both at once yet.)
> 
> However, "both" isn't really what's needed for sum and union.
> These look best if they preserve aspect ratio, but they shouldn't stretch
> horizontally just because their limits are wide, for example.
> I'd prefer that "both" meant that they stretch horizontally and/or vertically
> depending on their parent element.
> 
> Another tricky issue is making GetMaxWidth() work with operators that expand
> to arbitrary sizes, while preserving aspect ratio.  (Really we need a
> different layout algorithm, but I don't have a suitable algorithm in mind, so
> let's not try to do that now.)
> 

Yes, actually I changed my mind meanwhile and I agree with you for the default value. Anyway if in order to display large operators we are going to use font size rather than stretching (as TeX & MathML3 Operator Dictionary do) then we do not really need to preserve aspect ratio in that case.

> * When only stretched as a largeop, increase the font size by a factor.
>   Would a factor of 2 be suitable?  Or less maybe.  I haven't looked at
>   textbooks for fonts to see what they do.

I guess it's worth asking on mozilla.dev.tech.mathml what people expect. I'm still not really sure about what TeX do for integral symbols. Also, the factor may depend on whether the formula is in displaystyle="true" or not.

>   This would preserve aspect ratio.  I think font size may work better with
>   measuring and font hinting than a transformation, but if I'm wrong and a
>   transformation works just as well then that's fine too.

Is the transformation matrix changed before the curves composing the glyph are drawn? Or is the glyph drawn first and then the scale applied? I believe the former happens and in this case the glyph is rendered with the same quality as if we increase font size. At the moment I can't really imagine what would be best for measuring. In a first time, I'll start using what I've already made for scaling and we'll see later if we should rather use font size.
Comment 31 Karl Tomlinson (:karlt) 2009-09-28 15:13:43 PDT
(In reply to comment #30)
> > * When only stretched as a largeop, increase the font size by a factor.
> >   Would a factor of 2 be suitable?  Or less maybe.  I haven't looked at
> >   textbooks for fonts to see what they do.
> 
> I guess it's worth asking on mozilla.dev.tech.mathml what people expect.

These were discussed a bit in this thread, where Alvaro suggested "twice as
big".  I'm not sure if that was height or area, or whether it is even meant to
be precise.  It probably even depends on the operator, so the value will
probably be a compromise.

http://groups.google.com/group/mozilla.dev.tech.mathml/browse_frm/thread/39a3ac4e6bc34e50#

No harm in asking more specifically in the newsgroup if you like.

> I'm still not really sure about what TeX do for integral symbols.

There will be different preferences from different people here.

Check out bug 363240 comment 21 and subsequent.

It sounds like the MathML WG did some research and decided to suggest all
largeop operators default non-stretchy.
http://lists.w3.org/Archives/Public/www-math/2009Sep/0015.html

I would follow that suggestion, as eventually people will probably refer to
the upcoming suggested operator dictionary and realize that they should
explicitly specify stretchy="true" if they want that behavior.

>                                                   Also, the factor
> may depend on whether the formula is in displaystyle="true" or not.

Largeop should have no effect when displaystyle="false".
http://www.w3.org/TR/MathML3/chapter3.html#id.3.2.5.2

> 
> >   This would preserve aspect ratio.  I think font size may work better with
> >   measuring and font hinting than a transformation, but if I'm wrong and a
> >   transformation works just as well then that's fine too.
> 
> Is the transformation matrix changed before the curves composing the glyph are
> drawn? Or is the glyph drawn first and then the scale applied? I believe the
> former happens and in this case the glyph is rendered with the same quality as
> if we increase font size.

Yes, I think you are right.

> At the moment I can't really imagine what would be best for measuring.

Getting the bounding metrics is different: the glyph is first drawn and
measured, then the scale is applied:

http://hg.mozilla.org/mozilla-central/annotate/8a350f5584d4/gfx/thebes/src/gfxFont.cpp#l1089

But the bounds probably do not need to be that accurate.

> In a first time, I'll start using what I've already made
> for scaling and we'll see later if we should rather use font size.

The transformation will probably be fine.
Comment 32 s.g.mcateer@gmail.com 2010-01-10 15:05:05 PST
*** Bug 538895 has been marked as a duplicate of this bug. ***
Comment 33 Frédéric Wang (:fredw) 2010-01-21 03:13:11 PST
Created attachment 422713 [details] [diff] [review]
patch (5th version)

not many changes... just an update to be compatible with the patches from the bugs this depends on.
Comment 34 Frédéric Wang (:fredw) 2010-01-21 03:17:39 PST
Created attachment 422714 [details]
Testcase largeop

Largeops with displaystyle="true"
Comment 35 Frédéric Wang (:fredw) 2010-01-21 03:22:06 PST
Created attachment 422715 [details]
Screenshot of MathML Torture Test with/without the patch applied
Comment 36 Jeroen Ruigrok van der Werven 2010-01-21 03:45:04 PST
It is looking very impressive Frédéric. I, for one, appreciate the work you are doing here. Keep it up. :)
Comment 37 Frédéric Wang (:fredw) 2010-01-24 11:44:16 PST
Created attachment 423261 [details] [diff] [review]
patch (6th version)
Comment 38 Frédéric Wang (:fredw) 2010-01-26 09:09:04 PST
Created attachment 423536 [details]
Testcase largeop

The largeop testcase was not correct. It seems that the union was considered as "infix" (so not a largeop according to our dictionary). I added an mrow around the right hand side of the equality and the union is now displayed larger.
Comment 39 Frédéric Wang (:fredw) 2010-01-26 09:09:43 PST
Created attachment 423538 [details]
Testcase integral
Comment 40 Frédéric Wang (:fredw) 2010-01-26 09:10:25 PST
Created attachment 423539 [details]
Testcase root
Comment 41 Frédéric Wang (:fredw) 2010-01-26 09:11:26 PST
Created attachment 423540 [details]
Screenshot largeop
Comment 42 Frédéric Wang (:fredw) 2010-01-26 09:11:55 PST
Created attachment 423541 [details]
Screenshot integral
Comment 43 Frédéric Wang (:fredw) 2010-01-26 09:12:31 PST
Created attachment 423542 [details]
Screenshot root
Comment 44 Frédéric Wang (:fredw) 2010-01-26 09:13:51 PST
Created attachment 423544 [details] [diff] [review]
patch (7th version)
Comment 45 Frédéric Wang (:fredw) 2010-01-26 09:52:28 PST
The current version of the patch is able to apply the scale transform to any kind of nsMathMLChar (drawn with a "normal" char, a single glyph or a composite char). This improves size matching since we can first try to find the nearest size and then apply scaling to correct the error (see below the case of the roots). Also, it will be possible to add new transforms such that negative scaling (used to mirror operators in bug 208309) or rotation (for the diagonal arrows). BTW, I've made some tests to apply rotation but I think we should postpone this feature and open a separate bug for that (anyway, it will really be useful/testable only when bug is 236963 fixed). I think we should concentrate on the "refresh" bug I mention in comment 13 and below.

Largeops now have the behaviour described in comment 29. The increase factor is currently 2 (maybe it's too large?) and only apply when displaystyle=true (maybe we should also apply a factor when displaystyle=false?). The default behaviour is now changed to stretchy=false for largeop (as discussed previously) but it is possible for the user to modify it (see testcase "integral").
	
Without the patch, root symbols were drawn either too small (no STIX font) or too large (apparently, like TeX does in the MathML torture test). Scaling enables to correct the error. Note however a bug with the mroot when another frame overlap the sqrt char. I suspect to be an instance of the "refresh" bug.
	
As you might have understand, my main problem is this "refresh" bug: the operator is drawn again without the transforms applied. I first thought the member of nsMathMLChar that describes the transform parameter was reinitialized but it seems to come from elsewhere. I'm not able to explain it, but what I've noticed so far:
- it happens more often when STIX fonts are disabled
- it can appear / be fixed when you select/drag the char with your mouse.
- it can be fixed when you scroll the window with up/down keys (push the char outside and move back).
- it can appear when you scroll the window with page up/down keys.
- sometimes a part of the char is repaint correctly
	
Some examples: in the last test of attachment 422715 [details], the overbrace covered the b's but became smaller when I took the screenshot. In general, the torture test can be affected by the bug when you scroll with page up/down keys. See also this page with many sum symbols: http://www.cs.au.dk/~fwang/riemannian_geometry/exercise_4_12.xhtml.
Comment 46 Frédéric Wang (:fredw) 2010-01-26 14:58:14 PST
Created attachment 423646 [details]
bug parenthesis
Comment 47 Frédéric Wang (:fredw) 2010-01-26 14:58:43 PST
Created attachment 423647 [details]
screenshot bug parenthesis
Comment 48 Frédéric Wang (:fredw) 2010-01-26 15:00:08 PST
I've isolated a case where the bug happens with a parenthesis (attachment 423646 [details]). Screenshot is attachment 423647 [details]. From left to right:
- with STIX fonts
- without STIX fonts, obtained by selecting the line of the "x0)" with the mouse.
- without STIX fonts, just after the page is loaded.
Comment 49 Frédéric Wang (:fredw) 2010-01-26 15:03:18 PST
(In reply to comment #48)

Also, using printf in the function of nsMathMLChar that displays the char shows that the mScaleY value does not change and consequently I think the parenthesis is displayed elsewhere.
Comment 50 Frédéric Wang (:fredw) 2010-02-09 02:52:26 PST
I've just tried to print the test pages and the scale correction is always well-applied in the ps files. So I still think there is something in the refresh system that produces the bug I describes in comment 45 (BTW, I've also noticed moving another window in front of the char and then moving it out can also change the scale), i.e. that redisplay the operator without the scale. However I've no idea about how the bug precisely comes from. I'm curious to know whether other people can see this bug?
Comment 51 Karl Tomlinson (:karlt) 2010-02-09 22:13:53 PST
Maybe something that might be helpful is to add some logging to the appropriate nsThebesRenderingContext::DrawString to log the current transformations (and maybe the string also) to check that this gets called at the expected times and that the expected transformations are on the context.
Comment 52 Frédéric Wang (:fredw) 2010-02-10 14:46:10 PST
(In reply to comment #51)
> Maybe something that might be helpful is to add some logging to the appropriate
> nsThebesRenderingContext::DrawString to log the current transformations (and
> maybe the string also) to check that this gets called at the expected times and
> that the expected transformations are on the context.

I've added some logging and tested attachment 423646 [details]. When the function is called with ')', the transformation is a scale along the Y-axis as expected. I've tested both nsThebesRenderingContext::GetCurrentTransform and nsThebesRenderingContext::mThebes->CurrentMatrix() and got the same result.
Comment 53 Karl Tomlinson (:karlt) 2010-02-10 17:34:58 PST
What platform are you testing on, Frédéric?
There's chance that the wrong glyph is being pulled from a cache in cairo or similar.  Do you see sometimes the wrong glyph changing to the right glyph and sometimes right glyph to wrong glyph?
Comment 54 Frédéric Wang (:fredw) 2010-02-11 04:07:11 PST
(In reply to comment #53)
> What platform are you testing on, Frédéric?

Hardware: x86
OS: Linux

> There's chance that the wrong glyph is being pulled from a cache in cairo or
> similar.  Do you see sometimes the wrong glyph changing to the right glyph and
> sometimes right glyph to wrong glyph?

I'm not sure to understand what you mean by right/wrong glyph, but yes I see
the wrong size changing to right size and conversely. For the attachment 423646 [details]
with font.mathfont-family empty, I have:

- by default the parenthesis is not scaled.
- moving another window over the parenthesis and then moving this window out,
  the parenthesis is scaled correctly.
- focusing the browser's window makes the parenthesis come back to its 
  non-scaled size.
- and I can repeat the operation indefinitely
Comment 55 Frédéric Wang (:fredw) 2010-04-26 10:53:34 PDT
I've just tested with a recent build again and I can no longer produce the "refresh" bug. I don't know if some changes on the trunk have fixed it or if it's because I've upgraded some system libraries...

I think the patch will be ready for a first review once bug 524275 is fixed. Given the fact that it gives many improvements, I would appreciate if it is integrated in Firefox 3.7. However, I think some tests would also probably needed since it makes many changes to the rendering of operators (for example some problems could arise from our operator dictionary, we should fix bug 534970 and complete carefully the entries)... I wonder if there is a way to make binaries available, so that people can provide feedback.
Comment 56 Bill Gianopoulos [:WG9s] 2010-04-26 15:28:13 PDT
(In reply to comment #55)
> 534970 and complete carefully the entries)... I wonder if there is a way to
> make binaries available, so that people can provide feedback.

You could do tryserver builds.

I do Windows and Linux builds that include the patches here and from bug 542275 (among other pending MathML and SVG patches) more or less daily, which I make available at http://www.wg9s.com/mozilla/firefox/
Comment 57 Karl Tomlinson (:karlt) 2010-04-27 00:11:57 PDT
(In reply to comment #55)
> I've just tested with a recent build again and I can no longer produce the
> "refresh" bug.

Might be bug 542605.

> I wonder if there is a way to
> make binaries available, so that people can provide feedback.

Yes, tryserver: https://wiki.mozilla.org/Build:TryServer

I recommend "push to try", which requires an hg account with "Level 1" commit access.  I can vouch for you for that or for "Level 2" if you like.
Comment 58 Karl Tomlinson (:karlt) 2010-04-27 00:15:06 PDT
http://www.mozilla.org/hacking/committer/
Comment 59 Frédéric Wang (:fredw) 2010-04-28 08:24:29 PDT
Thanks! I've opened bug 562347.
Comment 60 Frédéric Wang (:fredw) 2010-04-29 11:56:55 PDT
Created attachment 442464 [details] [diff] [review]
patch (8th version)

This new version adds only minor changes with respect to the previous patch.
The modifications to the operator dictionary have been removed and are now available in a separate patch: attachment 442458 [details] [diff] [review].
Comment 61 Frédéric Wang (:fredw) 2010-04-30 10:12:15 PDT
Created attachment 442724 [details] [diff] [review]
Patch (9th version)

I've replaced the largeop factor: sqrt(2) in place of 2.
Comment 62 Joe Java 2010-05-13 11:23:26 PDT
Created attachment 445151 [details]
screen shot from https://eyeasme.com/Joe/MathML/MathML_browser_test_next

For the MathML browser testing site at
https://eyeasme.com/Joe/MathML/MathML_browser_test
the operators were manually set for a larger size.

A copy of this site with without operators being set larger is at
https://eyeasme.com/Joe/MathML/MathML_browser_test_next

The image is from this website.
The first formula in a row is from TeX, the second from Firefox 3.5
and the last for the latest trunk release.

Using the latest trunk build:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9.3a5pre) Gecko/20100513 Minefield/3.7a5pre

The operators scale well for large size fonts, but not small font sizes:
The font size is 24pt for "Test 1" and it looks great
The font size is 12pt for "Sophmore's Dream" and looks OK
The font size is 8pt for "Sphere Volume" but the integrals look bad.

Perhaps the algorithm for scaling should take the size of the font into account.
Comment 63 Joe Java 2010-05-13 11:26:39 PDT
Forgot to mention that the second formula shows the old manual setting of the operators to a larger size using FF 3.5 and the last formula in a row uses the new default operator scaling.
Comment 65 Frédéric Wang (:fredw) 2010-05-17 12:30:17 PDT
(In reply to comment #64)
> https://build.mozilla.org/tryserver-builds/fred.w...@free.fr-try-08c872a8d318/

https://build.mozilla.org/tryserver-builds/fred.wang@free.fr-try-08c872a8d318/ 

(In reply to comment #63)
> Forgot to mention that the second formula shows the old manual setting of the
> operators to a larger size using FF 3.5 and the last formula in a row uses the
> new default operator scaling.

Thank you for your comment, Joe. I don't think the issue with the size of largeop is related to the font size. The scale is relative to the base size, so it is automatically adapted. Rather, I guess the problem is that integrals are normally drawn larger than other largeops:

http://en.wikipedia.org/wiki/Sum_rule_in_integration

Hence I suggest adding a property "integral" in our operator dictionary and use a factor of 2 instead of sqrt(2) for largeop with that property.
Comment 66 Frédéric Wang (:fredw) 2010-05-17 12:31:32 PDT
Created attachment 445772 [details]
Bug tilde

I've also found a new bug with tilde. I suspect to be related to the negative bearings:

http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLmoverFrame.cpp#362
Comment 67 Joe Java 2010-05-17 13:41:09 PDT
Created attachment 445800 [details]
OverBar touches overscript

Using
Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.5; en-US; rv:1.9.3a5pre) Gecko/20100517 Minefield/3.7a5pre
on
https://eyeasme.com/Joe/MathML/MathML_browser_test_next
using STIX Beta font 
notice that the overbar touches overscript in the De Morgan's law test.
Comment 68 Joe Java 2010-05-17 13:45:56 PDT
Created attachment 445803 [details]
strange gaps in uparrow

Same setup as in Comment #67.
The uparrow gets stretched but "Gaps" occur.
Comment 69 Karl Tomlinson (:karlt) 2010-05-17 15:04:54 PDT
Joe, IIUC the issues in comments 67 and 68 are not related to Fred's build or this bug, but are a regression from some other change that has been made since Firefox 3.5.
They deserve a separate bug report for diagnosis of the change that caused the problem.
Comment 70 Joe Java 2010-05-20 05:24:36 PDT
Karl, thank you, you are correct these regressions do NOT seem to be because of this bug.  
I have filed bug 567090 for bug found in Comment #67 and bug 567091 for the bug in Comment #68
Comment 71 Frédéric Wang (:fredw) 2010-05-24 00:01:34 PDT
Created attachment 447042 [details] [diff] [review]
Patch v. 10
Comment 72 Frédéric Wang (:fredw) 2010-05-24 00:06:38 PDT
(In reply to comment #66)
> Created an attachment (id=445772) [details]
> Bug tilde
> 
> I've also found a new bug with tilde. I suspect to be related to the negative
> bearings:
> 
> http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLmoverFrame.cpp#362

This is not related to the patch of this bug either. I've reported bug 567718.
Comment 73 Frédéric Wang (:fredw) 2010-05-24 12:33:50 PDT
Created attachment 447147 [details] [diff] [review]
Patch v. 11

just fixing an ASSERT... No error found on the TryServer, but I don't know why no build is produced for Mac.

http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/fred.wang@free.fr-2c8ac775ab39
Comment 74 Frédéric Wang (:fredw) 2010-05-30 05:57:27 PDT
Comment on attachment 447147 [details] [diff] [review]
Patch v. 11

There does not seem to be much protest against the TryServer builds, so let's ask for a first review...
Comment 75 Joe Java 2010-05-30 12:48:49 PDT
Created attachment 448289 [details]
Triple integral looks strange

The latest TryServer build (24-May-2010) looks good on a Windows Vista machine.
I tried it on
https://www.eyeasme.com/Joe/MathML/MathML_browser_test_next
and it looks good, except for the triple integral in the sphere volume derivation.
Single integrals look good in the sphere volume derivation.
Comment 76 Frédéric Wang (:fredw) 2010-05-30 15:09:28 PDT
> except for the triple integral in the sphere volume derivation.

It seems to be a remaining instance of the "refresh" bug. Adding support for the STIXIntegrals* fonts fixes the issue for me.
Comment 77 Joe Java 2010-05-30 18:02:54 PDT
(In reply to comment #76)

> It seems to be a remaining instance of the "refresh" bug. Adding support for
> the STIXIntegrals* fonts fixes the issue for me.

OK, does this mean that the default settings for the "font.mathfont-family" preference will need to be changed for this bug to be fixed?  I am assuming that is what you meant by "Adding support for the STIXIntegrals* fonts".
Comment 78 Frédéric Wang (:fredw) 2010-05-31 02:21:43 PDT
> OK, does this mean that the default settings for the "font.mathfont-family"
> preference will need to be changed for this bug to be fixed?  I am assuming
> that is what you meant by "Adding support for the STIXIntegrals* fonts".

Yes, but it also requires a bit more of work, not configurable by the user. See bug 569195.
Comment 79 Karl Tomlinson (:karlt) 2010-06-07 22:19:08 PDT
Comment on attachment 447147 [details] [diff] [review]
Patch v. 11

It looks like leaving the code to set mDirection to NS_STRETCH_DIRECTION_UNSUPPORTED in nsMathMLChar::Stretch() after a stretch
http://hg.mozilla.org/mozilla-central/annotate/5b3604a3cfbe/layout/mathml/nsMathMLChar.cpp#l1745

may confuse some callers.
e.g.
http://hg.mozilla.org/mozilla-central/annotate/5b3604a3cfbe/layout/mathml/nsMathMLmoFrame.cpp#l793
http://hg.mozilla.org/mozilla-central/annotate/5b3604a3cfbe/layout/mathml/nsMathMLmfencedFrame.cpp#l499

>-  // quick return if there is nothing special about this char
>-  if (!mGlyphTable ||
>-      (aStretchDirection != direction &&
>-       aStretchDirection != NS_STRETCH_DIRECTION_DEFAULT) ||
>-      (aStretchHint & ~NS_STRETCH_MAXWIDTH) == NS_STRETCH_NONE) {
>-    return NS_OK;
>-  }
>-
>-  // if no specified direction, attempt to stretch in our preferred direction
>-  if (aStretchDirection == NS_STRETCH_DIRECTION_DEFAULT) {
>-    aStretchDirection = direction;
>-  }
>-
>   // see if this is a particular largeop or largeopOnly request
>   PRBool largeop = (NS_STRETCH_LARGEOP & aStretchHint) != 0;
>   PRBool stretchy = (NS_STRETCH_VARIABLE_MASK & aStretchHint) != 0;
>   PRBool largeopOnly = largeop && !stretchy;
> 
>   PRBool isVertical = (direction == NS_STRETCH_DIRECTION_VERTICAL);
> 
>   nscoord targetSize =
>     isVertical ? aContainerSize.ascent + aContainerSize.descent
>     : aContainerSize.rightBearing - aContainerSize.leftBearing;
> 
>+  mScaleY = mScaleX = 1.0;
>+  mTrueAscent = aDesiredStretchSize.ascent;
>+
>+  // quick return if there is nothing special about this char
>+  if ((aStretchDirection != direction &&
>+       aStretchDirection != NS_STRETCH_DIRECTION_DEFAULT) ||
>+      (aStretchHint & ~NS_STRETCH_MAXWIDTH) == NS_STRETCH_NONE) {
>+    return NS_OK;
>+  }
>+
>+  // if no specified direction, attempt to stretch in our preferred direction
>+  if (aStretchDirection == NS_STRETCH_DIRECTION_DEFAULT) {
>+    aStretchDirection = direction;
>+  }

Was there a reason for changing the order here?
(The old order made more sense to me.)

>+  // ensure a minimum targetSize in the case of a largeop in display mode
>+  if (largeop && (targetSize < largeopFactor * charSize)) {
>+    targetSize = largeopFactor * charSize;
>+  }
>+

I think it makes sense to have target largeop factor when we don't have font
support for the large variants.  However, these can only be heuristics to
guess what will be suitable.

As quirky as our "choose the largest size less than or equal to 2"
implementation is, it does provide a mechanism for the font to choose the
appropriate size and aspect-ratio changes, according to the particular
character.  If we have font support for a largeop, then I don't think we
should apply our own general heuristics as well.

>+      scale = float(largeopFactor *
>+                    (initialSize.ascent + initialSize.descent)) /
>+        (aDesiredStretchSize.ascent + aDesiredStretchSize.descent);

float() is not necessary here because largeopFactor is float, so
(initialSize.ascent + initialSize.descent) gets converted to float for the '*'
operator.

>+  // set the origin of the coordinate system to the center of the rectangle,
>+  // before applying any transform.
>+  aRenderingContext.Translate(r.x + dx, r.y + dy);
>+
>+  // scale the rectangle.
>+  aRenderingContext.Scale(mScaleX, mScaleY);
>+
>+  // set the origin of the coordinate system to the top left corner of the
>+  // bounding rectangle.
>+  dx /= mScaleX; dy /= mScaleY;
>+  aRenderingContext.Translate(-dx, -dy);

It would be simpler to just have a single Translate(r.x, r.y) followed by the
Scale(mScaleX, mScaleY).  Would that work fine?

>-    aRenderingContext.DrawString(mData.get(), len, mRect.x + aPt.x,
>-                                 mRect.y + aPt.y + mBoundingMetrics.ascent);
>+    aRenderingContext.PushState();
>+    nsRect r = mRect + aPt;
>+    ApplyTransforms(aRenderingContext, r);
>+    aRenderingContext.DrawString(mData.get(), len, 0, mTrueAscent);
>+    aRenderingContext.PopState();

Can a single PushState(); nsRect r = mRect + aPt;
ApplyTransforms(aRenderingContext, r); sequence be used around this set of
conditionals (instead of having the code repeated)?

>+  // mTrueAscent is the actual ascent of the char, i.e. not taking into account
>+  // the transform applied.
>+  nscoord            mTrueAscent;

What do you think about calling this "mUnscaledAscent"?

>-    if (NS_MATHML_IS_DISPLAYSTYLE(aPresentationData.flags) &&
>-        NS_MATHML_OPERATOR_IS_LARGEOP(aFlags)) {
>+    if (NS_MATHML_OPERATOR_IS_INTEGRAL(aFlags) ||
>+        (NS_MATHML_IS_DISPLAYSTYLE(aPresentationData.flags) &&
>+         NS_MATHML_OPERATOR_IS_LARGEOP(aFlags))) {
>       stretchHint = NS_STRETCH_LARGEOP; // (largeopOnly, not mask!)

This doesn't seem quite right to me.  I thought integrals would keep their
normal size when displaystyle="false".  Similarly, I would expect them to keep
their normal size when the author specifies largeop="false".
Comment 80 Frédéric Wang (:fredw) 2010-06-10 05:05:18 PDT
(In reply to comment #79)
> (From update of attachment 447147 [details] [diff] [review])
> It looks like leaving the code to set mDirection to
> NS_STRETCH_DIRECTION_UNSUPPORTED in nsMathMLChar::Stretch() after a stretch
> http://hg.mozilla.org/mozilla-central/annotate/5b3604a3cfbe/layout/mathml/nsMathMLChar.cpp#l1745
> 
> may confuse some callers.
> e.g.
> http://hg.mozilla.org/mozilla-central/annotate/5b3604a3cfbe/layout/mathml/nsMathMLmoFrame.cpp#l793
> http://hg.mozilla.org/mozilla-central/annotate/5b3604a3cfbe/layout/mathml/nsMathMLmfencedFrame.cpp#l499

Yes. Anyway, it's weird to keep direction = NS_STRETCH_DIRECTION_DEFAULT when we do actually stretch with the scaling method. I think I'm going to use a member mDrawNormal instead.

> I think it makes sense to have target largeop factor when we don't have font
> support for the large variants.  However, these can only be heuristics to
> guess what will be suitable.
> 
> As quirky as our "choose the largest size less than or equal to 2"
> implementation is, it does provide a mechanism for the font to choose the
> appropriate size and aspect-ratio changes, according to the particular
> character.  If we have font support for a largeop, then I don't think we
> should apply our own general heuristics as well.
 
I believe you're right. At least for the integral: it renders better with the STIX variants alone rather than the STIX variant + scale (because the aspect-ratio of the base char is used).

> It would be simpler to just have a single Translate(r.x, r.y) followed by the
> Scale(mScaleX, mScaleY).  Would that work fine?

I think the composition of transforms would be the same. The reason why I did that is to prepare other transforms which are best understood when having the origin at the center of the bounding box (mirroring and rotation). See for example the patch of bug 208309.

> Can a single PushState(); nsRect r = mRect + aPt;
> ApplyTransforms(aRenderingContext, r); sequence be used around this set of
> conditionals (instead of having the code repeated)?

I'm not sure whether they are all using the same nsRect.

> What do you think about calling this "mUnscaledAscent"?

Yes, it's fine (except if we add a rotation later).

> This doesn't seem quite right to me.  I thought integrals would keep their
> normal size when displaystyle="false".  Similarly, I would expect them to keep
> their normal size when the author specifies largeop="false".

As I said elsewhere, I think what I do is wrong. My intention was that integrals are drawn larger even when displaystyle="false". But I'm not sure.
Comment 81 Frédéric Wang (:fredw) 2010-06-10 14:12:58 PDT
Created attachment 450446 [details] [diff] [review]
Patch v12

Here is an updated version.

However, the integrals are drawn too small on Joe's test, when displaystyle=false:

Sphere volume
https://eyeasme.com/Joe/MathML/MathML_browser_test_next
Comment 82 Bill Gianopoulos [:WG9s] 2010-06-11 10:49:48 PDT
(In reply to comment #81)
> However, the integrals are drawn too small on Joe's test, when
> displaystyle=false:
> 
> Sphere volume
> https://eyeasme.com/Joe/MathML/MathML_browser_test_next

However it looks ok in his HTML5 version of the test:

https://eyeasme.com/Joe/MathML/HTML5_MathML_browser_test.html
Comment 83 Joe Java 2010-06-11 13:38:39 PDT
I know why.

In the HTML5 version 
https://eyeasme.com/Joe/MathML/HTML5_MathML_browser_test.html
the operators that have limits (Sums, Products, Integrals) have their 
font sizes set larger.

While in this (XHTML 1.1 plus MathML 2.0) version
https://eyeasme.com/Joe/MathML/MathML_browser_test_next
no operators are set to larger sizes

On of the goals of this bug is to automatically produce
aesthetically pleasing equations.  Operators with limits
generally look better when they are larger than the surrounding 
characters.  When this bug is fixed, the browser will have some
heuristics on when and how much to increase the size of some 
operators.
Comment 84 Frédéric Wang (:fredw) 2010-06-11 14:30:27 PDT
(In reply to comment #83)
> On of the goals of this bug is to automatically produce
> aesthetically pleasing equations.  Operators with limits
> generally look better when they are larger than the surrounding 
> characters.  When this bug is fixed, the browser will have some
> heuristics on when and how much to increase the size of some 
> operators.

The spec says:

"Specifies whether the operator is considered a largeop operator, that is, whether it should be drawn larger than normal when displaystyle="true" (similar to using TEX's \displaystyle). Examples of large operators include &int;  and &prod;."

"TEX's \displaystyle, \textstyle, \scriptstyle, and \scriptscriptstyle correspond to displaystyle and scriptlevel  as "true" and "0", "false" and "0", "false" and "1", and "false" and "2", respectively. Thus, math's display="block" corresponds to \displaystyle, while display="inline" corresponds to \textstyle."

In the "sphere volume" test, displaystyle=false so the largeops need not be drawn larger. However I was wondering if there should be a special case for integrals? What is TeX doing?

The factors I was considering to use as well as the start size:

-----------------------------------------------
| largeop | integral | width | height |  size |
|----------------------------------------------
|   no    |   no     |   1   |   1    |   0   |
|----------------------------------------------
|  yes    |   no     | sqrt2 | sqrt2  |   2   |
-----------------------------------------------
|  no     |   yes    |   1   | sqrt2  |   1   |
-----------------------------------------------
|  yes    |   yes    | sqrt2 |    2   |   2   |
-----------------------------------------------
Comment 85 Joe Java 2010-06-11 19:53:20 PDT
(In reply to comment #84)

Your ideas below look good.

> The factors I was considering to use as well as the start size:
> 
> -----------------------------------------------
> | largeop | integral | width | height |  size |
> |----------------------------------------------
> |   no    |   no     |   1   |   1    |   0   |
> |----------------------------------------------
> |  yes    |   no     | sqrt2 | sqrt2  |   2   |
> -----------------------------------------------
> |  no     |   yes    |   1   | sqrt2  |   1   |
> -----------------------------------------------
> |  yes    |   yes    | sqrt2 |    2   |   2   |
> -----------------------------------------------


I think that the "Sphere Volume" derivation should be in mode="display".

I have altered 
https://www.eyeasme.com/Joe/MathML/MathML_browser_test_next
to show this.  

The triple integral 
 <mo>&#x222D;<!-- &iiint; --></mo>
does not get set to the correct size, I have posted an image showing this.

TEX is the gold standard for mathematics representation.
The problem I have with default TEX is that it crams the lines too close (see the TEX rendering of the "Sphere Volume" derivation).  
FireFox MathML "display" mode does the opposite and spreads the lines too 
far apart. 

I am using the latest Windows trunk build, with the final STIX fonts installed
after the STIX Beta fonts (Beta fonts NOT uninstalled).
Comment 86 Joe Java 2010-06-11 19:54:59 PDT
Created attachment 450805 [details]
notice the triple integral in last colum is not correct size

Here is the image to go along with Comment #85.
Comment 87 Frédéric Wang (:fredw) 2010-06-11 23:10:30 PDT
> | largeop | integral | width | height |  size |

I meant display mode for the first column.

> I think that the "Sphere Volume" derivation should be in mode="display".

In your TeX source, you are using begin{align*} which I guess enables the display mode. So I think we don't need a special treatment for integrals in displaystyle=false i.e. what the patch currently does is correct.

> TEX is the gold standard for mathematics representation.
> The problem I have with default TEX is that it crams the lines too close (see
> the TEX rendering of the "Sphere Volume" derivation).  
> FireFox MathML "display" mode does the opposite and spreads the lines too 
> far apart. 

I'm not sure what is the "right" spacing... It is a separate issue anyway.

> The triple integral 
>  <mo>&#x222D;<!-- &iiint; --></mo>
> does not get set to the correct size, I have posted an image showing this.
> I am using the latest Windows trunk build, with the final STIX fonts installed
> after the STIX Beta fonts (Beta fonts NOT uninstalled).

Are you talking about a trunk build or Bill Gianopoulos's build? Normally, with the current set of patches (including those of bug 534970) this issue is fixed as said in comment 76.
Comment 88 Joe Java 2010-06-12 00:16:00 PDT
Created attachment 450825 [details]
MathML try build of 6_11_10

This image shows what I see on my machine.  As far as this bug is concerned things look OK.  I can see no more changes needed for this bug. 

Thank you! for the job well done.
Comment 89 Frédéric Wang (:fredw) 2010-06-12 01:05:25 PDT
Comment on attachment 450446 [details] [diff] [review]
Patch v12

For the remaining issues:

> It would be simpler to just have a single Translate(r.x, r.y) followed by the
> Scale(mScaleX, mScaleY).  Would that work fine?

Do you really want I do this change, given that I'll probably change back in subsequent work s.t. bug 208309?

> Can a single PushState(); nsRect r = mRect + aPt;
> ApplyTransforms(aRenderingContext, r); sequence be used around this set of
> conditionals (instead of having the code repeated)?

I think if I replace the return statement by a else, I can factor out PushState(); and PopState().
For the rectangle, most blocks are using mRect + aPt, but one is nsRect(aPt.x, aPt.y, mRect.width, mRect.height) so I'm not sure it would be relevant to factor out the other things.
Comment 90 Frédéric Wang (:fredw) 2010-06-12 06:05:07 PDT
Comment on attachment 450446 [details] [diff] [review]
Patch v12

>   NS_MATHML_OPERATOR_MOVABLELIMITS      = 1<<9,
>   NS_MATHML_OPERATOR_SYMMETRIC          = 1<<10,
>+  NS_MATHML_OPERATOR_INTEGRAL           = 1<<11,
> 
>   // Additional bits not stored in the dictionary
>   NS_MATHML_OPERATOR_MINSIZE_ABSOLUTE   = 1<<11,
>   NS_MATHML_OPERATOR_MAXSIZE_ABSOLUTE   = 1<<12,
>   NS_MATHML_OPERATOR_LEFTSPACE_ATTR     = 1<<13,
>   NS_MATHML_OPERATOR_RIGHTSPACE_ATTR    = 1<<14
> };

I forgot to increase the shifts here.
Comment 91 Frédéric Wang (:fredw) 2010-06-29 14:08:31 PDT
I think the ApplyTransforms should be static and used in nsDisplayMathMLSelectionRect, nsDisplayMathMLCharBackground, nsDisplayMathMLCharDebug classes.
Comment 92 Karl Tomlinson (:karlt) 2010-06-29 20:47:40 PDT
(In reply to comment #80)
> (In reply to comment #79)
> > It would be simpler to just have a single Translate(r.x, r.y) followed by the
> > Scale(mScaleX, mScaleY).  Would that work fine?
> 
> I think the composition of transforms would be the same. The reason why I did
> that is to prepare other transforms which are best understood when having the
> origin at the center of the bounding box (mirroring and rotation). See for
> example the patch of bug 208309.

(In reply to comment #89)
> Do you really want I do this change, given that I'll probably change back in
> subsequent work s.t. bug 208309?

For bug 208309, the mirroring can be done with a single Translate to the
right hand edge, followed by the Scale.

I think I prefer this approach.  There are fewer operations and
it avoids (minor) rounding issues with halving integer coords.

Things will get much more complicated with rotations, but I'm not sure that
translating to the middle is going to solve all the issues there, so I'd
prefer to leave that at this stage.

(In reply to comment #89)
> > Can a single PushState(); nsRect r = mRect + aPt;
> > ApplyTransforms(aRenderingContext, r); sequence be used around this set of
> > conditionals (instead of having the code repeated)?
> 
> I think if I replace the return statement by a else, I can factor out
> PushState(); and PopState().
> For the rectangle, most blocks are using mRect + aPt, but one is nsRect(aPt.x,
> aPt.y, mRect.width, mRect.height) so I'm not sure it would be relevant to
> factor out the other things.

That odd block is for composite chars which we don't use.

I'd be very happy if you'd like to remove the composite char code.

If you'd like to keep it, I don't think it can be working correctly as it is
now even, because it seems to be ignoring mRect.x/y as set on the parent.  It
seems that it should be passing aPt + mRect.TopLeft(), which would make it
consistent with the other blocks.

(In reply to comment #91)
> I think the ApplyTransforms should be static and used in
> nsDisplayMathMLSelectionRect, nsDisplayMathMLCharBackground,
> nsDisplayMathMLCharDebug classes.

This isn't necessary AFAIK.  I think these classes should just display as if
it were an unscaled regular variant or built-from-parts char.
Comment 93 Karl Tomlinson (:karlt) 2010-06-29 20:53:00 PDT
Comment on attachment 450446 [details] [diff] [review]
Patch v12

If a largeop is stretchy and a variant is found, it looks like this patch will
squish the operator down to the requested size.

Should largeops be only grown (not shrunk) in size, perhaps?

We seem to have a problem with stretchy largeops not being large even without
this patch.  It looks like nsMathMLChar::StretchEnumContext::TryVariants is
not forcing use of the larger variant unless largeoponly.
Comment 94 Frédéric Wang (:fredw) 2010-06-30 02:26:10 PDT
(In reply to comment #93)
> (From update of attachment 450446 [details] [diff] [review])
> If a largeop is stretchy and a variant is found, it looks like this patch will
> squish the operator down to the requested size.
> 
> Should largeops be only grown (not shrunk) in size, perhaps?
> 
> We seem to have a problem with stretchy largeops not being large even without
> this patch.  It looks like nsMathMLChar::StretchEnumContext::TryVariants is
> not forcing use of the larger variant unless largeoponly.

The standard properties regarding large operators become "largeop" but not "stretchy" after bug 534970. The "largeop"+"stretchy" case is only possible if user explicitly requests it.

I propose the following behavior:
- largeopOnly: use size=1. A scale increasing by a factor SQRT(2) is used when this variant does not exist.
- largeop: start with size=0 and use the flag asking to stretch greater, which is used for the square root sign. Apply the scale only when it increases the size (so when the stretch fails).

Currently for largeops, size=2 is used when it exists. For Asana Math, this variant seems to be too large compared to STIX font or to the SQRT(2) heuristic.
Comment 95 Karl Tomlinson (:karlt) 2010-06-30 15:56:00 PDT
(In reply to comment #94)
> I propose the following behavior:
> - largeopOnly: use size=1. A scale increasing by a factor SQRT(2) is used when
> this variant does not exist.

That sounds fine with me.

> - largeop: start with size=0 and use the flag asking to stretch greater, which
> is used for the square root sign.

I don't feel so good about this.  Consistency with largeopOnly makes more sense to me here.  Also NS_STRETCH_LARGER is not strictly greater but can be equal, I think.  Using NS_STRETCH_LARGER may make sense though.

Both of these are independent of this bug though.

> Apply the scale only when it increases the
> size (so when the stretch fails).

This part I think should be included in the patch here.
Comment 96 Joe Java 2010-07-12 16:52:41 PDT
Not sure if this is part of this bug, but in
http://www.w3.org/Math/testsuite/mml2-testsuite/index.html
Presentation -> TablesAndMatrices -> mtable 
test mtableBsize2
fails.  The arrows fail to automatically grow to fill the 
space allowed.

Should I file a new bug to cover this?

I have altered 
https://www.eyeasme.com/Joe/MathML/MathML_browser_test_next
"Cichoń's Diagram" to depend on this bug being fixed.
Comment 97 Frédéric Wang (:fredw) 2010-07-12 23:17:08 PDT
(In reply to comment #96)
I'm going to answer in bug 557086.
Comment 98 Karl Tomlinson (:karlt) 2010-07-28 21:59:12 PDT
The "refresh" bug observed here is most likely bug 518172.
Comment 99 Frédéric Wang (:fredw) 2010-07-28 23:38:38 PDT
(In reply to comment #98)
> The "refresh" bug observed here is most likely bug 518172.

I think there are actually two "refresh" bugs. The first happened only on Linux but has been fixed by the upgrade of Cairo. It remains some problems with radical symbols and characters drawn with the "normal drawing" (using the terminology of the source code). These remaining problems happen on other platforms. For instance Joe saw it for the triple integral (which disappeared when we use the glyph from STIX fonts).
Comment 100 Frédéric Wang (:fredw) 2010-08-01 14:07:33 PDT
Created attachment 461929 [details] [diff] [review]
Patch v13

Just an update to address the comments above. The patch is untested.
Comment 101 Frédéric Wang (:fredw) 2010-08-02 15:20:46 PDT
Comment on attachment 461929 [details] [diff] [review]
Patch v13

The remaining "refresh" bug is gone after the resolution of bug 518172.
Comment 102 Karl Tomlinson (:karlt) 2010-08-03 21:49:38 PDT
Comment on attachment 461929 [details] [diff] [review]
Patch v13

Thanks very much!
Comment 103 Karl Tomlinson (:karlt) 2010-08-03 22:19:32 PDT
This bug depends on bug 534970 (as well as bug 524275), right?

Is anything else needed?
Comment 104 Frédéric Wang (:fredw) 2010-08-04 01:14:10 PDT
(In reply to comment #103)
> This bug depends on bug 534970 (as well as bug 524275), right?
> 
> Is anything else needed?

Yes, this bug must be pushed together with the 6 patches of bug 534970 and the patch of bug 524275.
Comment 105 Karl Tomlinson (:karlt) 2010-08-08 22:14:07 PDT
Comment on attachment 461929 [details] [diff] [review]
Patch v13

Requesting approval2.0.
This significantly improves the MathML rendering for users without STIX fonts installed (and even improves rendering a little with STIX fonts installed).
Effects are limited to MathML.  Some more details are at http://groups.google.com/group/mozilla.dev.tech.mathml/browse_thread/thread/59d7a24aa44771b1
Comment 106 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-08-10 09:24:44 PDT
Comment on attachment 461929 [details] [diff] [review]
Patch v13

No tests?
Comment 107 Frédéric Wang (:fredw) 2010-08-10 12:32:31 PDT
(In reply to comment #106)
> Comment on attachment 461929 [details] [diff] [review]
> Patch v13
> 
> No tests?

If you mean reftests, I think it would be difficult for this bug.
Comment 108 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2010-08-10 12:35:45 PDT
Why would it be difficult? Is it not possible to create an SVG reference image with similar properties to the stretched glyphs here? Or do all the test machines have the STIX font and therefore we can't test this fallback codepath?

Why can't we just ship the STIX font as part of Firefox?
Comment 109 Karl Tomlinson (:karlt) 2010-08-10 17:43:20 PDT
(In reply to comment #108)
> Why would it be difficult? Is it not possible to create an SVG reference image
> with similar properties to the stretched glyphs here? Or do all the test
> machines have the STIX font and therefore we can't test this fallback codepath?
> 
> Why can't we just ship the STIX font as part of Firefox?

Bug 295193 covers supplying STIX fonts.  I'm not sure about Fedora 12 but I don't think any of the test machines have STIX fonts.

The difficulty with reftests and fonts is variations in rendering (which won't be matched by SVG).

Frédéric, maybe a reftest could check that a stretchy operator does not stretch more than a couple of pixels too far?
Perhaps two mathematical expressions with the same absolute position, equivalent but one with a stretchy operator and the other stretchy=false.  The stetchy=false expression has background the same color as foreground and a border of two or three pixels.  The stretchy=true expression has the same foreground color but no (transparent) background.  The reference only has the stretchy=false expression to check that the stretchy=true expression does not extend beyond the border of the stretchy=false expression.

Perhaps a reftest could also check that a stretchy operator does stretch, though this would be a bit harder I guess.  Maybe a visible stretchy operator and the rest of the expression is tall but made invisible by making the foreground color the same as the background.  I think it would need to be a != reftest, which reduces the usefulness, but could test that the stretchy attribute has some effect.
Comment 110 Frédéric Wang (:fredw) 2010-08-11 00:14:35 PDT
OK, I think I can do the second reftest. But for the first one, I'm not sure to understand why you want the stretchy=true expression to be close to the stretch=false? A priori, stretchy=true can be much more larger than the stretch=false.
Comment 111 Karl Tomlinson (:karlt) 2010-08-11 01:04:22 PDT
For vertical stretching, I'm expecting a stretchy operator to not stretch (more than a few pixels) more than the tallest unstretched character.

I probably didn't make it clear but I imagined the stretchy=false and =true expressions to be in the same test file.  The stretchy=false expression and its background/border would form a block to "black"-out the stretchy=true expression unless it stretched beyond the border.
Comment 112 Frédéric Wang (:fredw) 2010-08-11 08:09:08 PDT
Created attachment 464827 [details] [diff] [review]
add reftests

I'm not sure that it is exactly what you expected, but I've done something similar. There are two kinds of superpositions:

  1) the stretchy operator is covered by a red rectangle slightly longer than the expected size (== test)
  2) the stretchy operator is covered by a green rectangle slightly shorter than the expected size (!= test)

I've made these tests for both vertical and horizontal stretching. Moreover, I've written a != test for largeop stretching i.e. to check that a largeop is really drawn larger in display mode.

I've used triple arrows and quadruple integral, for which stretching fails without this patch (whatever the fonts installed).
Comment 113 Karl Tomlinson (:karlt) 2010-08-11 17:05:08 PDT
That looks great, thanks, better than what I had in mind.
The patch seems to be missing the files for scale-stretchy-4/5 though.
Do you have those?
Comment 114 Frédéric Wang (:fredw) 2010-08-11 23:41:12 PDT
Created attachment 465143 [details] [diff] [review]
add reftests

oops, sorry.
Comment 115 Karl Tomlinson (:karlt) 2010-08-12 20:46:04 PDT
Comment on attachment 465143 [details] [diff] [review]
add reftests

(No approval necessary.)
Comment 117 [not reading bugmail] 2010-08-20 01:22:10 PDT
Not bad, looks like it does on paper :)

Mozilla/5.0 (Windows NT 6.1; rv:2.0b5pre) Gecko/20100819 Minefield/4.0b5pre ID:20100819224140
Comment 118 Karl Tomlinson (:karlt) 2010-10-13 13:49:58 PDT
*** Bug 603927 has been marked as a duplicate of this bug. ***
Comment 119 Frédéric Wang (:fredw) 2010-11-19 21:35:34 PST
*** Bug 613676 has been marked as a duplicate of this bug. ***

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