GLVolume (and other GL types - updates to come) cannot update shader

state until image textures are ready to be used. New async.wait function
to help with this.

fsl/fsleyes/displaycontext/display.py

@@ -406,9 +406,6 @@ class DisplayOpts(props.SyncableHasProperties, actions.ActionProvider):
         nounbind = kwargs.get('nounbind', [])
         nounbind.append('bounds')
         kwargs['nounbind'] = nounbind
-
-        props.SyncableHasProperties.__init__(self, **kwargs)
-        actions.ActionProvider     .__init__(self)
         
         self.overlay     = overlay
         self.display     = display
@@ -417,6 +414,9 @@ class DisplayOpts(props.SyncableHasProperties, actions.ActionProvider):
         self.overlayType = display.overlayType
         self.name        = '{}_{}'.format(type(self).__name__, id(self))
 
+        props.SyncableHasProperties.__init__(self, **kwargs)
+        actions.ActionProvider     .__init__(self)
+
         log.memory('{}.init ({})'.format(type(self).__name__, id(self)))
 
         

fsl/fsleyes/gl/glvolume.py

@@ -12,12 +12,13 @@ encapsulates the data and logic required to render 2D slice of an
 
 import logging
 
-import OpenGL.GL      as gl
+import OpenGL.GL       as gl
 
-import fsl.fsleyes.gl as fslgl
-import                   textures
-import                   globject
-import resources      as glresources
+import fsl.fsleyes.gl  as fslgl
+import fsl.utils.async as async
+import                    textures
+import                    globject
+import resources       as glresources
 
 
 log = logging.getLogger(__name__)
@@ -159,6 +160,11 @@ class GLVolume(globject.GLImageObject):
         # Create an image texture, clip texture, and a colour map texture
         self.texName  = '{}_{}'.format(type(self).__name__, id(self.image))
 
+        # Ref to an OpenGL shader program -
+        # the glvolume_funcs module will
+        # create this for us.
+        self.shader   = None
+
         # References to the clip image and
         # associated DisplayOpts instance,
         # if it is set.
@@ -177,12 +183,21 @@ class GLVolume(globject.GLImageObject):
         # make sure we're registered with it.
         if self.displayOpts.clipImage is not None:
             self.registerClipImage()
-
-        self.refreshImageTexture()
-        self.refreshClipTexture()
+            
         self.refreshColourTextures()
+
+        # We can't initialise the shaders until
+        # the image textures are ready to go.
+        # The image textures are created
+        # asynchronously, so we need to wait
+        # for them to finish before initialising
+        # the shaders.
+        def onTextureReady():
+            fslgl.glvolume_funcs.init(self)
+            self.notify()
         
-        fslgl.glvolume_funcs.init(self)
+        async.wait((self.refreshImageTexture(), self.refreshClipTexture()),
+                   onTextureReady)
 
 
     def destroy(self):
