Bug 160077 – Painting issues with menclose and reflow / repaint

• Status: RESOLVED
• Resolution: FIXED
• Component: MathML

[Frédéric Wang (:fredw)]

Created attachment 284311 [details]
Testcase Dirac Equation

The following issue has been reported by Jacques Distler on his Instiki sandbox page: "Multiple missing symbols in the Dirac Equation.". I'm not exactly sure when it happens, it seems that his page currently uses MathJax for WebKit, which might causing unnecessary reflow or style that make the analysis complicated.

I have isolated a testcase, but I'm not sure it is fully reduced (the bug does not show up for a single equation for example). I noticed that changing the CSS font-size or selecting the content causes the 0 or the operators to disappear. It seems that the menclose element is also involved.

[Frédéric Wang (:fredw)]

Created attachment 284318 [details]
Patch

[Brent Fulgham]

Comment on attachment 284318 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284318&action=review

I am confident your patch fixes the problem you describe, but I think we should try to fix it in GraphicsContextStateSaver, unless we discover that GCSS explicitly avoids protecting the paint rect.

> Source/WebCore/ChangeLog:9
> +        restored by GraphicsContextStateSaver. To avoid some weird rendering bugs in MathOperator

Doesn't it seem like the problem we should be solving is why GraphicsContextStateSaver isn't working properly?

> Source/WebCore/ChangeLog:11
> +        original PaintInfo before applying the transform.

Perhaps there is no performance difference in ensuring that GraphicsContextStateSaver does the right thing, versus your (and SVG's) solution of duplicating the PaintInfo before making adjustments. Still, this seems like a bad pattern, since we presumable knew about the problem when it was fixed in SVG, but did not realize the problem existed here as well.

It sounds like we are very likely to have even more cases of this issue in our code!

[Brent Fulgham]

Comment on attachment 284318 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=284318&action=review

>> Source/WebCore/ChangeLog:11
>> +        original PaintInfo before applying the transform.
> 
> Perhaps there is no performance difference in ensuring that GraphicsContextStateSaver does the right thing, versus your (and SVG's) solution of duplicating the PaintInfo before making adjustments. Still, this seems like a bad pattern, since we presumable knew about the problem when it was fixed in SVG, but did not realize the problem existed here as well.
> 
> It sounds like we are very likely to have even more cases of this issue in our code!

Wait -- I'm way off base here. The GraphicsContextStateSaver only knows about the context, so of course it can't protect the rect.

It seems like your solution of copying the PaintInfo and transforming the copy is the right approach, though I do worry that this pattern may not be handled properly in other parts of the code resulting in weird drawing problems.

[Brent Fulgham]

Zalan: The bug Frédéric discovered here seems likely to exist in other parts of our code. Do we have some way to automate the process of protecting the drawing state when doing a transform like this code?

Since this was something found and fixed in SVG, and now again in MathML, I'm worried we have other cases where this is being handled improperly.

[WebKit Commit Bot]

Comment on attachment 284318 [details]
Patch

Clearing flags on attachment: 284318

Committed r203639: <http://trac.webkit.org/changeset/203639>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Frédéric Wang (:fredw)]

I agree with Brent, but does not see an easy way to fix it (unless we write such a save/restore mechanism for the paintInfo too).

For the menclose, we could also have gotten rid of the applyTransfrom since that's just a simple translate (that's how I initially fixed this bug). However, I did notsee how to do that for MathOperator as the transform is more complex, so just copied SVG in the two cases for consistency.