Fix ShaderProgram ownership bug, introduced in commit 67a723477ecd818fbc5859fe20ee536a3b4efae5 (reverting and clarifying)

All Graph ShaderPrograms used are owned by RegionRenderer, not RenderState nor [GL]Region*,
hence [GL]Region* shall only nullify the resources but not destroy the shader currently in use.

One RegionRenderer maybe used for multuple Regions.

6 files changed, 45 insertions, 70 deletions

diff --git a/src/jogl/classes/com/jogamp/graph/curve/opengl/GLRegion.java b/src/jogl/classes/com/jogamp/graph/curve/opengl/GLRegion.java
index a86060fab..c8e9a0113 100644
--- a/src/jogl/classes/com/jogamp/graph/curve/opengl/GLRegion.java
+++ b/src/jogl/classes/com/jogamp/graph/curve/opengl/GLRegion.java
@@ -179,7 +179,6 @@ public abstract class GLRegion extends Region {
      */

     protected abstract void updateImpl(final GL2ES2 gl);

 

-    protected abstract void clearShaderImpl(final GL2ES2 gl);

     protected abstract void destroyImpl(final GL2ES2 gl);

 

     protected abstract void clearImpl(final GL2ES2 gl);

@@ -212,16 +211,14 @@ public abstract class GLRegion extends Region {
         return this;

     }

 

-    /** Deletes all {@link ShaderProgram}s and nullifies its references. */

-    public final void clearShader(final GL2ES2 gl) {

-        clearShaderImpl(gl);

-    }

-

     /**

      * Delete and clear the associated OGL objects.

+     * <p>

+     * The {@link ShaderProgram}s references are nullified but not {@link ShaderProgram#destroy(GL2ES2) destroyed}

+     * as they are owned by {@link RegionRenderer}.

+     * </p>

      */

     public final void destroy(final GL2ES2 gl) {

-        clearShaderImpl(gl);

         clear(gl);

         destroyImpl(gl);

     }

diff --git a/src/jogl/classes/com/jogamp/graph/curve/opengl/RegionRenderer.java b/src/jogl/classes/com/jogamp/graph/curve/opengl/RegionRenderer.java
index 9fef381f4..54deb9e4b 100644
--- a/src/jogl/classes/com/jogamp/graph/curve/opengl/RegionRenderer.java
+++ b/src/jogl/classes/com/jogamp/graph/curve/opengl/RegionRenderer.java
@@ -50,10 +50,15 @@ import com.jogamp.graph.curve.Region;
 
 /**
  * OpenGL {@link Region} renderer
- * <p>
- * All OpenGL related operations regarding {@link Region}s
- * are passed through an instance of this class.
- * </p>
+ *
+ * All {@link Region} rendering operations utilize a RegionRenderer.
+ *
+ * The RegionRenderer owns its {@link RenderState}, a composition.
+ *
+ * The RegionRenderer manages and own all used {@link ShaderProgram}s, a composition.
+ *
+ * At its {@link #destroy(GL2ES2) destruction}, all {@link ShaderProgram}s and its {@link RenderState}
+ * will be destroyed and released.
  */
 public class RegionRenderer {
     protected static final boolean DEBUG = Region.DEBUG;
@@ -223,9 +228,12 @@ public class RegionRenderer {
         initialized = true;
     }
 
-    /** Deletes all {@link ShaderProgram}s and nullifies its references. */
-    public final void clearShader(final GL2ES2 gl) {
+    /** Deletes all {@link ShaderProgram}s and nullifies its references including {@link RenderState#destroy(GL2ES2)}. */
+    public final void destroy(final GL2ES2 gl) {
         if(!initialized){
+            if(DEBUG_INSTANCE) {
+                System.err.println("TextRenderer: Not initialized!");
+            }
             return;
         }
         for(final Iterator<IntObjectHashMap.Entry> i = shaderPrograms.iterator(); i.hasNext(); ) {
@@ -233,17 +241,7 @@ public class RegionRenderer {
             sp.destroy(gl);
         }
         shaderPrograms.clear();
-        rs.destroy(gl);
-    }
-
-    public final void destroy(final GL2ES2 gl) {
-        if(!initialized){
-            if(DEBUG_INSTANCE) {
-                System.err.println("TextRenderer: Not initialized!");
-            }
-            return;
-        }
-        clearShader(gl);
+        rs.destroy();
         initialized = false;
     }
 
diff --git a/src/jogl/classes/com/jogamp/graph/curve/opengl/RenderState.java b/src/jogl/classes/com/jogamp/graph/curve/opengl/RenderState.java
index 407f40b68..3f7e3f81d 100644
--- a/src/jogl/classes/com/jogamp/graph/curve/opengl/RenderState.java
+++ b/src/jogl/classes/com/jogamp/graph/curve/opengl/RenderState.java
@@ -199,14 +199,20 @@ public class RenderState {
     }
 
     public final int id() { return id; }
+
+    /** Return the current {@link ShaderProgram} */
     public final ShaderProgram getShaderProgram() { return sp; }
+
+    /** Return whether the current {@link ShaderProgram} is {@link ShaderProgram#inUse() in use}. */
     public final boolean isShaderProgramInUse() { return null != sp ? sp.inUse() : false; }
 
     /**
-     * Set a {@link ShaderProgram} and enable it. If the given {@link ShaderProgram} is new,
-     * method returns true, otherwise false.
+     * Sets the current {@link ShaderProgram} and enables it.
+     *
+     * If the given {@link ShaderProgram} is not {@link #getShaderProgram() the current}, method returns true, otherwise false.
+     *
      * @param gl
-     * @param spNext
+     * @param spNext the next current {@link ShaderProgram} to be set and enabled
      * @return true if a new shader program is being used and hence external uniform-data and -location,
      *         as well as the attribute-location must be updated, otherwise false.
      */
@@ -263,7 +269,8 @@ public class RenderState {
         if( updateLocation || 0 > data.getLocation() ) {
             final boolean ok = 0 <= data.setLocation(gl, sp.program());
             if( throwOnError && !ok ) {
-                throw new GLException("Could not locate "+data);
+                sp.dumpSource(System.err);
+                throw new GLException("Could not locate "+data.getName()+" in "+sp+", "+data);
             }
             return ok;
         } else {
@@ -285,7 +292,8 @@ public class RenderState {
         if( updateLocation ) {
             updateData = 0 <= data.setLocation(gl, sp.program());
             if( throwOnError && !updateData ) {
-                throw new GLException("Could not locate "+data);
+                sp.dumpSource(System.err);
+                throw new GLException("Could not locate "+data.getName()+" in "+sp+", "+data);
             }
         }
         if( updateData ){
@@ -306,7 +314,8 @@ public class RenderState {
         if( updateLocation || 0 > data.getLocation() ) {
             final boolean ok = 0 <= data.setLocation(gl, sp.program());
             if( throwOnError && !ok ) {
-                throw new GLException("Could not locate "+data);
+                sp.dumpSource(System.err);
+                throw new GLException("Could not locate "+data.getName()+" in "+sp+", "+data);
             }
             return ok;
         } else {
@@ -326,13 +335,10 @@ public class RenderState {
     }
 
     /**
-     * Issues {@link ShaderProgram#destroy(GL2ES2)} and nullifies reference.
+     * Only nullifies {@link ShaderProgram} reference owned by {@link RegionRenderer}.
      */
     public void destroy(final GL2ES2 gl) {
-        if( null != sp ) {
-            sp.destroy(gl);
-            sp = null;
-        }
+        sp = null; // owned by RegionRenderer
     }
 
     public final RenderState attachTo(final GL2ES2 gl) {
diff --git a/src/jogl/classes/jogamp/graph/curve/opengl/VBORegion2PMSAAES2.java b/src/jogl/classes/jogamp/graph/curve/opengl/VBORegion2PMSAAES2.java
index c09e75b53..bc87b3faa 100644
--- a/src/jogl/classes/jogamp/graph/curve/opengl/VBORegion2PMSAAES2.java
+++ b/src/jogl/classes/jogamp/graph/curve/opengl/VBORegion2PMSAAES2.java
@@ -40,6 +40,7 @@ import com.jogamp.opengl.GLUniformData;
 import jogamp.graph.curve.opengl.shader.AttributeNames;
 import jogamp.graph.curve.opengl.shader.UniformNames;
 
+import com.jogamp.graph.curve.Region;
 import com.jogamp.graph.curve.opengl.GLRegion;
 import com.jogamp.graph.curve.opengl.RegionRenderer;
 import com.jogamp.graph.curve.opengl.RenderState;
@@ -322,7 +323,7 @@ public final class VBORegion2PMSAAES2  extends GLRegion {
     /**
      * <p>
      * Since multiple {@link Region}s may share one
-     * {@link ShaderProgram}, the uniform data must always be updated.
+     * {@link ShaderProgram} managed and owned by {@link RegionRendered}, the uniform data must always be updated.
      * </p>
      *
      * @param gl
@@ -629,18 +630,6 @@ public final class VBORegion2PMSAAES2  extends GLRegion {
     }
 
     @Override
-    protected void clearShaderImpl(final GL2ES2 gl) {
-        if( null != spPass1 ) {
-            spPass1.destroy(gl);
-            spPass1 = null;
-        }
-        if( null != spPass2 ) {
-            spPass2.destroy(gl);
-            spPass2 = null;
-        }
-    }
-
-    @Override
     protected void destroyImpl(final GL2ES2 gl) {
         if(DEBUG_INSTANCE) {
             System.err.println("VBORegion2PES2 Destroy: " + this);
@@ -677,5 +666,7 @@ public final class VBORegion2PMSAAES2  extends GLRegion {
             indicesFbo.destroy(gl);
             indicesFbo = null;
         }
+        spPass1 = null; // owned by RegionRenderer
+        spPass2 = null; // owned by RegionRenderer
     }
 }
diff --git a/src/jogl/classes/jogamp/graph/curve/opengl/VBORegion2PVBAAES2.java b/src/jogl/classes/jogamp/graph/curve/opengl/VBORegion2PVBAAES2.java
index 96247c61a..21f454655 100644
--- a/src/jogl/classes/jogamp/graph/curve/opengl/VBORegion2PVBAAES2.java
+++ b/src/jogl/classes/jogamp/graph/curve/opengl/VBORegion2PVBAAES2.java
@@ -128,7 +128,7 @@ public final class VBORegion2PVBAAES2  extends GLRegion {
     /**
      * <p>
      * Since multiple {@link Region}s may share one
-     * {@link ShaderProgram}, the uniform data must always be updated.
+     * {@link ShaderProgram} managed and owned by {@link RegionRendered}, the uniform data must always be updated.
      * </p>
      *
      * @param gl
@@ -762,18 +762,6 @@ public final class VBORegion2PVBAAES2  extends GLRegion {
     }
 
     @Override
-    protected void clearShaderImpl(final GL2ES2 gl) {
-        if( null != spPass1 ) {
-            spPass1.destroy(gl);
-            spPass1 = null;
-        }
-        if( null != spPass2 ) {
-            spPass2.destroy(gl);
-            spPass2 = null;
-        }
-    }
-
-    @Override
     protected void destroyImpl(final GL2ES2 gl) {
         if(DEBUG_INSTANCE) {
             System.err.println("VBORegion2PES2 Destroy: " + this);
@@ -813,5 +801,7 @@ public final class VBORegion2PVBAAES2  extends GLRegion {
             indicesFbo.destroy(gl);
             indicesFbo = null;
         }
+        spPass1 = null; // owned by RegionRenderer
+        spPass2 = null; // owned by RegionRenderer
     }
 }
diff --git a/src/jogl/classes/jogamp/graph/curve/opengl/VBORegionSPES2.java b/src/jogl/classes/jogamp/graph/curve/opengl/VBORegionSPES2.java
index b8bdfbe37..0077186e6 100644
--- a/src/jogl/classes/jogamp/graph/curve/opengl/VBORegionSPES2.java
+++ b/src/jogl/classes/jogamp/graph/curve/opengl/VBORegionSPES2.java
@@ -265,7 +265,7 @@ public final class VBORegionSPES2 extends GLRegion {
     /**
      * <p>
      * Since multiple {@link Region}s may share one
-     * {@link ShaderProgram}, the uniform data must always be updated.
+     * {@link ShaderProgram} managed and owned by {@link RegionRendered}, the uniform data must always be updated.
      * </p>
      *
      * @param gl
@@ -345,14 +345,6 @@ public final class VBORegionSPES2 extends GLRegion {
     }
 
     @Override
-    protected void clearShaderImpl(final GL2ES2 gl) {
-        if( null != spPass1 ) {
-            spPass1.destroy(gl);
-            spPass1 = null;
-        }
-    }
-
-    @Override
     protected void destroyImpl(final GL2ES2 gl) {
         if(DEBUG_INSTANCE) {
             System.err.println("VBORegionSPES2 Destroy: " + this);
@@ -373,5 +365,6 @@ public final class VBORegionSPES2 extends GLRegion {
             indicesBuffer.destroy(gl);
             indicesBuffer = null;
         }
+        spPass1 = null; // owned by RegionRenderer
     }
 }