@@ -220,10 +235,12 @@ class GLVolume(globject.GLImageObject):
         """
 
         if self.displayOpts.clipImage is None:
-            return (self.imageTexture is not None and
+            return (self.shader       is not None and
+                    self.imageTexture is not None and
                     self.imageTexture.ready())
         else:
-            return (self.imageTexture is not None and
+            return (self.shader       is not None and
+                    self.imageTexture is not None and
                     self.clipTexture  is not None and
                     self.imageTexture.ready()     and
                     self.clipTexture .ready())
@@ -250,26 +267,22 @@ class GLVolume(globject.GLImageObject):
         
         def colourUpdate(*a):
             self.refreshColourTextures()
-            fslgl.glvolume_funcs.updateShaderState(self)
-            self.notify()
+            if self.ready():
+                fslgl.glvolume_funcs.updateShaderState(self)
+                self.notify()
 
         def shaderUpdate(*a):
-            fslgl.glvolume_funcs.updateShaderState(self)
-            self.notify()
+            if self.ready():
+                fslgl.glvolume_funcs.updateShaderState(self)
+                self.notify()
+
+        def onTextureRefresh():
+            if self.ready():
+                fslgl.glvolume_funcs.updateShaderState(self)
+                self.notify() 
 
         def imageRefresh(*a):
-            texChange = self.refreshImageTexture()
-            fslgl.glvolume_funcs.updateShaderState(self)
-
-            # If the texture settings have been changed,
-            # the texture will asynchronously notify
-            # this GLVolume (which is registered in the
-            # refreshImageTexture method). If texture
-            # settings have not been changed, the async
-            # notify will not occur, so we have to do it
-            # here.
-            if not texChange:
-                self.notify()
+            async.wait([self.refreshImageTexture()], onTextureRefresh)
 
         def imageUpdate(*a):
             volume     = opts.volume
@@ -278,23 +291,25 @@ class GLVolume(globject.GLImageObject):
             if opts.interpolation == 'none': interp = gl.GL_NEAREST
             else:                            interp = gl.GL_LINEAR
             
-            texChange = self.imageTexture.set(volume=volume,
-                                              interp=interp,
-                                              resolution=resolution)
+            self.imageTexture.set(volume=volume,
+                                  interp=interp,
+                                  resolution=resolution,
+                                  notify=False)
+
+            waitfor = [self.imageTexture.refreshThread()]
 
             if self.clipTexture is not None:
-                self.clipTexture.set(interp=interp, resolution=resolution)
+                self.clipTexture.set(interp=interp,
+                                     resolution=resolution,
+                                     notify=False)
+                waitfor.append(self.clipTexture.refreshThread())
 
-            fslgl.glvolume_funcs.updateShaderState(self)
-
-            if not texChange:
-                self.notify()
+            async.wait(waitfor, onTextureRefresh)
 
         def clipUpdate(*a):
             self.deregisterClipImage()
             self.registerClipImage()
-            self.refreshClipTexture()
-            fslgl.glvolume_funcs.updateShaderState(self)
+            async.wait([self.refreshClipTexture()], onTextureRefresh)
 
         display.addListener('alpha',          lName, colourUpdate,  weak=False)
         opts   .addListener('displayRange',   lName, colourUpdate,  weak=False)
@@ -394,7 +409,7 @@ class GLVolume(globject.GLImageObject):
         if self.imageTexture is not None:
             
             if self.imageTexture.getTextureName() == texName:
-                return
+                return None
             
             self.imageTexture.deregister(self.name)
             glresources.delete(self.imageTexture.getTextureName())
@@ -407,10 +422,13 @@ class GLVolume(globject.GLImageObject):
             textures.ImageTexture,
             texName,
             self.image,
-            interp=interp)
+            interp=interp,
+            notify=False)
 
         self.imageTexture.register(self.name, self.__textureChanged)
 
+        return self.imageTexture.refreshThread()
+
 
     def registerClipImage(self):
         """Called whenever the :attr:`.VolumeOpts.clipImage` property changes.
@@ -430,7 +448,8 @@ class GLVolume(globject.GLImageObject):
         
         def updateClipTexture(*a):
             self.clipTexture.set(volume=clipOpts.volume)
-            self.notify()
+            async.wait([self.clipTexture.refreshThread()],
+                       fslgl.glvolume_funcs.updateShaderState, self)
 
         clipOpts.addListener('volume',
                              self.name,
@@ -469,7 +488,7 @@ class GLVolume(globject.GLImageObject):
             self.clipTexture = None
             
         if clipImage is None:
-            return
+            return None
 
         if opts.interpolation == 'none': interp = gl.GL_NEAREST
         else:                            interp = gl.GL_LINEAR 
@@ -481,10 +500,13 @@ class GLVolume(globject.GLImageObject):
             clipImage,
             interp=interp,
             resolution=opts.resolution,
-            volume=clipOpts.volume)
+            volume=clipOpts.volume,
+            notify=False)
         
         self.clipTexture.register(self.name, self.__textureChanged)
 
