Bug 23816 – Clean up RenderSVGRoot.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Rob Buis]

RenderSVGRoot has some unneeded includes and unneeded if tests, clean that up.

[Rob Buis]

Created attachment 27445 [details]
Cleanup patch

Straightforward cleanup of RenderSVGRoot.cpp.
Cheers,

Rob.

[Eric Seidel (no email)]

Comment on attachment 27445 [details]
Cleanup patch

Looks fine.  You could even handle the hasSetContainerSize() case up front.  That would result in more code, but might be cleaner.

if (svg-> hasSetContainerSize()) { 
   m_viewport = FloatRect(0, 0, width.value(svg), height.value(svg));
   return;
}

Then the rest could be a terinary:

w = (width.unitType() == LengthTypePercent) ? svg->relativeWidthValue() : width.value(svg);
h = (height.unitType() == LengthTypePercent) ? svg->relativeHeightValue() : height.value(svg);
m_viewport = FloatRect(0, 0, w, h);

I'm not sure if that's cleaner or not though.  Either way, looks great and doesn't need any further review.

[Rob Buis]

Hi Eric,

(In reply to comment #2)
> (From update of attachment 27445 [details] [review])
> Looks fine.  You could even handle the hasSetContainerSize() case up front. 
> That would result in more code, but might be cleaner.
> 
> if (svg-> hasSetContainerSize()) { 
>    m_viewport = FloatRect(0, 0, width.value(svg), height.value(svg));
>    return;
> }
> 
> Then the rest could be a terinary:
> 
> w = (width.unitType() == LengthTypePercent) ? svg->relativeWidthValue() :
> width.value(svg);
> h = (height.unitType() == LengthTypePercent) ? svg->relativeHeightValue() :
> height.value(svg);
> m_viewport = FloatRect(0, 0, w, h);
> 
> I'm not sure if that's cleaner or not though.  Either way, looks great and
> doesn't need any further review.

Thanks for the thumbs up, I did try to clean it up that way in my final commit.
Cheers,

Rob.