From bc35b72b549755e4c5d888d11f1617c0bc980fdc Mon Sep 17 00:00:00 2001
From: Paul McCarthy <pauld.mccarthy@gmail.com>
Date: Sat, 14 Nov 2015 14:34:54 +0000
Subject: [PATCH] Bugfix - GLObject types were not removing sync change
 property listeners in some circumstances, and were not calling
 GLObject.onUpdate when transform property changed

---
 fsl/fsleyes/gl/gllabel.py  | 12 ++++++++++--
 fsl/fsleyes/gl/glmask.py   | 12 ++++++++++--
 fsl/fsleyes/gl/glvector.py | 12 ++++++++++--
 fsl/fsleyes/gl/glvolume.py | 20 +++++++++++++++-----
 4 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/fsl/fsleyes/gl/gllabel.py b/fsl/fsleyes/gl/gllabel.py
index e95017bac..ae358a136 100644
--- a/fsl/fsleyes/gl/gllabel.py
+++ b/fsl/fsleyes/gl/gllabel.py
@@ -86,6 +86,9 @@ class GLLabel(globject.GLImageObject):
         opts    = self.displayOpts
         name    = self.name
 
+        def update(*a):
+            self.onUpdate()
+
         def shaderUpdate(*a):
             fslgl.gllabel_funcs.updateShaderState(self)
             self.onUpdate()
@@ -133,9 +136,13 @@ class GLLabel(globject.GLImageObject):
         opts    .addListener('lut',          name, lutChanged,    weak=False)
         opts    .addListener('volume',       name, imageUpdate,   weak=False)
         opts    .addListener('resolution',   name, imageUpdate,   weak=False)
+        opts    .addListener('transform',    name, update,        weak=False)
         opts.lut.addListener('labels',       name, lutUpdate,     weak=False)
 
