RenderSVGRoot has some unneeded includes and unneeded if tests, clean that up.
Created attachment 27445 [details] Cleanup patch Straightforward cleanup of RenderSVGRoot.cpp. Cheers, Rob.
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.
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.