Bug 161084 – MathML renderers should use struct to pass long list of LayoutUnits

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

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

This was suggested by Darin in a previous bug.

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

Created attachment 286718 [details]
Patch

[Darin Adler]

Comment on attachment 286718 [details]
Patch

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

> Source/WebCore/rendering/mathml/RenderMathMLFraction.h:69
> +    void getFractionParameters(FractionParameters&);

Given modern C++ and return value optimizations, you should consider just having this be:

    FractionParameters fractionParameters() const;

No reason it needs to be a get function; move is quite efficient.

> Source/WebCore/rendering/mathml/RenderMathMLFraction.h:75
> +    void getStackParameters(StackParameters&);

Ditto.

> Source/WebCore/rendering/mathml/RenderMathMLScripts.h:65
> +    void getScriptMetricsAndLayoutIfNeeded(RenderBox* base, RenderBox* script, ScriptMetrics&);

Ditto. Return value instead of get style. Also, can these arguments be references instead of pointers?

> Source/WebCore/rendering/mathml/RenderMathMLUnderOver.h:69
> +    void getVerticalParameters(VerticalParameters&) const;

Same here.

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

Committed r205105: <http://trac.webkit.org/changeset/205105>

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

(In reply to comment #2)
> > Source/WebCore/rendering/mathml/RenderMathMLScripts.h:65
> > +    void getScriptMetricsAndLayoutIfNeeded(RenderBox* base, RenderBox* script, ScriptMetrics&);
> 
> Ditto. Return value instead of get style. Also, can these arguments be
> references instead of pointers?

That one is a bit complicated so I'll handle it in follow-up patches. Basically getScriptMetricsAndLayoutIfNeeded does three things: it retrieves MATH parameters, it calls layoutIfNeed on all the scripts and finally it determines the minimal shift/size of each script & take the maximum of the values (they are two separate calls for the pre and post script lists). I think this should be reorganized a bit (splitting the three parts and make the maximum of pre/post more explicit).