[webkit-reviews] review granted: [Bug 11272] Implement currentScale(), setCurrentScale() and currentTranslate() in SVGSVGElement : [Attachment 14784] Cleaning up

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Rob Buis <rwlbuis at gmail.com>'s
request for review:
Bug 11272: Implement currentScale(), setCurrentScale() and currentTranslate()
in SVGSVGElement
http://bugs.webkit.org/show_bug.cgi?id=11272

Attachment 14784: Cleaning up
http://bugs.webkit.org/attachment.cgi?id=14784&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+	 rootElement()->setCurrentTranslate(FloatPoint(pos.x() -
m_translate.x(), m_translate.y() - pos.y()));
+	 if (renderer())
+	     renderer()->repaint();

Shouldn't setCurrentTranslate be the code doing the repaint?

+	 FloatPoint m_translate;

I think this should be removed.

+    if (m_svgPan) {
+	 m_svgPan = false;
+	 return true;
+    }

I think we want to do a final "updatePan" at the end location.

+	 m_width = float(m_width) * svg->currentScale();
+	 m_height = float(m_height) * svg->currentScale();

This still has the cast, even though you mentioned removing it in your
comments.

review+, although I think this needs a little refinement