+        return self.clipTexture.refreshThread()
+
     
     def refreshColourTextures(self):
         """Refreshes the :class:`.ColourMapTexture` instances used to colour

fsl/fsleyes/gl/textures/imagetexture.py

@@ -69,7 +69,8 @@ class ImageTexture(texture.Texture, notifier.Notifier):
                  prefilter=None,
                  interp=gl.GL_NEAREST,
                  resolution=None,
-                 volume=None):
+                 volume=None,
+                 notify=True):
         """Create an ``ImageTexture``. A listener is added to the
         :attr:`.Image.data` property, so that the texture data can be
         refreshed whenever the image data changes - see the
@@ -89,6 +90,8 @@ class ImageTexture(texture.Texture, notifier.Notifier):
         :arg prefilter: An optional function which may perform any 
                         pre-processing on the data before it is copied to the 
                         GPU - see the :meth:`__prepareTextureData` method.
+
+        :arg notify:    Passed to the initial call to :meth:`refresh`.
         """
 
         texture.Texture.__init__(self, name, 3)
@@ -114,13 +117,13 @@ class ImageTexture(texture.Texture, notifier.Notifier):
         self.__volume     = None
         self.__normalise  = None
 
-        # The dataMin/dataMax/ready attributes
-        # are modified in the refresh method
-        # (which is called via the set method
-        # below). 
-        self.__dataMin    = None
-        self.__dataMax    = None
-        self.__ready      = False
+        # These attributes are modified
+        # in the refresh method (which is
+        # called via the set method below). 
+        self.__dataMin       = None
+        self.__dataMax       = None
+        self.__ready         = False
+        self.__refreshThread = None
 
         self.image.addListener('data', self.__name, self.refresh)
 
@@ -130,14 +133,8 @@ class ImageTexture(texture.Texture, notifier.Notifier):
                  volume=volume,
                  normalise=normalise,
                  refresh=False)
-        self.__refresh()
-
-
-    def ready(self):
-        """Returns ``True`` if this ``ImageTexture`` is ready to be used,
-        ``False`` otherwise.
-        """
-        return self.__ready
+        
+        self.__refresh(notify=notify)
 
 
     def destroy(self):
@@ -149,6 +146,21 @@ class ImageTexture(texture.Texture, notifier.Notifier):
         texture.Texture.destroy(self)
         self.image.removeListener('data', self.__name)
 
+        
+    def ready(self):
+        """Returns ``True`` if this ``ImageTexture`` is ready to be used,
+        ``False`` otherwise.
+        """
+        return self.__ready
+
+
+    def refreshThread(self):
+        """If this ``ImageTexture`` is in the process of being refreshed
+        on another thread, this method returns a reference to the ``Thread``
+        object. Otherwise, this method returns ``None``.
+        """
+        return self.__refreshThread
+
 
     def setInterp(self, interp):
         """Sets the texture interpolation - either ``GL_NEAREST`` or
@@ -197,6 +209,7 @@ class ImageTexture(texture.Texture, notifier.Notifier):
         ``normalise``  See :meth:`setNormalise`.
         ``refresh``    If ``True`` (the default), the :meth:`refresh` function
                        is called (but only if a setting has changed).
+        ``notify``     Passed through to the :meth:`refresh` method.
         ============== =======================================================
 
         :returns: ``True`` if any settings have changed and the
@@ -208,6 +221,7 @@ class ImageTexture(texture.Texture, notifier.Notifier):
         volume     = kwargs.get('volume',     self.__volume)
         normalise  = kwargs.get('normalise',  self.__normalise)
         refresh    = kwargs.get('refresh',    True)
+        notify     = kwargs.get('notify',     True)
 
         changed = {'interp'     : interp     != self.__interp,
                    'prefilter'  : prefilter  != self.__prefilter,
@@ -241,7 +255,9 @@ class ImageTexture(texture.Texture, notifier.Notifier):
                             changed['normalise']))
 
         if refresh:
-            self.refresh(refreshData=refreshData, refreshRange=refreshRange)
+            self.refresh(refreshData=refreshData,
+                         refreshRange=refreshRange,
+                         notify=notify)
         
         return True
 
@@ -269,12 +285,19 @@ class ImageTexture(texture.Texture, notifier.Notifier):
         :arg refreshRange: If ``True`` (the default), the data range is
                            re-calculated.
 
