diff --git a/fsl/fsleyes/displaycontext/display.py b/fsl/fsleyes/displaycontext/display.py
index 43cdbe3009c3d8e0cc216cc4f2a98f7aa674eef4..4041d2473506ba1559532b4ef17464a423ff7e28 100644
--- a/fsl/fsleyes/displaycontext/display.py
+++ b/fsl/fsleyes/displaycontext/display.py
@@ -366,12 +366,14 @@ class DisplayOpts(actions.ActionProvider):
bounds = props.Bounds(ndims=3)
"""Specifies a bounding box in the display coordinate system which is big
enough to contain the overlay described by this ``DisplayOpts``
- instance. The values in this ``bounds`` property must be updated by
- ``DisplayOpts`` subclasses.
-
- 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).
+ instance.
+
+ The values in this ``bounds`` property must be updated by ``DisplayOpts``
+ subclasses whenever the spatial representation of their overlay changes.
+ Additionally, whenever the spatial representation changes, sub-classes
+ must call the :meth:`.DisplayContext.cacheStandardCoordinates` method,
+ with a 'standard' space version of the current
+ :attr:`.DisplayContext.location`.
"""
@@ -400,6 +402,10 @@ class DisplayOpts(actions.ActionProvider):
:class:`.ActionProvider` constructor.
"""
+ nounbind = kwargs.get('nounbind', [])
+ nounbind.append('bounds')
+ kwargs['nounbind'] = nounbind
+
actions.ActionProvider.__init__(
self,
overlayList,
@@ -459,19 +465,28 @@ class DisplayOpts(actions.ActionProvider):
return self.overlay
return None
+
+ def displayToStandardCoordinates(self, coords):
+ """This method transforms the given display system coordinates into a
+ standard coordinate system which will remain constant for the given
+ overlay.
+
+ This method must be overridden by any sub-classes for which
+ the display space representation may change - for example, the
+ :class:`.Image` overlays can be transformed into the display
+ coordinate system in different ways, as defined by the
+ :attr:`.ImageOpts.transform` property.
+ """
+ return coords
+
- 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.
+ def standardToDisplayCoordinates(self, coords):
+ """This method transforms the given coordinates, assumed to be in
+ the standard coordinate system of the overlay, into the display
+ coordinate system.
+
+ This method must be overridden by sub-classes for which their standard
+ coordinate system is not the same as the display coordinate system.
"""
return coords
diff --git a/fsl/fsleyes/displaycontext/displaycontext.py b/fsl/fsleyes/displaycontext/displaycontext.py
index c547ded1fd749f7e952f8b83ae769623b38cecfc..62226cf229a45e1fae1391bb66adea95552863c8 100644
--- a/fsl/fsleyes/displaycontext/displaycontext.py
+++ b/fsl/fsleyes/displaycontext/displaycontext.py
@@ -12,6 +12,8 @@ general display settings for displaying the overlays in a
import sys
import logging
+import numpy as np
+
import props
import display as fsldisplay
@@ -163,7 +165,7 @@ class DisplayContext(props.SyncableHasProperties):
props.SyncableHasProperties.__init__(
self,
parent=parent,
- nounbind=['overlayGroups', 'displaySpace'],
+ nounbind=['overlayGroups', 'displaySpace', 'bounds'],
nobind=[ 'syncOverlayDisplay'])
self.__overlayList = overlayList
@@ -190,10 +192,30 @@ class DisplayContext(props.SyncableHasProperties):
# reset the location.
if parent is None: self.__prevOverlayListLen = 0
else: self.__prevOverlayListLen = len(overlayList)
-
+
+ # This dict contains the Display objects for
+ # every overlay in the overlay list, as
+ # {Overlay : Display} mappings
+ self.__displays = {}
+
+ # We keep a cache of 'standard' coordinates,
+ # one for each overlay - see the cacheStandardCoordinates
+ # method. We're storing this cache in a tricky
+ # way though - as an attribute of the location
+ # property. We do this so that the cache will be
+ # automatically synchronised between parent/child
+ # DC objects.
+ locPropVal = self.getPropVal('location')
+
+ # Only set the standardCoords cache if
+ # it has not already been set (if this
+ # is a child DC, the cache will have
+ # already been set on the parent)
+ try: locPropVal.getAttribute('standardCoords')
+ except KeyError: locPropVal.setAttribute('standardCoords', {})
+
# The overlayListChanged and displaySpaceChanged
# methods do important things - check them out
- self.__displays = {}
self.__overlayListChanged()
self.__displaySpaceChanged()
@@ -354,6 +376,39 @@ class DisplayContext(props.SyncableHasProperties):
return [self.__overlayList[idx] for idx in self.overlayOrder]
+ def cacheStandardCoordinates(self, overlay, coords):
+ """Stores the given 'standard' coordinates for the given overlay.
+
+ This method must be called by :class:`.DisplayOpts` sub-classes
+ whenever the spatial representation of their overlay changes. This is
+ necessary in order for the ``DisplayContext`` to update the
+ :attr:`location` with respect to the currently selected overlay - if
+ the current overlay has shifted in the display coordinate system, we
+ want the :attr:`location` to shift with it.
+
+ :arg overlay: The overlay object (e.g. an :class:`.Image`).
+
+ :arg coords: Coordinates in the standard coordinate system of the
+ overlay.
+ """
+
+ if self.getParent() is not None and self.isSyncedToParent('location'):
+ return
+
+ locPropVal = self.getPropVal('location')
+ standardCoords = dict(locPropVal.getAttribute('standardCoords'))
+
+ standardCoords[overlay] = np.array(coords).tolist()
+
+ locPropVal.setAttribute('standardCoords', standardCoords)
+
+ log.debug('Standard coordinates cached '
+ 'for overlay {}: {} <-> {}'.format(
+ overlay.name,
+ self.location.xyz,
+ standardCoords))
+
+
def __overlayListChanged(self, *a):
"""Called when the :attr:`.OverlayList.overlays` property
changes.
@@ -514,12 +569,51 @@ class DisplayContext(props.SyncableHasProperties):
in the :class:`.OverlayList`.
"""
+ # If this DC is synced to a parent, let the
+ # parent do the update - our location property
+ # will be automatically synced to it. If we
+ # don't do this check, we will clobber the
+ # parent's updated location (or vice versa)
+ if self.getParent() is not None and self.isSyncedToParent('location'):
+ return
+
+ selectedOverlay = self.getSelectedOverlay()
+
+ if selectedOverlay is None:
+ return
+
+ selectedOpts = self.getOpts(selectedOverlay)
+
+ # The transform of the currently selected
+ # overlay might change, so we want the
+ # location to be preserved with respect to it.
+ stdLoc = selectedOpts.displayToStandardCoordinates(self.location.xyz)
+
for overlay in self.__overlayList:
if not isinstance(overlay, fslimage.Image):
continue
+ opts = self.getOpts(overlay)
+
+ # Disable the bounds listener, so the
+ # __overlayBoundsChanged method does
+ # not get called when the image
+ # transform changes.
+ opts.disableListener('bounds', self.__name)
self.__setTransform(overlay)
+ opts.enableListener( 'bounds', self.__name)
+
+ # Update the display world bounds,
+ # and then update the location
+ self.disableNotification('location')
+ self.__updateBounds()
+ self.enableNotification('location')
+
+ # making sure that the location is kept
+ # in the same place, relative to the
+ # currently selected overlay
+ self.location.xyz = selectedOpts.standardToDisplayCoordinates(stdLoc)
def __syncOverlayOrder(self):
@@ -594,58 +688,35 @@ class DisplayContext(props.SyncableHasProperties):
overlay.
"""
- # This check is ugly, and is required due to
- # an ugly circular relationship which exists
- # between parent/child DCs and the *Opts/
- # location properties:
- #
- # 1. When a property of a child DC DisplayOpts
- # object changes (e.g. ImageOpts.transform)
- # this should always be due to user input),
- # that change is propagated to the parent DC
- # DisplayOpts object.
- #
- # 2. This results in the DC._displayOptsChanged
- # method (this method) being called on the
- # parent DC.
- #
- # 3. Said method correctly updates the DC.location
- # property, so that the world location of the
- # selected overlay is preserved.
- #
- # 4. This location update is propagated back to
- # the child DC.location property, which is
- # updated to have the new correct location
- # value.
- #
- # 5. Then, the child DC._displayOpteChanged method
- # is called, which goes and updates the child
- # DC.location property to contain a bogus
- # value.
- #
- # So this test is in place to prevent this horrible
- # circular loop behaviour from occurring. If the
- # location properties are synced, we assume that
- # they don't need to be updated again, and escape
- # from ths system.
+ # See the note at top of __displaySpaceChanged
+ # method regarding this test
if self.getParent() is not None and self.isSyncedToParent('location'):
return
overlay = opts.display.getOverlay()
- # Save a copy of the location before
- # updating the bounds, as the change
- # to the bounds may result in the
- # location being modified
- oldDispLoc = self.location.xyz
-
+ # If the bounds of an overlay have changed, the
+ # overlay might have been moved in the dispaly
+ # coordinate system. We want to keep the
+ # current location in the same position, relative
+ # to that overlay. So we get the cached standard
+ # coords (which should have been updated by the
+ # overlay's DisplayOpts instance - see the docs
+ # for the cacheStandardCoordinates), and use them
+ # below to restore the location
+ locPropVal = self.getPropVal('location')
+ stdLoc = locPropVal.getAttribute('standardCoords')[overlay]
+
# Update the display context bounds
# to take into account any changes
- # to individual overlay bounds
+ # to individual overlay bounds.
+ # Inhibit notification on the location
+ # property - it will be updated properly
+ # below
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
# currently selected overlay, when the overlay
@@ -658,7 +729,7 @@ class DisplayContext(props.SyncableHasProperties):
# Now we want to update the display location
# so that it is preserved with respect to the
# currently selected overlay.
- newDispLoc = opts.transformDisplayLocation(oldDispLoc)
+ newDispLoc = opts.standardToDisplayCoordinates(stdLoc)
# Ignore the new display location
# if it is not in the display bounds
@@ -668,7 +739,7 @@ class DisplayContext(props.SyncableHasProperties):
overlay,
type(opts).__name__,
name,
- oldDispLoc,
+ stdLoc,
newDispLoc))
self.location.xyz = newDispLoc
diff --git a/fsl/fsleyes/displaycontext/group.py b/fsl/fsleyes/displaycontext/group.py
index 392592f0c5c300caff475636ffb9944060caf380..a40339b969b2880473296e59695c241823e89f06 100644
--- a/fsl/fsleyes/displaycontext/group.py
+++ b/fsl/fsleyes/displaycontext/group.py
@@ -63,8 +63,7 @@ class OverlayGroup(props.HasProperties):
_groupBindings = td.TypeDict({
'Display' : ['enabled'],
- 'ImageOpts' : ['volume',
- 'transform'],
+ 'ImageOpts' : ['volume'],
'VolumeOpts' : ['interpolation'],
'LabelOpts' : ['outline',
'outlineWidth'],
diff --git a/fsl/fsleyes/displaycontext/volumeopts.py b/fsl/fsleyes/displaycontext/volumeopts.py
index 8dbad52349ab14933cad5d33e9ff75d76d48904a..76f29a39fe82c2e00fc87481a204d0d8110b25f1 100644
--- a/fsl/fsleyes/displaycontext/volumeopts.py
+++ b/fsl/fsleyes/displaycontext/volumeopts.py
@@ -146,8 +146,14 @@ class ImageOpts(fsldisplay.DisplayOpts):
overlay = self.overlay
- self.addListener('transform', self.name, self.__transformChanged)
- self.addListener('customXform', self.name, self.__customXformChanged)
+ self.addListener('transform',
+ self.name,
+ self.__transformChanged,
+ immediate=True)
+ self.addListener('customXform',
+ self.name,
+ self.__customXformChanged,
+ immediate=True)
# The display<->* transformation matrices
# are created in the _setupTransforms method
@@ -179,6 +185,17 @@ class ImageOpts(fsldisplay.DisplayOpts):
:attr:`.DisplayOpts.bounds` property accordingly.
"""
+ oldValue = self.getLastValue('transform')
+
+ if oldValue is None:
+ oldValue = self.transform
+
+ self.displayCtx.cacheStandardCoordinates(
+ self.overlay,
+ self.transformCoords(self.displayCtx.location.xyz,
+ oldValue,
+ 'world'))
+
lo, hi = transform.axisBounds(
self.overlay.shape[:3],
self.getTransform('voxel', 'display'))
@@ -193,10 +210,23 @@ class ImageOpts(fsldisplay.DisplayOpts):
:meth:`__transformChanged`).
"""
+ stdLoc = self.displayToStandardCoordinates(
+ self.displayCtx.location.xyz)
+
self.__setupTransforms()
- self.__transformChanged()
+ if self.transform == 'custom':
+ self.__transformChanged()
+
+ # if transform == custom, the cached value
+ # calculated in __transformChanged will be
+ # wrong, so we have to overwrite it here.
+ # The stdLoc value calculated above is valid,
+ # because it was calculated before the
+ # transformation matrices were recalculated
+ # in __setupTransforms
+ self.displayCtx.cacheStandardCoordinates(self.overlay, stdLoc)
+
-
def __setupTransforms(self):
"""Calculates transformation matrices between all of the possible
spaces in which the overlay may be displayed.
@@ -347,8 +377,6 @@ class ImageOpts(fsldisplay.DisplayOpts):
- ``world``: The image world coordinate system
- ``custom``: The coordinate system defined by the custom
transformation matrix (see :attr:`customXform`)
-
- See also the :meth:`transformToVoxels` method.
"""
xform = self.getTransform( from_, to_)
@@ -360,36 +388,22 @@ class ImageOpts(fsldisplay.DisplayOpts):
return coords + post
- def transformDisplayLocation(self, oldLoc):
- """Overrides :meth:`.DisplayOpts.transformDisplayLocation`.
-
- If the :attr:`transform` property has changed, returns the given
- location, assumed to be in the old display coordinate system,
- transformed into the new display coordinate system.
-
- .. note:: This method will probably break if the ``custom``
- transformation matrix changes between the time that
- the ``transform`` property changes, and the time that
- this method is called.
+ def displayToStandardCoordinates(self, coords):
+ """Overrides :meth:`.DisplayOpts.displayToStandardCoordinates`.
+ Transforms the given display system coordinates into the world
+ coordinates of the :class:`.Image` associated with this
+ ``ImageOpts`` instance.
"""
+ return self.transformCoords(coords, 'display', 'world')
- 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.
- worldLoc = transform.transform(
- [oldLoc],
- self.getTransform(lastVal, 'world'))[0]
-
- newLoc = transform.transform(
- [worldLoc],
- self.getTransform('world', 'display'))[0]
-
- return newLoc
+
+ def standardToDisplayCoordinates(self, coords):
+ """Overrides :meth:`.DisplayOpts.standardToDisplayCoordinates`.
+ Transforms the given coordinates (assumed to be in the world
+ coordinate system of the ``Image`` associated with this ``ImageOpts``
+ instance) into the display coordinate system.
+ """
+ return self.transformCoords(coords, 'world', 'display')
class VolumeOpts(ImageOpts):
@@ -592,10 +606,9 @@ class VolumeOpts(ImageOpts):
# The parent.getChildren() method will
# contain this VolumeOpts instance,
# so the below loop toggles listeners
- # for this instance, the parent instance,
- # and all of the other children of the
- # parent
- peers = [parent] + parent.getChildren()
+ # for this instance and all of the other
+ # children of the parent
+ peers = parent.getChildren()
for peer in peers: