From 55abf00c8674dcc9a5507e0b8033455f293fa8d1 Mon Sep 17 00:00:00 2001
From: Paul McCarthy <pauld.mccarthy@gmail.com>
Date: Mon, 27 Jul 2015 15:11:00 +0100
Subject: [PATCH] DisplayOpts instances now have a 'bounds' property.
 DisplayContext (and other things) listen on this property to detect overlay
 bounds/spatial representation changes, instead of using a global listener on
 DisplayOpts instances. ModelOpts is correspondingly broken, but about to be
 fixed.

---
 fsl/fslview/controls/atlasinfopanel.py       |  9 ++---
 fsl/fslview/controls/locationpanel.py        | 26 +++++---------
 fsl/fslview/displaycontext/display.py        | 37 ++++++++++++++------
 fsl/fslview/displaycontext/displaycontext.py | 31 +++++++++-------
 fsl/fslview/displaycontext/volumeopts.py     | 35 ++++++++----------
 fsl/fslview/gl/globject.py                   |  5 +--
 fsl/fslview/gl/slicecanvas.py                |  8 +++--
 fsl/fslview/views/lightboxpanel.py           |  5 +--
 fsl/fslview/views/orthopanel.py              | 15 ++++----
 9 files changed, 91 insertions(+), 80 deletions(-)

diff --git a/fsl/fslview/controls/atlasinfopanel.py b/fsl/fslview/controls/atlasinfopanel.py
index 8cb248237..eb1e26873 100644
--- a/fsl/fslview/controls/atlasinfopanel.py
+++ b/fsl/fslview/controls/atlasinfopanel.py
@@ -162,11 +162,12 @@ class AtlasInfoPanel(fslpanel.FSLViewPanel):
             opts = self._displayCtx.getOpts(ovl)
 
             if ovl == selOverlay:
-                opts.addGlobalListener(   self._name,
-                                          self.__locationChanged,
-                                          overwrite=True)
+                opts.addListener('bounds',
+                                 self._name,
+                                 self.__locationChanged,
+                                 overwrite=True)
             else:
-                opts.removeGlobalListener(self._name)
+                opts.removeListener('bounds', self._name)
 
         self.__locationChanged()
 
diff --git a/fsl/fslview/controls/locationpanel.py b/fsl/fslview/controls/locationpanel.py
index 2071813d7..c30e51533 100644
--- a/fsl/fslview/controls/locationpanel.py
+++ b/fsl/fslview/controls/locationpanel.py
@@ -254,26 +254,22 @@ class LocationPanel(fslpanel.FSLViewPanel):
         for ovl in self._overlayList:
             display = self._displayCtx.getDisplay(ovl)
             opts    = display.getDisplayOpts()
+            
             if ovl is overlay:
-                opts   .addGlobalListener(self._name,
-                                          self._overlayOptsChanged,
-                                          overwrite=True)
-                display.addGlobalListener(self._name,
-                                          self._overlayOptsChanged,
-                                          overwrite=True) 
+                opts.addListener('bounds',
+                                 self._name,
+                                 self._overlayBoundsChanged,
+                                 overwrite=True)
             else:
-                opts   .removeGlobalListener(self._name)
-                display.removeGlobalListener(self._name)
+                opts.removeListener('bounds', self._name)
 
         # Refresh the world/voxel location properties
         self._displayLocationChanged()
 
 
-    def _overlayOptsChanged(self, *a):
+    def _overlayBoundsChanged(self, *a):
 
-        if not self._updateReferenceImage():
-            return
-        
+        self._updateReferenceImage()
         self._updateWidgets()
         self._displayLocationChanged()
         
@@ -299,10 +295,6 @@ class LocationPanel(fslpanel.FSLViewPanel):
             overlay  = self._displayCtx.getSelectedOverlay()
             refImage = self._displayCtx.getReferenceImage(overlay)
 
-            # Reference image has not changed
-            if refImage == self._refImage:
-                return False
-
             log.debug('Reference image for overlay {}: {}'.format(
                 overlay, refImage))
 
@@ -324,8 +316,6 @@ class LocationPanel(fslpanel.FSLViewPanel):
             self._voxToWorldMat     = None
             self._worldToVoxMat     = None
 
-        return True
-
 
     def _updateWidgets(self):
 
