Browse Source

test and fix for issue #37 (warn when >16 textures are used) (#1574)

* testcase for issue #37 (warn when more than 16 textures are used)

* address issue #37 (warn when more than 16 textures are used)
Stephen Gold 4 years ago
parent
commit
5c4347bfa2

+ 17 - 2
jme3-core/src/main/java/com/jme3/material/Material.java

@@ -43,6 +43,7 @@ import com.jme3.math.*;
 import com.jme3.renderer.Caps;
 import com.jme3.renderer.Caps;
 import com.jme3.renderer.RenderManager;
 import com.jme3.renderer.RenderManager;
 import com.jme3.renderer.Renderer;
 import com.jme3.renderer.Renderer;
+import com.jme3.renderer.TextureUnitException;
 import com.jme3.renderer.queue.RenderQueue.Bucket;
 import com.jme3.renderer.queue.RenderQueue.Bucket;
 import com.jme3.scene.Geometry;
 import com.jme3.scene.Geometry;
 import com.jme3.shader.*;
 import com.jme3.shader.*;
@@ -806,7 +807,14 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
 
 
             if (override.getValue() != null) {
             if (override.getValue() != null) {
                 if (type.isTextureType()) {
                 if (type.isTextureType()) {
-                    renderer.setTexture(unit, (Texture) override.getValue());
+                    try {
+                        renderer.setTexture(unit, (Texture) override.getValue());
+                    } catch (TextureUnitException exception) {
+                        int numTexParams = unit + 1;
+                        String message = "Too many texture parameters ("
+                                + numTexParams + ") assigned\n to " + toString();
+                        throw new IllegalStateException(message);
+                    }
                     uniform.setValue(VarType.Int, unit);
                     uniform.setValue(VarType.Int, unit);
                     unit++;
                     unit++;
                 } else {
                 } else {
@@ -848,7 +856,14 @@ public class Material implements CloneableSmartAsset, Cloneable, Savable {
                 }
                 }
 
 
                 if (type.isTextureType()) {
                 if (type.isTextureType()) {
-                    renderer.setTexture(unit, (Texture) param.getValue());
+                    try {
+                        renderer.setTexture(unit, (Texture) param.getValue());
+                    } catch (TextureUnitException exception) {
+                        int numTexParams = unit + 1;
+                        String message = "Too many texture parameters ("
+                                + numTexParams + ") assigned\n to " + toString();
+                        throw new IllegalStateException(message);
+                    }
                     uniform.setValue(VarType.Int, unit);
                     uniform.setValue(VarType.Int, unit);
                     unit++;
                     unit++;
                 } else {
                 } else {

+ 13 - 2
jme3-core/src/main/java/com/jme3/material/logic/SinglePassAndImageBasedLightingLogic.java

@@ -39,6 +39,7 @@ import com.jme3.math.*;
 import com.jme3.renderer.*;
 import com.jme3.renderer.*;
 import com.jme3.scene.Geometry;
 import com.jme3.scene.Geometry;
 import com.jme3.shader.*;
 import com.jme3.shader.*;
+import com.jme3.texture.TextureCubeMap;
 import com.jme3.util.TempVars;
 import com.jme3.util.TempVars;
 
 
 import java.util.*;
 import java.util.*;
@@ -245,9 +246,19 @@ public final class SinglePassAndImageBasedLightingLogic extends DefaultTechnique
         lightProbeData.setValue(VarType.Matrix4, lightProbe.getUniformMatrix());
         lightProbeData.setValue(VarType.Matrix4, lightProbe.getUniformMatrix());
                 //setVector4InArray(lightProbe.getPosition().x, lightProbe.getPosition().y, lightProbe.getPosition().z, 1f / area.getRadius() + lightProbe.getNbMipMaps(), 0);
                 //setVector4InArray(lightProbe.getPosition().x, lightProbe.getPosition().y, lightProbe.getPosition().z, 1f / area.getRadius() + lightProbe.getNbMipMaps(), 0);
         shCoeffs.setValue(VarType.Vector3Array, lightProbe.getShCoeffs());
         shCoeffs.setValue(VarType.Vector3Array, lightProbe.getShCoeffs());
-        //assigning new texture indexes
+        /*
+         * Assign the prefiltered env map to the next available texture unit.
+         */
         int pemUnit = lastTexUnit++;
         int pemUnit = lastTexUnit++;
-        rm.getRenderer().setTexture(pemUnit, lightProbe.getPrefilteredEnvMap());
+        Renderer renderer = rm.getRenderer();
+        TextureCubeMap pemTexture = lightProbe.getPrefilteredEnvMap();
+        try {
+            renderer.setTexture(pemUnit, pemTexture);
+        } catch (TextureUnitException exception) {
+            String message = "Can't assign texture unit for LightProbe."
+                    + " lastTexUnit=" + lastTexUnit;
+            throw new IllegalArgumentException(message);
+        }
         lightProbePemMap.setValue(VarType.Int, pemUnit);
         lightProbePemMap.setValue(VarType.Int, pemUnit);
         return lastTexUnit;
         return lastTexUnit;
     }
     }

+ 6 - 1
jme3-core/src/main/java/com/jme3/renderer/RenderContext.java

@@ -45,6 +45,10 @@ import com.jme3.texture.Image;
  * internally to reduce state changes. NOTE: This class is specific to OpenGL.
  * internally to reduce state changes. NOTE: This class is specific to OpenGL.
  */
  */
 public class RenderContext {
 public class RenderContext {
+    /**
+     * number of texture units that JME supports
+     */
+    final public static int maxTextureUnits = 16;
 
 
     /**
     /**
      * @see RenderState#setFaceCullMode(com.jme3.material.RenderState.FaceCullMode)
      * @see RenderState#setFaceCullMode(com.jme3.material.RenderState.FaceCullMode)
@@ -217,7 +221,8 @@ public class RenderContext {
      *
      *
      * @see Renderer#setTexture(int, com.jme3.texture.Texture)
      * @see Renderer#setTexture(int, com.jme3.texture.Texture)
      */
      */
-    public final WeakReference<Image> boundTextures[] = new WeakReference[16];
+    public final WeakReference<Image> boundTextures[]
+            = new WeakReference[maxTextureUnits];
 
 
     /**
     /**
      * IDList for texture units
      * IDList for texture units

+ 5 - 4
jme3-core/src/main/java/com/jme3/renderer/Renderer.java

@@ -258,12 +258,13 @@ public interface Renderer {
     public void deleteFrameBuffer(FrameBuffer fb);
     public void deleteFrameBuffer(FrameBuffer fb);
 
 
     /**
     /**
-     * Sets the texture to use for the given texture unit.
+     * Assign a Texture to the specified texture unit.
      *
      *
-     * @param unit which unit
-     * @param tex the Texture to use
+     * @param unit the index of the texture unit (&ge;0)
+     * @param tex the Texture to assign
      */
      */
-    public void setTexture(int unit, Texture tex);
+    public void setTexture(int unit, Texture tex)
+            throws TextureUnitException;
 
 
     /**
     /**
      * Modify the given Texture with the given Image.
      * Modify the given Texture with the given Image.

+ 39 - 0
jme3-core/src/main/java/com/jme3/renderer/TextureUnitException.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2021 jMonkeyEngine
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in the
+ *   documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of 'jMonkeyEngine' nor the names of its contributors
+ *   may be used to endorse or promote products derived from this software
+ *   without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package com.jme3.renderer;
+
+/**
+ * A checked exception, to be thrown (in place of an IndexOutOfBoundsException)
+ * when a non-existent texture unit is assigned.
+ */
+public class TextureUnitException extends Exception {
+}

+ 25 - 4
jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java

@@ -2057,7 +2057,12 @@ public final class GLRenderer implements Renderer {
                 RenderBuffer rb = context.boundFB.getColorBuffer(i);
                 RenderBuffer rb = context.boundFB.getColorBuffer(i);
                 Texture tex = rb.getTexture();
                 Texture tex = rb.getTexture();
                 if (tex != null && tex.getMinFilter().usesMipMapLevels()) {
                 if (tex != null && tex.getMinFilter().usesMipMapLevels()) {
-                    setTexture(0, rb.getTexture());
+                    try {
+                        final int textureUnitIndex = 0;
+                        setTexture(textureUnitIndex, rb.getTexture());
+                    } catch (TextureUnitException exception) {
+                        throw new RuntimeException("Renderer lacks texture units?");
+                    }
                     if (tex.getType() == Texture.Type.CubeMap) {
                     if (tex.getType() == Texture.Type.CubeMap) {
                         glfbo.glGenerateMipmapEXT(GL.GL_TEXTURE_CUBE_MAP);
                         glfbo.glGenerateMipmapEXT(GL.GL_TEXTURE_CUBE_MAP);
                     } else {
                     } else {
@@ -2583,7 +2588,11 @@ public final class GLRenderer implements Renderer {
     }
     }
 
 
     @Override
     @Override
-    public void setTexture(int unit, Texture tex) {
+    public void setTexture(int unit, Texture tex) throws TextureUnitException {
+        if (unit < 0 || unit >= RenderContext.maxTextureUnits) {
+            throw new TextureUnitException();
+        }
+
         Image image = tex.getImage();
         Image image = tex.getImage();
         if (image.isUpdateNeeded() || (image.isGeneratedMipmapsRequired() && !image.isMipmapsGenerated())) {
         if (image.isUpdateNeeded() || (image.isGeneratedMipmapsRequired() && !image.isMipmapsGenerated())) {
             // Check NPOT requirements
             // Check NPOT requirements
@@ -2619,7 +2628,13 @@ public final class GLRenderer implements Renderer {
     @Deprecated
     @Deprecated
     @Override
     @Override
     public void modifyTexture(Texture tex, Image pixels, int x, int y) {
     public void modifyTexture(Texture tex, Image pixels, int x, int y) {
-        setTexture(0, tex);
+        final int textureUnitIndex = 0;
+        try {
+            setTexture(textureUnitIndex, tex);
+        } catch (TextureUnitException exception) {
+            throw new RuntimeException("Renderer lacks texture units?");
+        }
+
         if(caps.contains(Caps.OpenGLES20) && pixels.getFormat()!=tex.getImage().getFormat() ) {
         if(caps.contains(Caps.OpenGLES20) && pixels.getFormat()!=tex.getImage().getFormat() ) {
             logger.log(Level.WARNING, "Incompatible texture subimage");
             logger.log(Level.WARNING, "Incompatible texture subimage");
         }
         }
@@ -2639,7 +2654,13 @@ public final class GLRenderer implements Renderer {
      * @param areaH Height of the area to copy
      * @param areaH Height of the area to copy
      */
      */
     public void modifyTexture(Texture2D dest, Image src, int destX, int destY, int srcX, int srcY, int areaW, int areaH) {
     public void modifyTexture(Texture2D dest, Image src, int destX, int destY, int srcX, int srcY, int areaW, int areaH) {
-        setTexture(0, dest);
+        final int textureUnitIndex = 0;
+        try {
+            setTexture(textureUnitIndex, dest);
+        } catch (TextureUnitException exception) {
+            throw new RuntimeException("Renderer lacks texture units?");
+        }
+
         if(caps.contains(Caps.OpenGLES20) && src.getFormat()!=dest.getImage().getFormat() ) {
         if(caps.contains(Caps.OpenGLES20) && src.getFormat()!=dest.getImage().getFormat() ) {
             logger.log(Level.WARNING, "Incompatible texture subimage");
             logger.log(Level.WARNING, "Incompatible texture subimage");
         }
         }

+ 3 - 1
jme3-core/src/main/java/com/jme3/system/NullRenderer.java

@@ -39,6 +39,7 @@ import com.jme3.renderer.Caps;
 import com.jme3.renderer.Limits;
 import com.jme3.renderer.Limits;
 import com.jme3.renderer.Renderer;
 import com.jme3.renderer.Renderer;
 import com.jme3.renderer.Statistics;
 import com.jme3.renderer.Statistics;
+import com.jme3.renderer.TextureUnitException;
 import com.jme3.scene.Mesh;
 import com.jme3.scene.Mesh;
 import com.jme3.scene.VertexBuffer;
 import com.jme3.scene.VertexBuffer;
 import com.jme3.shader.BufferObject;
 import com.jme3.shader.BufferObject;
@@ -166,7 +167,8 @@ public class NullRenderer implements Renderer {
     }
     }
 
 
     @Override
     @Override
-    public void setTexture(int unit, Texture tex) {
+    public void setTexture(int unit, Texture tex) throws TextureUnitException {
+        // do nothing
     }
     }
 
 
     @Override
     @Override

+ 110 - 0
jme3-examples/src/main/java/jme3test/renderer/TestIssue37.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2021 jMonkeyEngine
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in the
+ *   documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of 'jMonkeyEngine' nor the names of its contributors
+ *   may be used to endorse or promote products derived from this software
+ *   without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package jme3test.renderer;
+
+import com.jme3.app.Application;
+import com.jme3.app.SimpleApplication;
+import com.jme3.material.MatParamOverride;
+import com.jme3.material.Material;
+import com.jme3.scene.Geometry;
+import com.jme3.scene.Mesh;
+import com.jme3.scene.shape.Box;
+import com.jme3.shader.VarType;
+import com.jme3.texture.Texture;
+
+/**
+ * Test a Material with increasing numbers of texture parameters, to see what
+ * happens when the renderer's dynamic limit is exceeded.
+ *
+ * If successful, this test throws an IllegalStateException with a helpful
+ * diagnostic message.
+ */
+public class TestIssue37 extends SimpleApplication {
+
+    /**
+     * Edit this field to change how parameters are assigned (which determines
+     * where the exception is caught): true to use mat param overrides, false to
+     * use ordinary mat params.
+     */
+    final private boolean useOverrides = true;
+
+    private int numTextures;
+    private Material manyTexturesMaterial;
+    private Texture testTexture;
+
+    public static void main(String[] args) {
+        Application application = new TestIssue37();
+        application.start();
+    }
+
+    @Override
+    public void simpleInitApp() {
+        /*
+         * Attach a test geometry to the scene.
+         */
+        Mesh cubeMesh = new Box(1f, 1f, 1f);
+        Geometry cubeGeometry = new Geometry("Box", cubeMesh);
+        rootNode.attachChild(cubeGeometry);
+        /*
+         * Apply a test material (with no textures assigned) to the geometry.
+         */
+        manyTexturesMaterial = new Material(assetManager,
+                "jme3test/materials/TestIssue37.j3md");
+        manyTexturesMaterial.setName("manyTexturesMaterial");
+        cubeGeometry.setMaterial(manyTexturesMaterial);
+        numTextures = 0;
+        /*
+         * Load the test texture.
+         */
+        String texturePath = "Interface/Logo/Monkey.jpg";
+        testTexture = assetManager.loadTexture(texturePath);
+    }
+
+    /**
+     * During each update, define another texture parameter until the dynamic
+     * limit is reached.
+     *
+     * @param tpf ignored
+     */
+    @Override
+    public void simpleUpdate(float tpf) {
+        String parameterName = "ColorMap" + numTextures;
+        if (useOverrides) {
+            MatParamOverride override = new MatParamOverride(VarType.Texture2D,
+                    parameterName, testTexture);
+            rootNode.addMatParamOverride(override);
+        } else {
+            manyTexturesMaterial.setTexture(parameterName, testTexture);
+        }
+        ++numTextures;
+    }
+}

+ 41 - 0
jme3-examples/src/main/resources/jme3test/materials/TestIssue37.j3md

@@ -0,0 +1,41 @@
+MaterialDef TestIssue37 {
+    MaterialParameters {
+        // For instancing
+        Boolean UseInstancing
+
+        Texture2D ColorMap0
+        Texture2D ColorMap1
+        Texture2D ColorMap2
+        Texture2D ColorMap3
+        Texture2D ColorMap4
+        Texture2D ColorMap5
+        Texture2D ColorMap6
+        Texture2D ColorMap7
+        Texture2D ColorMap8
+        Texture2D ColorMap9
+        Texture2D ColorMap10
+        Texture2D ColorMap11
+        Texture2D ColorMap12
+        Texture2D ColorMap13
+        Texture2D ColorMap14
+        Texture2D ColorMap15
+        Texture2D ColorMap16
+        Texture2D ColorMap17
+    }
+
+    Technique {
+        VertexShader GLSL100 GLSL150:   Common/MatDefs/Misc/ShowNormals.vert
+        FragmentShader GLSL100 GLSL150: Common/MatDefs/Misc/ShowNormals.frag
+
+        WorldParameters {
+            WorldViewProjectionMatrix
+            ViewProjectionMatrix
+            ViewMatrix
+            ProjectionMatrix
+        }
+
+        Defines {
+            INSTANCING : UseInstancing
+        }
+    }
+}