Refactor Magnifier.ZoomRegion to avoid permanent Clutter.Clone

This basic point of this change is to avoid always creating a
hidden Clutter.Clone actor for the default present-but-not-active
zoom region. The position of the viewport and region of interest
are now stored in member variables, and the actors are only created
and updated when the region is active.

Other significant changes:

 * Unused public functions are removed or made private
 * The mouse tracking position is immediately  updated when options
   like the zoom are changed, not just on the next mouse motion.
 * ZoomRegion.setROI() now updates the zoom, not just the position;
   a FIXME is added to the D-Bus interface for a place where the
   D-Bus interface contains duplicate possibly conflicting information
 * Lens-mode is now only effectively off when the magnifier is
   fullscreen, instead of actually modifying the member variable;
   this makes things work properly when changing out of full-screen
   mode.
 * When the clamping to screen edges is turned on, we now immediately
   clamp.
 * The handling of setting the position to fullscreen as compared
   to just setting the viewport to fullscreen is untangled.

https://bugzilla.gnome.org/show_bug.cgi?id=633582

js/ui/magnifier.js

@@ -10,6 +10,7 @@ const Signals = imports.signals;
 
 const Main = imports.ui.main;
 const MagnifierDBus = imports.ui.magnifierDBus;
+const Params = imports.misc.params;
 
 // Keep enums in sync with GSettings schemas
 const MouseTrackingMode = {
@@ -212,9 +213,14 @@ Magnifier.prototype = {
      */
     createZoomRegion: function(xMagFactor, yMagFactor, roi, viewPort) {
         let zoomRegion = new ZoomRegion(this, this._cursorRoot);
-        zoomRegion.setMagFactor(xMagFactor, yMagFactor);
         zoomRegion.setViewPort(viewPort);
-        zoomRegion.setROI(roi);
+
+        // We ignore the redundant width/height on the ROI
+        let fixedROI = new Object(roi);
+        fixedROI.width = viewPort.width / xMagFactor;
+        fixedROI.height = viewPort.height / yMagFactor;
+        zoomRegion.setROI(fixedROI);
+
         zoomRegion.addCrosshairs(this._crossHairs);
         return zoomRegion;
     },
@@ -247,22 +253,9 @@ Magnifier.prototype = {
      * Remove all the zoom regions from this Magnfier's ZoomRegion list.
      */
     clearAllZoomRegions: function() {
-        // First ZoomRegion is special since its magnified mouse and crosshairs
+        for (let i = 0; i < this._zoomRegions.length; i++)
-        // are the original -- all the others are Clutter.Clone's.  Deal with
-        // all but first zoom region.
-        for (let i = 1; i < this._zoomRegions.length; i++) {
             this._zoomRegions[i].setActive(false);
-            this._zoomRegions[i].removeFromStage();
-        }
-        this._zoomRegions[0].setActive(false);
 
-        // Detach the (original) magnified mouse and cross hair for later reuse
-        // before removing ZoomRegion from the stage.
-        this._cursorRoot.get_parent().remove_actor(this._cursorRoot);
-        if (this._crossHairs)
-            this._crossHairs.removeFromParent();
-
-        this._zoomRegions[0].removeFromStage();
         this._zoomRegions.length = 0;
         this.stopTrackingMouse();
         this.showSystemCursor();
@@ -567,44 +560,35 @@ Magnifier.prototype = {
 };
 Signals.addSignalMethods(Magnifier.prototype);
 
-function ZoomRegion(magnifier, mouseRoot) {
+function ZoomRegion(magnifier, mouseSourceActor) {
-    this._init(magnifier, mouseRoot);
+    this._init(magnifier, mouseSourceActor);
 }
 
 ZoomRegion.prototype = {
-    _init: function(magnifier, mouseRoot) {
+    _init: function(magnifier, mouseSourceActor) {
         this._magnifier = magnifier;
 
-        // The root actor for the zoom region
+        this._mouseTrackingMode = MouseTrackingMode.NONE;
-        this._magView = new St.Bin({ style_class: 'magnifier-zoom-region', x_fill: true, y_fill: true });
+        this._clampScrollingAtEdges = false;
-        global.stage.add_actor(this._magView);
+        this._lensMode = false;
-        this._magView.hide();
+        this._screenPosition = ScreenPosition.FULL_SCREEN;
 
-        // Append a Clutter.Group to clip the contents of the magnified view.
+        this._magView = null;
-        this._mainGroup = new Clutter.Group({ clip_to_allocation: true });
+        this._uiGroupClone = null;
-        this._magView.set_child(this._mainGroup);
+        this._mouseSourceActor = mouseSourceActor;
-
+        this._mouseActor  = null;
-        // Add a background for when the magnified uiGroup is scrolled
-        // out of view (don't want to see desktop showing through).
-        let background = new Clutter.Rectangle({ color: Main.DEFAULT_BACKGROUND_COLOR });
-        this._mainGroup.add_actor(background);
-
-        // Clone the group that contains all of UI on the screen.  This is the
-        // chrome, the windows, etc.
-        this._uiGroupClone = new Clutter.Clone({ source: Main.uiGroup });
-        this._mainGroup.add_actor(this._uiGroupClone);
-        Main.uiGroup.set_size(global.screen_width, global.screen_height);
-        background.set_size(global.screen_width, global.screen_height);
-        this._uiGroupClone.set_size(global.screen_width, global.screen_height);
-
-        // Add either the given mouseRoot to the ZoomRegion, or a clone of
-        // it.
-        if (mouseRoot.get_parent() != null)
-            this._mouseRoot = new Clutter.Clone({ source: mouseRoot });
-        else
-            this._mouseRoot = mouseRoot;
-        this._mainGroup.add_actor(this._mouseRoot);
         this._crossHairs = null;
+        this._crossHairsActor = null;
+
+        this._viewPortX = 0;
+        this._viewPortY = 0;
+        this._viewPortWidth = global.screen_width;
+        this._viewPortWidth = global.screen_height;
+        this._xCenter = this._viewPortWidth / 2;
+        this._yCenter = this._viewPortHeight / 2;
+        this._xMagFactor = 1;
+        this._yMagFactor = 1;
+        this._followingCursor = false;
     },
 
     /**
@@ -612,14 +596,16 @@ ZoomRegion.prototype = {
      * @activate:   Boolean to show/hide the ZoomRegion.
      */
     setActive: function(activate) {
-        if (activate) {
+        if (activate && !this.isActive()) {
-            this._magView.show();
+            this._createActors();
-            if (this.isMouseOverRegion())
+            if (this._isMouseOverRegion())
                 this._magnifier.hideSystemCursor();
-            this._updateMousePosition(false /* mouse didn't move */);
+            this._updateMagViewGeometry();
+            this._updateCloneGeometry();
+            this._updateMousePosition();
+        } else if (!activate && this.isActive()) {
+            this._destroyActors();
         }
-        else
-            this._magView.hide();
     },
 
     /**
@@ -627,18 +613,7 @@ ZoomRegion.prototype = {
      * @return  Whether this ZoomRegion is active (boolean).
      */
     isActive: function() {
-        return this._magView.visible;
+        return this._magView != null;
-    },
-
-    /**
-     * removeFromStage:
-     * Remove the magnified view from the stage.
-     */
-    removeFromStage: function() {
-        global.stage.remove_actor(this._magView);
-        this._mouseRoot = null;
-        this._uiGroupClone = null;
-        this._magView = null;
     },
 
     /**
@@ -650,19 +625,9 @@ ZoomRegion.prototype = {
      *                  of the magnified view.
      */
     setMagFactor: function(xMagFactor, yMagFactor) {
-        if (xMagFactor > 0 && yMagFactor > 0) {
+        this._changeROI({ xMagFactor: xMagFactor,
-            // Changing the mag factor moves the pixels along the axes of
+                          yMagFactor: yMagFactor,
-            // magnification.  Set the view back to the point that was at the centre
+                          redoCursorTracking: this._followingCursor });
-            // of the region of interest.
-            let [x, y, width, height] = this.getROI();
-            let xCentre = x + width / 2;
-            let yCentre = y + height / 2;
-            this._uiGroupClone.set_scale(xMagFactor, yMagFactor);
-            this._mouseRoot.set_scale(xMagFactor, yMagFactor);
-            this._calcRightBottomStops();
-            this._scrollToPosition(xCentre, yCentre);
-            this._updateMousePosition(false /* mouse didn't move */);
-        }
     },
 
     /**
@@ -673,7 +638,7 @@ ZoomRegion.prototype = {
      *          in size, and so on.
      */
     getMagFactor: function() {
-        return this._uiGroupClone.get_scale();
+        return [this._xMagFactor, this._yMagFactor];
     },
 
     /**
@@ -696,29 +661,12 @@ ZoomRegion.prototype = {
     /**
      * setViewPort
      * Sets the position and size of the ZoomRegion on screen.
-     * @viewPort:   Object defining the position and size of the view port.  It
+     * @viewPort:   Object defining the position and size of the view port.
-     *              has the form { x, y, width, height }.  The values are in
+     *              It has members x, y, width, height.  The values are in
      *              stage coordinate space.
      */
     setViewPort: function(viewPort) {
-        let [xRoi, yRoi, wRoi, hRoi] = this.getROI();
+        this._setViewPort(viewPort);
-
-        // Remove border if the view port is the entire screen.  Otherwise,
-        // ensure that the border is there.
-        if (viewPort.x == 0 && viewPort.y == 0 && viewPort.width == global.screen_width && viewPort.height == global.screen_height)
-            this._magView.add_style_class_name('full-screen');
-        else
-            this._magView.remove_style_class_name('full-screen');
-
-        this.setSize(viewPort.width, viewPort.height);
-        this.setPosition(viewPort.x, viewPort.y);
-        if (this._crossHairs)
-            this._crossHairs.reCenter();
-
-        this.scrollContentsTo(xRoi + wRoi / 2, yRoi + hRoi / 2);
-        if (this.isMouseOverRegion())
-            this._magnifier.hideSystemCursor();
-
         this._screenPosition = ScreenPosition.NONE;
     },
 
@@ -726,88 +674,18 @@ ZoomRegion.prototype = {
      * setROI
      * Sets the "region of interest" that the ZoomRegion is magnifying.
      * @roi:    Object that defines the region of the screen to magnify.  It
-     *          has the form { x, y, width, height }.  The values are in
+     *          has members x, y, width, height.  The values are in
      *          screen (unmagnified) coordinate space.
      */
     setROI: function(roi) {
-        let xRoiCenter = roi.x + roi.width  / 2;
+        if (roi.width <= 0 || roi.height <= 0)
-        let yRoiCenter = roi.y + roi.height / 2;
+            return;
-        this.scrollContentsTo(xRoiCenter, yRoiCenter);
-    },
 
-    /**
+        this._followingCursor = false;
-     * setSize:
+        this._changeROI({ xMagFactor: this._viewPortWidth / roi.width,
-     * @width:    The width to set the magnified view to.
+                          yMagFactor: this._viewPortHeight / roi.height,
-     * @height:   The height to set the magnified view to.
+                          xCenter: roi.x + roi.width  / 2,
-     */
+                          yCenter: roi.y + roi.height / 2 });
-    setSize: function(width, height) {
-        this._magView.set_size(width, height);
-        this._calcRightBottomStops();
-    },
-
-    /**
-     * getSize:
-     * @return  an array, [width, height], that specifies the size of the
-     *          magnified view.
-     */
-    getSize: function() {
-        return this._magView.get_size();
-    },
-
-    /**
-     * setPosition:
-     * Position the magnified view at the given coordinates.
-     * @x:    The x-coord of the new position.
-     * @y:    The y-coord of the new position.
-     */
-    setPosition: function(x, y) {
-        let [width, height] = this._magView.get_size();
-        if (this._clampScrollingAtEdges) {
-            // Restrict positioning so view doesn't go beyond any edge of the
-            // screen.
-            if (x < 0)
-                x = 0;
-            if (x + width > global.screen_width)
-                x = global.screen_width - width;
-            if (y < 0)
-                y = 0;
-            if (y + height > global.screen_height)
-                y = global.screen_height - height;
-        }
-        this._magView.set_position(x, y);
-    },
-
-    /**
-     * getPosition:
-     * @return  an array, [x, y], that gives the position of the
-     *          magnified view on screen.
-     */
-    getPosition: function() {
-        return this._magView.get_position();
-    },
-
-    /**
-     * getCenter:
-     * @return  an array, [x, y], that is half the width and height of the
-     *          magnified view (the center of the magnified view).
-     */
-    getCenter: function() {
-        let [width, height] = this._magView.get_size();
-        return [width / 2, height / 2];
-    },
-
-    /**
-     * isFullScreenMode:
-     * Does the magnified view occupy the whole screen?
-     */
-    isFullScreenMode: function() {
-        let [x, y] = this._magView.get_position();
-        if (x != 0 || y != 0)
-            return false;
-        [width, height] = this._magView.get_size();
-        if (width != global.screen_width || height != global.screen_height)
-            return false;
-        return true;
     },
 
     /**
@@ -819,24 +697,23 @@ ZoomRegion.prototype = {
      *          rectangle of what is shown in the magnified view.
      */
     getROI: function() {
-        let [xMagnified, yMagnified] = this._uiGroupClone.get_position();
+        let roiWidth = this._viewPortWidth / this._xMagFactor;
-        let [xMagFactor, yMagFactor] = this.getMagFactor();
+        let roiHeight = this._viewPortHeight / this._yMagFactor;
-        let [width, height] = this.getSize();
+
-        let x = (0 - xMagnified) / xMagFactor;
+        return [this._xCenter - roiWidth / 2,
-        let y = (0 - yMagnified) / yMagFactor;
+                this._yCenter - roiHeight / 2,
-        return [x, y, width / xMagFactor, height / yMagFactor];
+                roiWidth, roiHeight];
     },
 
     /**
      * setLensMode:
-     * Turn lens mode on/off.  In full screen mode, lens mode is alway off since
+     * Turn lens mode on/off.  In full screen mode, lens mode does nothing since
      * a lens the size of the screen is pointless.
      * @lensMode:   A boolean to set the sense of lens mode.
      */
     setLensMode: function(lensMode) {
-        let fullScreen = this.isFullScreenMode();
+        this._lensMode = lensMode;
-        this._lensMode = (lensMode && !fullScreen);
+        if (!this._lensMode)
-        if (!this._lensMode && !fullScreen)
             this.setScreenPosition (this._screenPosition);
     },
 
@@ -857,6 +734,8 @@ ZoomRegion.prototype = {
      */
     setClampScrollingAtEdges: function(clamp) {
         this._clampScrollingAtEdges = clamp;
+        if (clamp)
+            this._changeROI();
     },
 
     /**
@@ -869,7 +748,7 @@ ZoomRegion.prototype = {
         viewPort.y = 0;
         viewPort.width = global.screen_width;
         viewPort.height = global.screen_height/2;
-        this.setViewPort(viewPort);
+        this._setViewPort(viewPort);
         this._screenPosition = ScreenPosition.TOP_HALF;
     },
 
@@ -883,7 +762,7 @@ ZoomRegion.prototype = {
         viewPort.y = global.screen_height/2;
         viewPort.width = global.screen_width;
         viewPort.height = global.screen_height/2;
-        this.setViewPort(viewPort);
+        this._setViewPort(viewPort);
         this._screenPosition = ScreenPosition.BOTTOM_HALF;
     },
 
@@ -897,7 +776,7 @@ ZoomRegion.prototype = {
         viewPort.y = 0;
         viewPort.width = global.screen_width/2;
         viewPort.height = global.screen_height;
-        this.setViewPort(viewPort);
+        this._setViewPort(viewPort);
         this._screenPosition = ScreenPosition.LEFT_HALF;
     },
 
@@ -911,62 +790,24 @@ ZoomRegion.prototype = {
         viewPort.y = 0;
         viewPort.width = global.screen_width/2;
         viewPort.height = global.screen_height;
-        this.setViewPort(viewPort);
+        this._setViewPort(viewPort);
         this._screenPosition = ScreenPosition.RIGHT_HALF;
     },
 
-    /**
-     * getScreenPosition:
-     * Tell the outside world what the current mode is -- magnifiying the
-     * top half, bottom half, etc.
-     * @return:  the current mode.
-     */
-    getScreenPosition: function() {
-        return this._screenPosition;
-    },
-
-    /**
-     * scrollToMousePos:
-     * Set the region of interest based on the position of the system pointer.
-     * @return:     Whether the system mouse pointer is over the magnified view.
-     */
-    scrollToMousePos: function() {
-        let [xMouse, yMouse, mask] = global.get_pointer();
-
-        if (this._mouseTrackingMode == MouseTrackingMode.PROPORTIONAL) {
-            this._setROIProportional(xMouse, yMouse);
-        }
-        else if (this._mouseTrackingMode == MouseTrackingMode.PUSH) {
-            this._setROIPush(xMouse, yMouse);
-        }
-        else if (this._mouseTrackingMode == MouseTrackingMode.CENTERED) {
-            this._setROICentered(xMouse, yMouse);
-        }
-        this._updateMousePosition(true);
-
-        // Determine whether the system mouse pointer is over this zoom region.
-        return this.isMouseOverRegion(xMouse, yMouse);
-    },
-
     /**
      * setFullScreenMode:
      * Set the ZoomRegion to full-screen mode.
      * Note:  disallows lens mode.
      */
     setFullScreenMode: function() {
-        if (!this.isFullScreenMode()) {
+        let viewPort = {};
-            let viewPort = {};
+        viewPort.x = 0;
-            viewPort.x = 0;
+        viewPort.y = 0;
-            viewPort.y = 0;
+        viewPort.width = global.screen_width;
-            viewPort.width = global.screen_width;
+        viewPort.height = global.screen_height;
-            viewPort.height = global.screen_height;
+        this.setViewPort(viewPort);
-            this.setViewPort(viewPort);
-            this.setLensMode(false);
-            if (this.isActive())
-                this._magnifier.hideSystemCursor();
 
-            this._screenPosition = ScreenPosition.FULL_SCREEN;
+        this._screenPosition = ScreenPosition.FULL_SCREEN;
-        }
     },
 
     /**
@@ -997,111 +838,239 @@ ZoomRegion.prototype = {
         }
     },
 
+    /**
+     * getScreenPosition:
+     * Tell the outside world what the current mode is -- magnifiying the
+     * top half, bottom half, etc.
+     * @return:  the current mode.
+     */
+    getScreenPosition: function() {
+        return this._screenPosition;
+    },
+
+    /**
+     * scrollToMousePos:
+     * Set the region of interest based on the position of the system pointer.
+     * @return:     Whether the system mouse pointer is over the magnified view.
+     */
+    scrollToMousePos: function() {
+        this._followingCursor = true;
+        if (this._mouseTrackingMode != MouseTrackingMode.NONE)
+            this._changeROI({ redoCursorTracking: true });
+        else
+            this._updateMousePosition();
+
+        // Determine whether the system mouse pointer is over this zoom region.
+        return this._isMouseOverRegion();
+    },
+
     /**
      * scrollContentsTo:
      * Shift the contents of the magnified view such it is centered on the given
-     * coordinate.  Also, update the position of the magnified mouse image after
+     * coordinate.
-     * the shift.
      * @x:      The x-coord of the point to center on.
      * @y:      The y-coord of the point to center on.
      */
     scrollContentsTo: function(x, y) {
-        this._scrollToPosition(x, y);
+        this._followingCursor = false;
-        this._updateMousePosition(false /* mouse didn't move */);
+        this._changeROI({ xCenter: x,
-    },
+                          yCenter: y });
-
-    /**
-     * isMouseOverRegion:
-     * Return whether the system mouse sprite is over this ZoomRegion.  If the
-     * mouse's position is not given, then it is fetched.
-     * @xMouse:     The system mouse's x-coord.  Optional.
-     * @yMouse:     The system mouse's y-coord.  Optional.
-     * @return:     Boolean:  true if the mouse is over the zoom region; false
-     *              otherwise.
-     */
-    isMouseOverRegion: function(xMouse, yMouse) {
-        let mouseIsOver = false;
-        if (this.isActive()) {
-            if (!xMouse || !yMouse) {
-                let [x, y, mask] = global.get_pointer();
-                xMouse = x;
-                yMouse = y;
-            }
-            let [x, y] = this.getPosition();
-            let [width, height] = this.getSize();
-            mouseIsOver = (
-                xMouse >= x && xMouse < (x + width) &&
-                yMouse >= y && yMouse < (y + height)
-            );
-        }
-        return mouseIsOver;
     },
 
     /**
      * addCrosshairs:
      * Add crosshairs centered on the magnified mouse.
-     * @crossHairs  Clutter.Group that contains the actors for the crosshairs.
+     * @crossHairs: Crosshairs instance
      */
     addCrosshairs: function(crossHairs) {
+        this._crossHairs = crossHairs;
+
         // If the crossHairs is not already within a larger container, add it
         // to this zoom region.  Otherwise, add a clone.
-        if (crossHairs) {
+        if (crossHairs && this.isActive()) {
-            this._crosshairsActor = crossHairs.addToZoomRegion(this, this._mouseRoot);
+            this._crossHairsActor = crossHairs.addToZoomRegion(this, this._mouseActor);
-            this._crossHairs = crossHairs;
         }
     },
 
     //// Private methods ////
 
-    _scrollToPosition: function(x, y) {
+    _createActors: function() {
-        // Given the point (x, y) in non-magnified coordinates, scroll the
+        // The root actor for the zoom region
-        // magnified contenst such that the point is at the centre of the
+        this._magView = new St.Bin({ style_class: 'magnifier-zoom-region', x_fill: true, y_fill: true });
-        // magnified view.
+        global.stage.add_actor(this._magView);
-        let [xMagFactor, yMagFactor] = this.getMagFactor();
-        let xMagnified = x * xMagFactor;
-        let yMagnified = y * yMagFactor;
 
-        let [xCenterMagView, yCenterMagView] = this.getCenter();
+        // Append a Clutter.Group to clip the contents of the magnified view.
-        let newX = xCenterMagView - xMagnified;
+        let mainGroup = new Clutter.Group({ clip_to_allocation: true });
-        let newY = yCenterMagView - yMagnified;
+        this._magView.set_child(mainGroup);
+
+        // Add a background for when the magnified uiGroup is scrolled
+        // out of view (don't want to see desktop showing through).
+        let background = new Clutter.Rectangle({ color: Main.DEFAULT_BACKGROUND_COLOR });
+        mainGroup.add_actor(background);
+
+        // Clone the group that contains all of UI on the screen.  This is the
+        // chrome, the windows, etc.
+        this._uiGroupClone = new Clutter.Clone({ source: Main.uiGroup });
+        mainGroup.add_actor(this._uiGroupClone);
+        Main.uiGroup.set_size(global.screen_width, global.screen_height);
+        background.set_size(global.screen_width, global.screen_height);
+
+        // Add either the given mouseSourceActor to the ZoomRegion, or a clone of
+        // it.
+        if (this._mouseSourceActor.get_parent() != null)
+            this._mouseActor = new Clutter.Clone({ source: this._mouseSourceActor });
+        else
+            this._mouseActor = this._mouseSourceActor;
+        mainGroup.add_actor(this._mouseActor);
+
+        if (this._crossHairs)
+            this._crossHairsActor = this._crossHairs.addToZoomRegion(this, this._mouseActor);
+        else
+            this._crossHairsActor = null;
+    },
+
+    _destroyActors: function() {
+        if (this._mouseActor == this._mouseSourceActor)
+            this._mouseActor.get_parent().remove_actor (this._mouseActor);
+        if (this._crossHairs)
+            this._crossHairs.removeFromParent(this._crossHairsActor);
+
+        this._magView.destroy();
+        this._magView = null;
+        this._uiGroupClone = null;
+        this._mouseActor = null;
+        this._crossHairsActor = null;
+    },
+
+    _setViewPort: function(viewPort, fromROIUpdate) {
+        // Sets the position of the zoom region on the screen
+
+        let width = Math.round(Math.min(viewPort.width, global.screen_width));
+        let height = Math.round(Math.min(viewPort.height, global.screen_height));
+        let x = Math.max(viewPort.x, 0);
+        let y = Math.max(viewPort.y, 0);
+
+        x = Math.round(Math.min(x, global.screen_width - width));
+        y = Math.round(Math.min(y, global.screen_height - height));
+
+        this._viewPortX = x;
+        this._viewPortY = y;
+        this._viewPortWidth = width;
+        this._viewPortHeight = height;
+
+        this._updateMagViewGeometry();
+
+        if (!fromROIUpdate)
+            this._changeROI({ redoCursorTracking: this._followingCursor }); // will update mouse
+
+        if (this.isActive() && this._isMouseOverRegion())
+            this._magnifier.hideSystemCursor();
+    },
+
+    _changeROI: function(params) {
+        // Updates the area we are viewing; the magnification factors
+        // and center can be set explicitly, or we can recompute
+        // the position based on the mouse cursor position
+
+        params = Params.parse(params, { xMagFactor: this._xMagFactor,
+                                        yMagFactor: this._yMagFactor,
+                                        xCenter: this._xCenter,
+                                        yCenter: this._yCenter,
+                                        redoCursorTracking: false });
+
+        if (params.xMagFactor <= 0)
+            params.xMagFactor = this._xMagFactor;
+        if (params.yMagFactor <= 0)
+            params.yMagFactor = this._yMagFactor;
+
+        this._xMagFactor = params.xMagFactor;
+        this._yMagFactor = params.yMagFactor;
+
+        if (params.redoCursorTracking &&
+            this._mouseTrackingMode != MouseTrackingMode.NONE) {
+            // This depends on this.xMagFactor/yMagFactor already being updated
+            [params.xCenter, params.yCenter] = this._centerFromMousePosition();
+        }
 
         if (this._clampScrollingAtEdges) {
-            if (newX > 0)
+            let roiWidth = this._viewPortWidth / this._xMagFactor;
-                newX = 0;
+            let roiHeight = this._viewPortHeight / this._yMagFactor;
-            else if (newX < this._rightStop)
+
-                newX = this._rightStop;
+            params.xCenter = Math.min(params.xCenter, global.screen_width - roiWidth / 2);
-            if (newY > 0)
+            params.xCenter = Math.max(params.xCenter, roiWidth / 2);
-                newY = 0;
+            params.yCenter = Math.min(params.yCenter, global.screen_height - roiHeight / 2);
-            else if (newY < this._bottomStop)
+            params.yCenter = Math.max(params.yCenter, roiHeight / 2);
-                newY = this._bottomStop;
-            this._uiGroupClone.set_position(newX, newY);
         }
-        else
+
-            this._uiGroupClone.set_position(newX, newY);
+        this._xCenter = params.xCenter;
+        this._yCenter = params.yCenter;
 
         // If in lens mode, move the magnified view such that it is centered
         // over the actual mouse. However, in full screen mode, the "lens" is
         // the size of the screen -- pointless to move such a large lens around.
-        if (this._lensMode && !this.isFullScreenMode())
+        if (this._lensMode && !this._isFullScreen())
-            this.setPosition(x - xCenterMagView, y - yCenterMagView);
+            this._setViewPort({ x: this._xCenter - this._viewPortWidth / 2,
+                                y: this._yCenter - this._viewPortHeight / 2,
+                                width: this._viewPortWidth,
+                                height: this._viewPortHeight }, true);
+
+        this._updateCloneGeometry();
+        this._updateMousePosition();
     },
 
-    _calcRightBottomStops: function() {
+    _isMouseOverRegion: function(xMouse, yMouse) {
-        // Calculate the location of the top-left corner of _uiGroupClone
+        // Return whether the system mouse sprite is over this ZoomRegion.  If the
-        // when its right and bottom edges are coincident with the right and
+        // mouse's position is not given, then it is fetched.
-        // bottom edges of the _magView.
+        let mouseIsOver = false;
-        let [contentWidth, contentHeight] = this._uiGroupClone.get_size();
+        if (this.isActive()) {
-        let [viewWidth, viewHeight] = this.getSize();
+            if (xMouse == null || yMouse == null) {
-        let [xMagFactor, yMagFactor] = this.getMagFactor();
+                let [x, y, mask] = global.get_pointer();
-        let rightStop = viewWidth - (contentWidth * xMagFactor);
+                xMouse = x;
-        let bottomStop = viewHeight - (contentHeight * yMagFactor);
+                yMouse = y;
-        this._rightStop = parseInt(rightStop.toFixed(1));
+            }
-        this._bottomStop = parseInt(bottomStop.toFixed(1));
+            mouseIsOver = (
+                xMouse >= this._viewPortX && xMouse < (this._viewPortX + this._viewPortWidth) &&
+                yMouse >= this._viewPortY && yMouse < (this._viewPortY + this._viewPortHeight)
+            );
+        }
+        return mouseIsOver;
     },
 
-    _setROIPush: function(xMouse, yMouse) {
+    _isFullScreen: function() {
+        // Does the magnified view occupy the whole screen? Note that this
+        // doesn't necessarily imply
+        // this._screenPosition = ScreenPosition.FULL_SCREEN;
+
+        if (this._viewPortX != 0 || this._viewPortY != 0)
+            return false;
+        if (this._viewPortWidth != global.screen_width ||
+            this._viewPortHeight != global.screen_height)
+            return false;
+        return true;
+    },
+
+    _centerFromMousePosition: function() {
+        // Determines where the center should be given the current cursor
+        // position and mouse tracking mode
+
+        let [xMouse, yMouse, mask] = global.get_pointer();
+
+        if (this._mouseTrackingMode == MouseTrackingMode.PROPORTIONAL) {
+            return this._centerFromMouseProportional(xMouse, yMouse);
+        }
+        else if (this._mouseTrackingMode == MouseTrackingMode.PUSH) {
+            return this._centerFromMousePush(xMouse, yMouse);
+        }
+        else if (this._mouseTrackingMode == MouseTrackingMode.CENTERED) {
+            return this._centerFromMouseCentered(xMouse, yMouse);
+        }
+
+        return null; // Should never be hit
+    },
+
+    _centerFromMousePush: function(xMouse, yMouse) {
         let [xRoi, yRoi, widthRoi, heightRoi] = this.getROI();
-        let [cursorWidth, cursorHeight] = this._mouseRoot.get_size();
+        let [cursorWidth, cursorHeight] = this._mouseSourceActor.get_size();
         let xPos = xRoi + widthRoi / 2;
         let yPos = yRoi + heightRoi / 2;
         let xRoiRight = xRoi + widthRoi - cursorWidth;
@@ -1117,10 +1086,10 @@ ZoomRegion.prototype = {
         else if (yMouse > yRoiBottom)
             yPos += (yMouse - yRoiBottom);
 
-        this._scrollToPosition(xPos, yPos);
+        return [xPos, yPos];
     },
 
-    _setROIProportional: function(xMouse, yMouse) {
+    _centerFromMouseProportional: function(xMouse, yMouse) {
         let [xRoi, yRoi, widthRoi, heightRoi] = this.getROI();
         let halfScreenWidth = global.screen_width / 2;
         let halfScreenHeight = global.screen_height / 2;
@@ -1129,46 +1098,62 @@ ZoomRegion.prototype = {
         let xPos = xMouse + xProportion * widthRoi / 2;
         let yPos = yMouse + yProportion * heightRoi / 2;
 
-        this._scrollToPosition(xPos, yPos);
+        return [xPos, yPos];
     },
 
-    _setROICentered: function(xMouse, yMouse) {
+    _centerFromMouseCentered: function(xMouse, yMouse) {
-        this._scrollToPosition(xMouse, yMouse);
+        return [xMouse, yMouse];
     },
 
-    _updateMousePosition: function(mouseMoved) {
+    _screenToViewPort: function(screenX, screenY) {
-        let [x, y] = this._uiGroupClone.get_position();
+        // Converts coordinates relative to the (unmagnified) screen to coordinates
-        x = parseInt(x.toFixed(1));
+        // relative to the origin of this._magView
-        y = parseInt(y.toFixed(1));
+        return [this._viewPortWidth / 2 + (screenX - this._xCenter) * this._xMagFactor,
-        let [xCenterMagView, yCenterMagView] = this.getCenter();
+                this._viewPortHeight / 2 + (screenY - this._yCenter) * this._yMagFactor];
+    },
+
+    _updateMagViewGeometry: function() {
+        if (!this.isActive())
+            return;
+
+        if (this._isFullScreen())
+            this._magView.add_style_class_name('full-screen');
+        else
+            this._magView.remove_style_class_name('full-screen');
+
+        this._magView.set_size(this._viewPortWidth, this._viewPortHeight);
+        this._magView.set_position(this._viewPortX, this._viewPortY);
+    },
+
+    _updateCloneGeometry: function() {
+        if (!this.isActive())
+            return;
+
+        this._uiGroupClone.set_scale(this._xMagFactor, this._yMagFactor);
+        this._mouseActor.set_scale(this._xMagFactor, this._yMagFactor);
+
+        let [x, y] = this._screenToViewPort(0, 0);
+        this._uiGroupClone.set_position(x, y);
+
+        this._updateMousePosition();
+    },
+
+    _updateMousePosition: function() {
+        if (!this.isActive())
+            return;
+
         let [xMouse, yMouse, mask] = global.get_pointer();
-        let [xMagFactor, yMagFactor] = this.getMagFactor();
+        let [xMagMouse, yMagMouse] = this._screenToViewPort(xMouse, yMouse);
 
-        let xMagMouse = xMouse * xMagFactor + x;
+        xMagMouse = Math.round(xMagMouse);
-        let yMagMouse = yMouse * yMagFactor + y;
+        yMagMouse = Math.round(yMagMouse);
-        if (mouseMoved) {
-            if (x == 0)
-                xMagMouse = xMouse * xMagFactor;
-            else if (x == this._rightStop)
-                xMagMouse = (xMouse * xMagFactor) + this._rightStop;
-            else if (this._mouseTrackingMode == MouseTrackingMode.CENTERED)
-                xMagMouse = xCenterMagView;
 
-            if (y == 0)
+        this._mouseActor.set_position(xMagMouse, yMagMouse);
-                yMagMouse = yMouse * yMagFactor;
-            else if (y == this._bottomStop)
-                yMagMouse = (yMouse * yMagFactor) + this._bottomStop;
-            else if (this._mouseTrackingMode == MouseTrackingMode.CENTERED)
-                yMagMouse = yCenterMagView;
-        }
-        this._mouseRoot.set_position(xMagMouse, yMagMouse);
-        this._updateCrosshairsPosition(xMagMouse, yMagMouse);
-    },
 
-    _updateCrosshairsPosition: function(x, y) {
+        if (this._crossHairsActor) {
-        if (this._crosshairsActor) {
+            let [groupWidth, groupHeight] = this._crossHairsActor.get_size();
-            let [groupWidth, groupHeight] = this._crosshairsActor.get_size();
+            this._crossHairsActor.set_position(xMagMouse - groupWidth / 2,
-            this._crosshairsActor.set_position(x - groupWidth / 2, y - groupHeight / 2);
+                                               yMagMouse - groupHeight / 2);
         }
     }
 };
@@ -1243,10 +1228,15 @@ Crosshairs.prototype = {
 
     /**
      * removeFromParent:
-     * Remove the crosshairs actor from its parent container.
+     * @childActor: the actor returned from addToZoomRegion
+     * Remove the crosshairs actor from its parent container, or destroy the
+     * child actor if it was just a clone of the crosshairs actor.
      */
-    removeFromParent: function() {
+    removeFromParent: function(childActor) {
-        this._actor.get_parent().remove_actor(this._actor);
+        if (childActor == this._actor)
+            childActor.get_parent().remove_actor(childActor);
+        else
+            childActor.destroy();
     },
 
     /**

js/ui/magnifierDBus.js

@@ -112,6 +112,11 @@ ShellMagnifier.prototype = {
      *                  [left, top, right, bottom].
      * @viewPort        Array of integers, [left, top, right, bottom] that defines
      *                  the position of the ZoomRegion on screen.
+     *
+     * FIXME: The arguments here are redundant, since the width and height of
+     *   the ROI are determined by the viewport and magnification factors.
+     *   We ignore the passed in width and height.
+     *
      * @return          The newly created ZoomRegion.
      */
     createZoomRegion: function(xMagFactor, yMagFactor, roi, viewPort) {