diff --git a/fsl/fslview/displaycontext/display.py b/fsl/fslview/displaycontext/display.py
index fae54eeae..36b1a6c93 100644
--- a/fsl/fslview/displaycontext/display.py
+++ b/fsl/fslview/displaycontext/display.py
@@ -28,6 +28,18 @@ log = logging.getLogger(__name__)
 
 class DisplayOpts(props.SyncableHasProperties):
 
+
+    bounds = props.Bounds(ndims=3)
+    """Specifies a bounding box (in display coordinates) which is big enough
+    to contain the overlay described by this ``DisplayOpts`` instance. The
+    values in this ``bounds`` property  must be maintained by subclass
+    implementations.
+
+    Whenever the spatial representation of this overlay changes, but the
+    bounds do not change, subclass implementations should force notification
+    on this property (via the :meth:`.HasProperties.notify` method). 
+    """
+
     def __init__(
             self,
             overlay,
@@ -81,19 +93,22 @@ class DisplayOpts(props.SyncableHasProperties):
         if isinstance(self.overlay, fslimage.Image):
             return self.overlay
         return None
-    
-    
-    def getDisplayBounds(self):
-        """
-        """
-        raise NotImplementedError(
-            'The getDisplayBounds method must be implemented by subclasses')
-
 
-    def transformDisplayLocation(self, propName, oldLoc):
-        """
+    
+    def transformDisplayLocation(self, coords):
+        """This method may be called after the overlay :attr:`bounds` have
+        changed.
+
+        If the bounds were changed as a result of a change to the spatial
+        representation of the overlay (e.g. the :attr:`.ImageOpts.transform`
+        property for :class:`.Image` overlays), this method should return
+        a copy of the given coordinates which has been transformed into the
+        new space.
+
+        If the bounds were changed for some other reason, this method can
+        just return the ``coords`` unchanged.
         """
-        return oldLoc
+        return coords
 
 
 class Display(props.SyncableHasProperties):
diff --git a/fsl/fslview/displaycontext/displaycontext.py b/fsl/fslview/displaycontext/displaycontext.py
index 931b25acf..90988fdfb 100644
--- a/fsl/fslview/displaycontext/displaycontext.py
+++ b/fsl/fslview/displaycontext/displaycontext.py
@@ -274,7 +274,7 @@ class DisplayContext(props.SyncableHasProperties):
                 opts    = display.getDisplayOpts()
 
                 display.removeListener('overlayType', self.__name)
-                opts.removeGlobalListener(self.__name)
+                opts   .removeListener('bounds',      self.__name)
                 
                 # The display instance will destroy the
                 # opts instance, so we don't do it here
@@ -305,9 +305,10 @@ class DisplayContext(props.SyncableHasProperties):
             # DisplayOpts properties change, the
             # overlay display bounds may have changed,
             # so we need to know when this happens.
-            opts.addGlobalListener(self.__name,
-                                   self.__displayOptsChanged,
-                                   overwrite=True)
+            opts.addListener('bounds',
+                             self.__name,
+                             self.__overlayBoundsChanged,
+                             overwrite=True)
 
         # Ensure that the overlayOrder
         # property is valid ...
@@ -390,10 +391,11 @@ class DisplayContext(props.SyncableHasProperties):
             self.setConstraint('selectedOverlay', 'maxval', 0)
 
             
-    def __displayOptsChanged(self, value, valid, opts, name):
-        """Called when the :class:`.DisplayOpts` properties of any overlay
-        change. If the bounds o the Updates the :attr:`bounds` property and,
-        if the currently selected 
+    def __overlayBoundsChanged(self, value, valid, opts, name):
+        """Called when the :attr:`.DisplayOpts.bounds` property of any
+        overlay changes. Updates the :attr:`bounds` property and preserves
+        the display :attr:`location` in terms of hte currently selected
+        overlay.
         """
 
         # This check is ugly, and is required due to
@@ -431,8 +433,7 @@ class DisplayContext(props.SyncableHasProperties):
         # same value, we assume that they don't need to be
         # updated again, and escape from ths system.
         if self.getParent() is not None and self.isSyncedToParent('location'):
-            if self.getParent().location == self.location:
-                return
+            return
 
         overlay = opts.display.getOverlay()
 
@@ -445,7 +446,9 @@ class DisplayContext(props.SyncableHasProperties):
         # Update the display context bounds
         # to take into account any changes 
         # to individual overlay bounds
+        self.disableNotification('location')
         self.__updateBounds()
+        self.enableNotification('location')
  
         # The main purpose of this method is to preserve
         # the current display location in terms of the
@@ -453,12 +456,13 @@ class DisplayContext(props.SyncableHasProperties):
         # bounds have changed. We don't care about changes
         # to the options for other overlays.
         if (overlay != self.getSelectedOverlay()):
+            self.notify('location')
             return
 
         # Now we want to update the display location
         # so that it is preserved with respect to the
         # currently selected overlay.
-        newDispLoc = opts.transformDisplayLocation(name, oldDispLoc)
+        newDispLoc = opts.transformDisplayLocation(oldDispLoc)
 
         # Ignore the new display location
         # if it is not in the display bounds
@@ -472,6 +476,8 @@ class DisplayContext(props.SyncableHasProperties):
                           newDispLoc))
 
             self.location.xyz = newDispLoc
