[Webkit-unassigned] [Bug 11272] Implement currentScale(), setCurrentScale() and currentTranslate() in SVGSVGElement

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 30 07:50:04 PDT 2007


http://bugs.webkit.org/show_bug.cgi?id=11272


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #14782|review?                     |review-
               Flag|                            |




------- Comment #10 from darin at apple.com  2007-05-30 07:50 PDT -------
(From update of attachment 14782)
I don't think we should necessarily mix the zoom factor together with the text
size multiplier. The text size multiplier is still called "zoomFactor" in
Frame.cpp, but I think we may want a separate way to manipulate the zoom factor
and text size multiplier independently. And I think a zoom factor should zoom
everything, not just SVG content.

+    if (document() && document()->frame())
+        return float(document()->frame()->zoomFactor()) / 100.;
+    return 1.0;

The right way to write the above so it will be floating point is:

    if (document() && document()->frame())
        return document()->frame()->zoomFactor() / 100.0f;
    return 1.0f;

There's no need for the cast to "float".

+        document()->frame()->setZoomFactor(scale / 100.);

This will do the math as double. You want "scale / 100.0f".

The word "translate" is a verb not a noun. It should be "translation" instead
in the various function names.

-    //canvasView()->enableZoomAndPan(zoomAndPan == SVG_ZOOMANDPAN_MAGNIFY);

We normally don't check in commented-out code. if we must check this in, it
needs a comment to indicate why it's commented out.

         return m_lastScrollbarUnderMouse->handleMouseMoveEvent(mouseEvent);
-
     // Treat mouse move events while the mouse is pressed as "read-only" in
prepareMouseEvent

Why remove this blank line?

+       static_cast<SVGDocument*>(m_frame->document())->zoomEnabled()) {

Why does zoomEnabled() control panning? Should it be named zoomAndPanEnabled()
instead?

+        AffineTransform ctm;

This local variable is unused?

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

No need for the cast to float here.

review- at least for the "translate" name.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list