From d132b40e3baaa35145d8d8685abe151ace719e1b Mon Sep 17 00:00:00 2001
From: David Manthey <david.manthey@kitware.com>
Date: Mon, 17 Dec 2018 13:57:07 -0500
Subject: [PATCH] Improve cleanup.

When removing an actor, cleanup after it so it can be moved to another
renderer.
---
 .npmignore                               |  1 +
 src/mapper.js                            |  6 ++++++
 src/material.js                          |  2 ++
 src/renderWindow.js                      |  1 +
 src/renderer.js                          | 13 ++++++++++---
 src/shaderProgram.js                     |  9 +++++++--
 testing/cases/phantomjs/material.js      |  2 ++
 testing/cases/phantomjs/shaderProgram.js |  1 +
 8 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/.npmignore b/.npmignore
index 9f5f00e..24fd503 100644
--- a/.npmignore
+++ b/.npmignore
@@ -2,4 +2,5 @@
 !/vgl.js
 !/vgl.min.js
 !/vgl.min.js.map
+!src/**/*
 .eslintcache
diff --git a/src/mapper.js b/src/mapper.js
index ecc8522..d96a36d 100644
--- a/src/mapper.js
+++ b/src/mapper.js
@@ -52,6 +52,12 @@ vgl.mapper = function (arg) {
     }
   };
 
+  this._cleanup = function (renderState) {
+    m_this.deleteVertexBufferObjects(renderState);
+    cleanUpDrawObjects(renderState);
+    m_this.modified();
+  };
+
   ////////////////////////////////////////////////////////////////////////////
   /**
    * Create new VBO for all its geometryData sources and primitives
diff --git a/src/material.js b/src/material.js
index 4cac302..ff6bb01 100644
--- a/src/material.js
+++ b/src/material.js
@@ -211,6 +211,8 @@ vgl.material = function () {
         m_textureAttributes[key]._cleanup(renderState);
       }
     }
+    m_shaderProgram._cleanup(renderState);
+    m_this.modified();
   };
 
   ////////////////////////////////////////////////////////////////////////////
diff --git a/src/renderWindow.js b/src/renderWindow.js
index 564ee32..fbec24d 100644
--- a/src/renderWindow.js
+++ b/src/renderWindow.js
@@ -294,6 +294,7 @@ vgl.renderWindow = function (canvas) {
       m_renderers[i]._cleanup(renderState);
     }
     vgl.clearCachedShaders(renderState ? renderState.m_context : null);
+    m_this.modified();
   };
 
   ////////////////////////////////////////////////////////////////////////////
diff --git a/src/renderer.js b/src/renderer.js
index 4b27b7e..17fc5c9 100644
--- a/src/renderer.js
+++ b/src/renderer.js
@@ -298,6 +298,7 @@ vgl.renderer = function (arg) {
 
     renSt.m_context.finish();
     m_this.m_contextChanged = false;
+    m_this.m_lastRenderState = renSt;
   };
 
   ////////////////////////////////////////////////////////////////////////////
@@ -561,10 +562,15 @@ vgl.renderer = function (arg) {
     if (m_this.m_sceneRoot.children().indexOf(actor) !== -1) {
       /* When we remove an actor, free the VBOs of the mapper and mark the
        * mapper as modified; it will reallocate VBOs as necessary. */
-      if (actor.mapper()) {
-        actor.mapper().deleteVertexBufferObjects();
-        actor.mapper().modified();
+      if (m_this.m_lastRenderState) {
+        if (actor.mapper()) {
+          actor.mapper()._cleanup(m_this.m_lastRenderState);
+        }
+        if (actor.material()) {
+          actor.material()._cleanup(m_this.m_lastRenderState);
+        }
       }
+      actor.modified();
       m_this.m_sceneRoot.removeChild(actor);
       m_this.modified();
       return true;
@@ -785,6 +791,7 @@ vgl.renderer = function (arg) {
     }
 
     m_this.m_sceneRoot.removeChildren();
+    m_this.modified();
   };
 
   return m_this;
diff --git a/src/shaderProgram.js b/src/shaderProgram.js
index 5d44b93..8a9b0d0 100644
--- a/src/shaderProgram.js
+++ b/src/shaderProgram.js
@@ -276,6 +276,7 @@ vgl.shaderProgram = function () {
   this._cleanup = function (renderState) {
     m_this.deleteVertexAndFragment(renderState);
     m_this.deleteProgram(renderState);
+    m_this.modified();
   };
 
   /////////////////////////////////////////////////////////////////////////////
@@ -284,7 +285,9 @@ vgl.shaderProgram = function () {
    */
   /////////////////////////////////////////////////////////////////////////////
   this.deleteProgram = function (renderState) {
-    renderState.m_context.deleteProgram(m_programHandle);
+    if (m_programHandle) {
+      renderState.m_context.deleteProgram(m_programHandle);
+    }
     m_programHandle = 0;
   };
 
@@ -296,7 +299,9 @@ vgl.shaderProgram = function () {
   this.deleteVertexAndFragment = function (renderState) {
     var i;
     for (i = 0; i < m_shaders.length; i += 1) {
-      renderState.m_context.detachShader(m_shaders[i].shaderHandle(renderState));
+      if (m_shaders[i].shaderHandle(renderState)) {
+        renderState.m_context.detachShader(m_programHandle, m_shaders[i].shaderHandle(renderState));
+      }
       renderState.m_context.deleteShader(m_shaders[i].shaderHandle(renderState));
       m_shaders[i].removeContext(renderState);
     }
diff --git a/testing/cases/phantomjs/material.js b/testing/cases/phantomjs/material.js
index f905a00..27c2d04 100644
--- a/testing/cases/phantomjs/material.js
+++ b/testing/cases/phantomjs/material.js
@@ -139,6 +139,8 @@ describe('vgl.material', function () {
       mat.addAttribute(blend);
       mat.addAttribute(tex);
       mat.setAttribute(prog);
+      mat.bind(renderState);
+      mat.undoBind(renderState);
       glCounts = $.extend({}, vgl.mockCounts());
       mat._cleanup(renderState);
       expect(vgl.mockCounts().deleteProgram).toBe((glCounts.deleteProgram || 0) + 1);
diff --git a/testing/cases/phantomjs/shaderProgram.js b/testing/cases/phantomjs/shaderProgram.js
index c3e5180..901967e 100644
--- a/testing/cases/phantomjs/shaderProgram.js
+++ b/testing/cases/phantomjs/shaderProgram.js
@@ -127,6 +127,7 @@ describe('vgl.shaderProgram', function () {
       expect(vgl.mockCounts().useProgram).toBe((glCounts.useProgram || 0) + 1);
     });
     it('deleteProgram', function () {
+      sp._setup(renderState);
       glCounts = $.extend({}, vgl.mockCounts());
       sp.deleteProgram(renderState);
       expect(vgl.mockCounts().deleteProgram).toBe((glCounts.deleteProgram || 0) + 1);