+        else:
+            self.notify('location')
 
 
     def __syncOverlayDisplayChanged(self, *a):
@@ -515,7 +521,8 @@ class DisplayContext(props.SyncableHasProperties):
 
             display = self.__displays[ovl]
             opts    = display.getDisplayOpts()
-            lo, hi  = opts   .getDisplayBounds()
+            lo      = opts.bounds.getLo()
+            hi      = opts.bounds.getHi()
 
             for ax in range(3):
 
diff --git a/fsl/fslview/displaycontext/volumeopts.py b/fsl/fslview/displaycontext/volumeopts.py
index 928a92b96..bed9dcf77 100644
--- a/fsl/fslview/displaycontext/volumeopts.py
+++ b/fsl/fslview/displaycontext/volumeopts.py
@@ -73,11 +73,14 @@ class ImageOpts(fsldisplay.DisplayOpts):
 
         overlay = self.overlay
 
+        self.addListener('transform', self.name, self.__transformChanged)
+
         # The display<->* transformation matrices
         # are created in the _setupTransforms method
         self.__xforms = {}
         self.__setupTransforms()
-
+        self.__transformChanged()
+ 
         # is this a 4D volume?
         if self.overlay.is4DImage():
             self.setConstraint('volume', 'maxval', overlay.shape[3] - 1)
@@ -90,22 +93,17 @@ class ImageOpts(fsldisplay.DisplayOpts):
     def destroy(self):
         fsldisplay.DisplayOpts.destroy(self)
 
