Summary: | MathML renderers should use struct to pass long list of LayoutUnits | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Frédéric Wang (:fredw) <fred.wang> | ||||
Component: | MathML | Assignee: | Frédéric Wang (:fredw) <fred.wang> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, dbarton, esprehn+autocc, glenn, kondapallykalyan | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Frédéric Wang (:fredw)
2016-08-23 09:04:08 PDT
Created attachment 286718 [details]
Patch
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. Committed r205105: <http://trac.webkit.org/changeset/205105> (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). |