+        :arg notify:       If ``True`` (the default), a notification is
+                           triggered via the :class:`.Notifier` base-class,
+                           when ``ImageTexture`` has been refreshed, and is
+                           ready to use. Otherwise, the notification is
+                           suppressed.
+
         .. note:: The texture data is generated on a separate thread, using
                   the :func:`.async.run` function. 
         """
 
         refreshData  = kwargs.get('refreshData',  True)
         refreshRange = kwargs.get('refreshRange', True)
+        notify       = kwargs.get('notify',       True)
 
         self.__ready = False
 
@@ -377,10 +400,14 @@ class ImageTexture(texture.Texture, notifier.Notifier):
             self.unbindTexture()
             log.debug('{}({}) is ready to use'.format(
                 type(self).__name__, self.image))
-            self.__ready = True
-            self.notify()
+            
+            self.__refreshThread = None
+            self.__ready         = True
+
+            if notify:
+                self.notify()
 
-        async.run(
+        self.__refreshThread = async.run(
             genData,
             onFinish=configTexture,
             name='{}.genData({})'.format(type(self).__name__, self.image))

fsl/utils/async.py

@@ -4,8 +4,7 @@
 #
 # Author: Paul McCarthy <pauldmccarthy@gmail.com>
 #
-"""This module provides a couple of functions for running tasks
-asynchronously - :func:`run` and :func:`idle`.
+"""This module provides functions for running tasks asynchronously.
 
 
 .. note:: The functions in this module are intended to be run from within a
@@ -26,6 +25,10 @@ The :func:`idle` function is a simple way to run a task on an ``wx``
 ``EVT_IDLE`` event handler. This effectively performs the same job as the
 :func:`run` function, but is more suitable for short tasks which do not
 warrant running in a separate thread.
+
+The :func:`wait` function is given one or more ``Thread`` instances, and a
+task to run. It waits until all the threads have finished, and then runs
+the task (via :func:`idle`).
 """
 
 import Queue
@@ -85,6 +88,7 @@ def run(task, onFinish=None, name=None):
             log.debug('Scheduling task "{}" finish handler '
                       'on wx.MainLoop'.format(name))
 
+            # Should I use the idle function here?
             wx.CallAfter(onFinish)
 
     if haveWX:
@@ -122,7 +126,8 @@ def _wxIdleLoop(ev):
 
     try:                task, args, kwargs = _idleQueue.get_nowait()
     except Queue.Empty: return
-        
+
+    log.debug('Running function on wx idle loop')
     task(*args, **kwargs)
 
     if _idleQueue.qsize() > 0:
@@ -135,6 +140,8 @@ def idle(task, *args, **kwargs):
     :arg task: The task to run.
 
     All other arguments are passed through to the task function.
+
+    If a ``wx.App`` is not running, the task is called directly.
     """
 
     global _idleRegistered
@@ -154,3 +161,39 @@ def idle(task, *args, **kwargs):
     else:
         log.debug('Running idle task directly') 
         task(*args, **kwargs)
+
+
+def wait(threads, task, *args, **kwargs):
+    """Creates and starts a new ``Thread`` which waits for all of the ``Thread``
+    instances to finsih (by ``join``ing them), and then runs the given
+    ``task`` via :func:`idle`.
+
+    If a ``wx.App`` is not running, this function ``join``s the threads
+    directly instead of creating a new ``Thread`` to do so.
+
+    :arg threads: A sequence of ``Thread`` instances to join. Elements in the
+                  sequence may be ``None``.
+
+    :arg task:    The task to run.
+
+    All other arguments are passed to the ``task`` function.
+    """
+    haveWX = _haveWX()
+
+    def joinAll():
+        log.debug('Wait thread joining on all targets')
+        for t in threads:
+            if t is not None:
+                t.join()
+
+        log.debug('Wait thread scheduling task on idle loop')
+        idle(task, *args, **kwargs)
+
+    if haveWX:
+        thread = threading.Thread(target=joinAll)
+        thread.start()
+        return thread
+    
+    else:
+        joinAll()
+        return None