-
-    def getDisplayBounds(self):
-        """Calculates and returns the min/max values of a 3D bounding box,
-        in the display coordinate system, which is big enough to contain
-        the image.
-
-        The coordinate system in which the bounding box is defined is
-        determined by the current value of the :attr:`transform` property.
-
-        A tuple containing two values is returned, with the first value
-        a sequence of three low bounds, and the second value a sequence
-        of three high bounds.
+        
+    def __transformChanged(self, *a):
+        """Calculates the min/max values of a 3D bounding box, in the display
+        coordinate system, which is big enough to contain the image. Sets the
+        :attr:`.DisplayOpts.bounds` property accordingly.
         """
 
-        return transform.axisBounds(
+        lo, hi = transform.axisBounds(
             self.overlay.shape[:3], self.getTransform('voxel', 'display'))
+        
+        self.bounds[:] = [lo[0], hi[0], lo[1], hi[1], lo[2], hi[2]]
 
                             
     def __setupTransforms(self):
@@ -183,15 +181,10 @@ class ImageOpts(fsldisplay.DisplayOpts):
         return self.__xforms[from_, to]
 
 
-    def transformDisplayLocation(self, propName, oldLoc):
-
-        if propName != 'transform':
-            return oldLoc
+    def transformDisplayLocation(self, oldLoc):
 
         lastVal = self.getLastValue('transform')
-        if lastVal is None:
-            lastVal = self.transform
-
+        
         # Calculate the image world location using the
         # old display<-> world transform, then transform
         # it back to the new world->display transform. 
diff --git a/fsl/fslview/gl/globject.py b/fsl/fslview/gl/globject.py
index 6a143bd43..4ca712145 100644
--- a/fsl/fslview/gl/globject.py
+++ b/fsl/fslview/gl/globject.py
@@ -247,7 +247,8 @@ class GLImageObject(GLObject):
 
 
     def getDisplayBounds(self):
-        return self.displayOpts.getDisplayBounds()
+        return (self.displayOpts.bounds.getLo(),
+                self.displayOpts.bounds.getHi())
 
 
     def getDataResolution(self, xax, yax):
@@ -265,7 +266,7 @@ class GLImageObject(GLObject):
             return np.array(res.round(), dtype=np.uint32)
         
         else:
-            lo, hi = opts.getDisplayBounds()
+            lo, hi = self.getDisplayBounds()
             minres = int(round(((hi - lo) / res).min()))
             return [minres] * 3
 
diff --git a/fsl/fslview/gl/slicecanvas.py b/fsl/fslview/gl/slicecanvas.py
index 6e43d1205..fff6ed6b1 100644
--- a/fsl/fslview/gl/slicecanvas.py
+++ b/fsl/fslview/gl/slicecanvas.py
@@ -926,7 +926,8 @@ class SliceCanvas(props.HasProperties):
             rt      = self._offscreenTextures.get(overlay, None)
             display = self.displayCtx.getDisplay(overlay)
             opts    = display.getDisplayOpts()
-            lo, hi  = opts.getDisplayBounds()
+            lo      = opts.bounds.getLo()
+            hi      = opts.bounds.getHi()
 
             if rt is None or not display.enabled:
                 continue
@@ -987,8 +988,9 @@ class SliceCanvas(props.HasProperties):
             # target, and draw to it
             elif self.renderMode == 'offscreen':
                 
-                rt     = self._offscreenTextures.get(overlay, None)
-                lo, hi = opts.getDisplayBounds()
+                rt = self._offscreenTextures.get(overlay, None)
+                lo = opts.bounds.getLo()
+                hi = opts.bounds.getHi()
 
                 # Assume that all is well - the texture
                 # just has not yet been created
diff --git a/fsl/fslview/views/lightboxpanel.py b/fsl/fslview/views/lightboxpanel.py
index 616e3ee7a..e694f59c5 100644
--- a/fsl/fslview/views/lightboxpanel.py
+++ b/fsl/fslview/views/lightboxpanel.py
@@ -191,8 +191,9 @@ class LightBoxPanel(canvaspanel.CanvasPanel):
         if overlay is None:
             return
         
-        opts               = self._displayCtx.getOpts(overlay)
-        loBounds, hiBounds = opts.getDisplayBounds()
+        opts     = self._displayCtx.getOpts(overlay)
+        loBounds = opts.bounds.getLo()
+        hiBounds = opts.bounds.getHi()
 
         if opts.transform == 'id':
             sceneOpts.sliceSpacing = 1
diff --git a/fsl/fslview/views/orthopanel.py b/fsl/fslview/views/orthopanel.py
index 567f29e84..6e79baa3e 100644
--- a/fsl/fslview/views/orthopanel.py
+++ b/fsl/fslview/views/orthopanel.py
@@ -200,7 +200,7 @@ class OrthoPanel(canvaspanel.CanvasPanel):
         # so we have to remove them too
         for ovl in self._overlayList:
             opts = self._displayCtx.getOpts(ovl)
-            opts.removeGlobalListener(self._name)
+            opts.removeListener('bounds', self._name)
 
         canvaspanel.CanvasPanel.destroy(self)
 
@@ -286,14 +286,15 @@ class OrthoPanel(canvaspanel.CanvasPanel):
 
             opts = self._displayCtx.getOpts(ovl)
 
-            # Update anatomy labels when any 
-            # overlay display properties change
+            # Update anatomy labels when 
+            # overlay bounds change
             if i == self._displayCtx.selectedOverlay:
-                opts.addGlobalListener(self._name,
-                                       self._refreshLabels,
-                                       overwrite=True)
+                opts.addListener('bounds',
+                                 self._name,
+                                 self._refreshLabels,
+                                 overwrite=True)
             else:
-                opts.removeGlobalListener(self._name)
+                opts.removeListener('bounds', self._name)
                 
         # anatomical orientation may have changed with an image change
         self._refreshLabels()
-- 
GitLab