Bug 161084 - MathML renderers should use struct to pass long list of LayoutUnits
Summary: MathML renderers should use struct to pass long list of LayoutUnits
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:
Blocks:
 
Reported: 2016-08-23 09:04 PDT by Frédéric Wang (:fredw)
Modified: 2016-08-28 10:52 PDT (History)
5 users (show)

See Also:


Attachments
Patch (41.82 KB, patch)
2016-08-23 09:09 PDT, Frédéric Wang (:fredw)
darin: 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-08-23 09:04:08 PDT
This was suggested by Darin in a previous bug.
Comment 1 Frédéric Wang (:fredw) 2016-08-23 09:09:28 PDT
Created attachment 286718 [details]
Patch
Comment 2 Darin Adler 2016-08-27 17:36:02 PDT
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.
Comment 3 Frédéric Wang (:fredw) 2016-08-28 10:38:00 PDT
Committed r205105: <http://trac.webkit.org/changeset/205105>
Comment 4 Frédéric Wang (:fredw) 2016-08-28 10:42:13 PDT
(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).