-        if opts.getParent() is not None:
+        # See comment in GLVolume.addDisplayListeners about this
+        self.__syncListenersRegistered = opts.getParent() is not None
+
+        if self.__syncListenersRegistered: 
             opts.addSyncChangeListener(
                 'volume',     name, imageRefresh, weak=False)
             opts.addSyncChangeListener(
@@ -158,9 +165,10 @@ class GLLabel(globject.GLImageObject):
         opts    .removeListener(          'lut',          name)
         opts    .removeListener(          'volume',       name)
         opts    .removeListener(          'resolution',   name)
+        opts    .removeListener(          'transform',    name)
         opts.lut.removeListener(          'labels',       name)
 
-        if opts.getParent() is not None:
+        if self.__syncListenersRegistered:
             opts.removeSyncChangeListener('volume',     name)
             opts.removeSyncChangeListener('resolution', name)
 
diff --git a/fsl/fsleyes/gl/glmask.py b/fsl/fsleyes/gl/glmask.py
index ac8547e4e..c246f3344 100644
--- a/fsl/fsleyes/gl/glmask.py
+++ b/fsl/fsleyes/gl/glmask.py
@@ -47,6 +47,9 @@ class GLMask(glvolume.GLVolume):
         opts    = self.displayOpts
         name    = self.name
 
+        def update(*a):
+            self.onUpdate()
+
         def shaderCompile(*a):
             fslgl.glvolume_funcs.compileShaders(   self)
             fslgl.glvolume_funcs.updateShaderState(self)
@@ -83,8 +86,12 @@ class GLMask(glvolume.GLVolume):
         opts   .addListener('invert',        name, colourUpdate,  weak=False)
         opts   .addListener('volume',        name, imageUpdate,   weak=False)
         opts   .addListener('resolution',    name, imageUpdate,   weak=False)
+        opts   .addListener('transform',     name, update,        weak=False)
+
+        # See comment in GLVolume.addDisplayListeners about this
+        self.__syncListenersRegistered = opts.getParent() is not None
 
-        if opts.getParent() is not None:
+        if self.__syncListenersRegistered:
             opts.addSyncChangeListener(
                 'volume',     name, imageRefresh, weak=False)
             opts.addSyncChangeListener(
@@ -109,8 +116,9 @@ class GLMask(glvolume.GLVolume):
         opts   .removeListener(          'invert',        name)
         opts   .removeListener(          'volume',        name)
         opts   .removeListener(          'resolution',    name)
+        opts   .removeListener(          'transform',     name)
 
-        if opts.getParent() is not None:
+        if self.__syncListenersRegistered:
             opts.removeSyncChangeListener('volume',     name)
             opts.removeSyncChangeListener('resolution', name)
 
diff --git a/fsl/fsleyes/gl/glvector.py b/fsl/fsleyes/gl/glvector.py
index a6e361f3d..886cfd75f 100644
--- a/fsl/fsleyes/gl/glvector.py
+++ b/fsl/fsleyes/gl/glvector.py
@@ -127,6 +127,9 @@ class GLVector(globject.GLImageObject):
         display = self.display
         opts    = self.displayOpts
         name    = self.name
+
+        def update(*a):
+            self.onUpdate()
         
         def modUpdate( *a):
             self.refreshModulateTexture()
@@ -170,8 +173,12 @@ class GLVector(globject.GLImageObject):
         opts   .addListener('modulate',      name, modUpdate,     weak=False)
         opts   .addListener('modThreshold',  name, shaderUpdate,  weak=False)
         opts   .addListener('resolution',    name, imageUpdate,   weak=False)
+        opts   .addListener('transform',     name, update,        weak=False)
+
+        # See comment in GLVolume.addDisplayListeners about this
+        self.__syncListenersRegistered = opts.getParent() is not None 
 
-        if opts.getParent() is not None:
+        if self.__syncListenersRegistered:
             opts.addSyncChangeListener(
                 'resolution', name, imageRefresh, weak=False)
 
@@ -198,8 +205,9 @@ class GLVector(globject.GLImageObject):
         opts   .removeListener('modThreshold', name)
         opts   .removeListener('volume',       name)
         opts   .removeListener('resolution',   name)
+        opts   .removeListener('transform' ,   name)
 
-        if opts.getParent() is not None:
+        if self.__syncListenersRegistered:
             opts.removeSyncChangeListener('resolution', name)
 
 
diff --git a/fsl/fsleyes/gl/glvolume.py b/fsl/fsleyes/gl/glvolume.py
index 2608e926a..2a9317a70 100644
--- a/fsl/fsleyes/gl/glvolume.py
+++ b/fsl/fsleyes/gl/glvolume.py
@@ -163,6 +163,9 @@ class GLVolume(globject.GLImageObject):
         display = self.display
         opts    = self.displayOpts
         lName   = self.name
+
+        def update(*a):
+            self.onUpdate()
         
         def colourUpdate(*a):
             self.refreshColourTexture()
@@ -183,9 +186,6 @@ class GLVolume(globject.GLImageObject):
             fslgl.glvolume_funcs.updateShaderState(self)
             self.onUpdate()
 
-        def update(*a):
-            self.onUpdate()
-
         def imageUpdate(*a):
             volume     = opts.volume
             resolution = opts.resolution
@@ -211,7 +211,16 @@ class GLVolume(globject.GLImageObject):
         opts   .addListener('interpolation',  lName, imageUpdate,   weak=False)
         opts   .addListener('transform',      lName, update,        weak=False)
 
-        if opts.getParent() is not None:
+        # Save a flag so the removeDisplayListeners
+        # method knows whether it needs to de-register
+        # sync change listeners - using opts.getParent()
+        # as the test in that method is dangerous, as
+        # the DisplayOpts instance might have already
+        # had its destroy method called on it, and might
+        # have been detached from its parent.
+        self.__syncListenersRegistered = opts.getParent() is not None
+
+        if self.__syncListenersRegistered:
             opts.addSyncChangeListener(
                 'volume',        lName, imageRefresh, weak=False)
             opts.addSyncChangeListener(
@@ -240,7 +249,8 @@ class GLVolume(globject.GLImageObject):
         opts   .removeListener(          'resolution',     lName)
         opts   .removeListener(          'interpolation',  lName)
         opts   .removeListener(          'transform',      lName)
-        if opts.getParent() is not None:
+        
+        if self.__syncListenersRegistered:
             opts.removeSyncChangeListener('volume',        lName)
             opts.removeSyncChangeListener('resolution',    lName)
             opts.removeSyncChangeListener('interpolation', lName)
-- 
GitLab