From f7495a0fa1f61faa032961c6766a806f98697b69 Mon Sep 17 00:00:00 2001
From: Paul McCarthy <pauld.mccarthy@gmail.com>
Date: Tue, 14 Jul 2015 15:17:53 +0100
Subject: [PATCH] Making sure that Display, *Opts and overlay instances are
GC'd appropriately. Some things are broken due to weakref-related changes in
the props package.
---
fsl/data/featimage.py | 4 +
fsl/fslview/controls/clusterpanel.py | 48 +++++++-
fsl/fslview/displaycontext/display.py | 26 ++++-
fsl/fslview/displaycontext/displaycontext.py | 19 +++-
fsl/fslview/displaycontext/volumeopts.py | 11 +-
fsl/fslview/gl/globject.py | 10 ++
fsl/fslview/gl/glvolume.py | 19 +++-
fsl/fslview/gl/slicecanvas.py | 113 ++++++++++---------
8 files changed, 177 insertions(+), 73 deletions(-)
diff --git a/fsl/data/featimage.py b/fsl/data/featimage.py
index e6f6f9430..c87121f21 100644
--- a/fsl/data/featimage.py
+++ b/fsl/data/featimage.py
@@ -58,6 +58,10 @@ class FEATImage(fslimage.Image):
self.name = '{}: {}'.format(self.__analysisName, self.name)
+ def getFEATDir(self):
+ return self.__featDir
+
+
def getAnalysisName(self):
return self.__analysisName
diff --git a/fsl/fslview/controls/clusterpanel.py b/fsl/fslview/controls/clusterpanel.py
index 8cd1d2d97..f22c70f9f 100644
--- a/fsl/fslview/controls/clusterpanel.py
+++ b/fsl/fslview/controls/clusterpanel.py
@@ -65,7 +65,7 @@ class ClusterPanel(fslpanel.FSLViewPanel):
overlayList.addListener('overlays',
self._name,
- self.__selectedOverlayChanged)
+ self.__overlayListChanged)
displayCtx .addListener('selectedOverlay',
self._name,
self.__selectedOverlayChanged)
@@ -137,26 +137,60 @@ class ClusterPanel(fslpanel.FSLViewPanel):
self._displayCtx.getDisplay(mask).overlayType = 'label'
- def __selectedOverlayChanged(self, *a):
+ def __overlayListChanged(self, *a):
+ self.__selectedOverlayChanged()
+ self.__enableOverlayButtons()
- self.__statSelect .Clear()
- self.__clusterList.ClearGrid()
- self.__selectedOverlay = None
+ def __enableOverlayButtons(self):
+
+ if self.__selectedOverlay is None:
+ return
+
+ overlay = self.__selectedOverlay
+ contrast = self.__statSelect.GetSelection()
+ zstat = overlay.getZStats( contrast)
+ clustMask = overlay.getClusterMask(contrast)
+
+ dss = [ovl.dataSource for ovl in self._overlayList]
+
+ self.__addZStats .Enable(zstat .dataSource not in dss)
+ self.__addClusterMask.Enable(clustMask.dataSource not in dss)
+
+
+ def __selectedOverlayChanged(self, *a):
+
+ prevOverlay = self.__selectedOverlay
+ self.__selectedOverlay = None
+
# No overlays are loaded
if len(self._overlayList) == 0:
self.__disable(strings.messages[self, 'noOverlays'])
return
overlay = self._displayCtx.getSelectedOverlay()
-
+
# Not a FEAT image, can't
# do anything with that
if not isinstance(overlay, featimage.FEATImage):
self.__disable(strings.messages[self, 'notFEAT'])
return
+ # Selected overlay is either the
+ # same one (maybe the overlay list,
+ # rather than the selected overlay,
+ # changed) or the newly selected
+ # overlay is from the same FEAT
+ # analysis. No need to do anything.
+ if prevOverlay is not None and (prevOverlay is overlay or
+ prevOverlay.getFEATDir() == overlay.getFEATDir()):
+ self.__selectedOverlay = overlay
+ return
+
+ self.__statSelect .Clear()
+ self.__clusterList.ClearGrid()
+
self.__selectedOverlay = overlay
self.__sizer.Show(self.__disabledText, False)
@@ -190,6 +224,7 @@ class ClusterPanel(fslpanel.FSLViewPanel):
self.__statSelect.Append(name, clusterList)
self.__overlayName.SetLabel(overlay.getAnalysisName())
+
self.__statSelect.SetSelection(0)
self.__displayClusterData(clusts[0][1])
@@ -201,6 +236,7 @@ class ClusterPanel(fslpanel.FSLViewPanel):
idx = self.__statSelect.GetSelection()
data = self.__statSelect.GetClientData(idx)
self.__displayClusterData(data)
+ self.__enableOverlayButtons()
def __displayClusterData(self, clusters):
diff --git a/fsl/fslview/displaycontext/display.py b/fsl/fslview/displaycontext/display.py
index 34e292ac1..4d930bd31 100644
--- a/fsl/fslview/displaycontext/display.py
+++ b/fsl/fslview/displaycontext/display.py
@@ -49,7 +49,14 @@ class DisplayOpts(props.SyncableHasProperties):
def destroy(self):
- pass
+ """If overridden, this method should be called by the subclass
+ implementation.
+ """
+
+ self.overlay = None
+ self.display = None
+ self.overlayList = None
+ self.displayCtx = None
def getReferenceImage(self):
@@ -193,6 +200,23 @@ class Display(props.SyncableHasProperties):
self.__displayOpts = None
self.__overlayTypeChanged()
+
+
+ def destroy(self):
+ """This method should be called when this ``Display`` instance
+ is no longer needed.
+ """
+
+ if self.__displayOpts is not None:
+ self.__displayOpts.destroy()
+
+ self.removeListener('overlayType', 'Display_{}'.format(id(self)))
+
+ self.detachFromParent()
+
+ self.__displayOpts = None
+ self.__overlay = None
+
def getDisplayOpts(self):
"""
diff --git a/fsl/fslview/displaycontext/displaycontext.py b/fsl/fslview/displaycontext/displaycontext.py
index 125c65802..0fab40e11 100644
--- a/fsl/fslview/displaycontext/displaycontext.py
+++ b/fsl/fslview/displaycontext/displaycontext.py
@@ -208,8 +208,25 @@ class DisplayContext(props.SyncableHasProperties):
constraints on the :attr:`selectedOverlay` property.
"""
+ # Discard all Display instances
+ # which refer to overlays that
+ # are no longer in the list
+ for overlay in list(self.__displays.keys()):
+ if overlay not in self.__overlayList:
+
+ display = self.__displays.pop(overlay)
+ opts = display.getDisplayOpts()
+
+ display.removeListener('overlayType', self.__name)
+ opts.removeGlobalListener(self.__name)
+
+ # The display instance will destroy the
+ # opts instance, so we don't do it here
+ display.destroy()
+
# Ensure that a Display object
- # exists for every overlay
+ # exists for every overlay in
+ # the list
for overlay in self.__overlayList:
# The getDisplay method
diff --git a/fsl/fslview/displaycontext/volumeopts.py b/fsl/fslview/displaycontext/volumeopts.py
index e20d3c525..2a189638c 100644
--- a/fsl/fslview/displaycontext/volumeopts.py
+++ b/fsl/fslview/displaycontext/volumeopts.py
@@ -88,7 +88,7 @@ class ImageOpts(fsldisplay.DisplayOpts):
def destroy(self):
- pass
+ fsldisplay.DisplayOpts.destroy(self)
def getDisplayBounds(self):
@@ -327,11 +327,10 @@ class VolumeOpts(ImageOpts):
display.getSyncPropertyName('brightness'))
self.bindProps(self .getSyncPropertyName('displayRange'),
display,
- display.getSyncPropertyName('contrast'))
+ display.getSyncPropertyName('contrast'))
- def destroy(self):
- ImageOpts.destroy(self)
+ def destroy(self):
if self.getParent() is not None:
display = self.display
@@ -343,7 +342,9 @@ class VolumeOpts(ImageOpts):
display.getSyncPropertyName('brightness'))
self.unbindProps(self .getSyncPropertyName('displayRange'),
display,
- display.getSyncPropertyName('contrast'))
+ display.getSyncPropertyName('contrast'))
+
+ ImageOpts.destroy(self)
def __toggleListeners(self, enable=True):
diff --git a/fsl/fslview/gl/globject.py b/fsl/fslview/gl/globject.py
index 861b87116..ce1463265 100644
--- a/fsl/fslview/gl/globject.py
+++ b/fsl/fslview/gl/globject.py
@@ -226,6 +226,16 @@ class GLImageObject(GLObject):
self.displayOpts = display.getDisplayOpts()
+ def destroy(self):
+ """If this method is overridden, it should be called by the subclass
+ implementation. It clears references to the :class:`.Image`,
+ :class:`.Display`, and :class:`.DisplayOpts` instances.
+ """
+ self.image = None
+ self.display = None
+ self.displayOpts = None
+
+
def getDisplayBounds(self):
return self.displayOpts.getDisplayBounds()
diff --git a/fsl/fslview/gl/glvolume.py b/fsl/fslview/gl/glvolume.py
index b0ba1d940..c5eb1f7be 100644
--- a/fsl/fslview/gl/glvolume.py
+++ b/fsl/fslview/gl/glvolume.py
@@ -111,6 +111,8 @@ class GLVolume(globject.GLImageObject):
self.removeDisplayListeners()
fslgl.glvolume_funcs.destroy(self)
+
+ globject.GLImageObject.destroy(self)
def testUnsynced(self):
@@ -121,6 +123,8 @@ class GLVolume(globject.GLImageObject):
:class:`GLVolume` instance needs to create its own image texture;
returns ``False`` otherwise.
"""
+ if self.displayOpts.getParent() is None:
+ return True
return (not self.displayOpts.isSyncedToParent('volume') or
not self.displayOpts.isSyncedToParent('resolution') or
not self.displayOpts.isSyncedToParent('interpolation'))
@@ -225,9 +229,11 @@ class GLVolume(globject.GLImageObject):
opts .addListener( 'volume', lName, imageUpdate)
opts .addListener( 'resolution', lName, imageUpdate)
opts .addListener( 'interpolation', lName, imageUpdate)
- opts .addSyncChangeListener('volume', lName, imageRefresh)
- opts .addSyncChangeListener('resolution', lName, imageRefresh)
- opts .addSyncChangeListener('interpolation', lName, imageRefresh)
+
+ if opts.getParent() is not None:
+ opts .addSyncChangeListener('volume', lName, imageRefresh)
+ opts .addSyncChangeListener('resolution', lName, imageRefresh)
+ opts .addSyncChangeListener('interpolation', lName, imageRefresh)
def removeDisplayListeners(self):
@@ -250,9 +256,10 @@ class GLVolume(globject.GLImageObject):
opts .removeListener( 'volume', lName)
opts .removeListener( 'resolution', lName)
opts .removeListener( 'interpolation', lName)
- opts .removeSyncChangeListener('volume', lName)
- opts .removeSyncChangeListener('resolution', lName)
- opts .removeSyncChangeListener('interpolation', lName)
+ if opts.getParent() is not None:
+ opts .removeSyncChangeListener('volume', lName)
+ opts .removeSyncChangeListener('resolution', lName)
+ opts .removeSyncChangeListener('interpolation', lName)
def preDraw(self):
diff --git a/fsl/fslview/gl/slicecanvas.py b/fsl/fslview/gl/slicecanvas.py
index 4ede3efb6..63eaa4df6 100644
--- a/fsl/fslview/gl/slicecanvas.py
+++ b/fsl/fslview/gl/slicecanvas.py
@@ -505,6 +505,60 @@ class SliceCanvas(props.HasProperties):
# display axes. Easiest way to do this
# is to destroy and re-create them
self._renderModeChange()
+
+
+ def __overlayTypeChanged(self, value, valid, display, name):
+ """Called when the :attr:`.Display.overlayType` setting for any
+ overlay changes. Makes sure that an appropriate :class:`.GLObject`
+ has been created for the overlay (see the :meth:`__genGLObject`
+ method).
+ """
+
+ log.debug('GLObject representation for {} '
+ 'changed to {}'.format(display.name,
+ display.overlayType))
+
+ self.__genGLObject(display.getOverlay(), display)
+ self._refresh()
+
+
+ def __genGLObject(self, overlay, display, updateRenderTextures=True):
+ """Creates a :class:`.GLObject` instance for the given ``overlay``,
+ destroying any existing instance.
+
+ If ``updateRenderTextures`` is ``True`` (the default), and the
+ :attr:`.renderMode` is ``offscreen`` or ``prerender``, any
+ render texture associated with the overlay is destroyed.
+ """
+
+ if not self._setGLContext():
+ return
+
+ # Tell the previous GLObject (if
+ # any) to clean up after itself
+ globj = self._glObjects.get(overlay, None)
+ if globj is not None:
+ globj.destroy()
+
+ if updateRenderTextures:
+ if self.renderMode == 'offscreen':
+ tex = self._offscreenTextures.pop(overlay, None)
+ if tex is not None:
+ tex.destroy()
+
+ elif self.renderMode == 'prerender':
+ tex, name = self._prerenderTextures.pop(
+ overlay, (None, None))
+ if tex is not None:
+ glresources.delete(name)
+
+ globj = globject.createGLObject(overlay, display)
+
+ if globj is not None:
+ globj.setAxes(self.xax, self.yax)
+ globj.addUpdateListener(self.name, self._refresh)
+
+ self._glObjects[overlay] = globj
def _overlayListChanged(self, *a):
@@ -537,64 +591,15 @@ class SliceCanvas(props.HasProperties):
display = self.displayCtx.getDisplay(overlay)
- # Called when the GL object representation
- # of the overlay needs to be re-created
- #
- # TODO make this an instance method,
- # instead of an inner method
- def genGLObject(value=None,
- valid=None,
- ctx=None,
- name=None,
- disp=display,
- ovl=overlay,
- updateRenderTextures=True):
-
- log.debug('GLObject representation for {} '
- 'changed to {}'.format(disp.name,
- disp.overlayType))
-
- if not self._setGLContext():
- return
-
- # Tell the previous GLObject (if
- # any) to clean up after itself
- globj = self._glObjects.get(ovl, None)
- if globj is not None:
- globj.destroy()
-
- if updateRenderTextures:
- if self.renderMode == 'offscreen':
- tex = self._offscreenTextures.pop(ovl, None)
- if tex is not None:
- tex.destroy()
-
- elif self.renderMode == 'prerender':
- tex, name = self._prerenderTextures.pop(
- ovl, (None, None))
- if tex is not None:
- glresources.delete(name)
-
- globj = globject.createGLObject(ovl, disp)
-
- if globj is not None:
- globj.setAxes(self.xax, self.yax)
-
- self._glObjects[ovl] = globj
-
- if updateRenderTextures:
- self._updateRenderTextures()
-
- globj.addUpdateListener(self.name, self._refresh)
-
- self._refresh()
-
- genGLObject(updateRenderTextures=False)
+ self.__genGLObject(overlay, display, updateRenderTextures=False)
# Bind Display.softwareMode to SliceCanvas.softwareMode
display.bindProps('softwareMode', self)
- display.addListener('overlayType', self.name, genGLObject)
+ display.addListener('overlayType',
+ self.name,
+ self.__overlayTypeChanged)
+
display.addListener('enabled', self.name, self._refresh)
display.addListener('softwareMode', self.name, self._refresh)
--
GitLab