fixes and cleanup

riccardobl · riccardobl · commit ed940433b6ef · 2026-05-01T15:24:05.000+02:00
diff --git a/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java b/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java
@@ -2810,62 +2810,79 @@ private void updateTexImageData(Image img, Texture.Type type, int unit, boolean
         bindTextureAndUnit(target, img, unit);
 
         int imageSamples = img.getMultiSamples();
-        boolean needsMipmaps = !img.hasMipmaps() && img.isGeneratedMipmapsRequired();
+        boolean sourceMipmapsUsable = img.hasMipmaps() && !scaleToPot;
+        boolean needsMipmaps = !sourceMipmapsUsable && img.isGeneratedMipmapsRequired();
         boolean hwMipmapSupported = needsMipmaps && isMipmapGenerationSupported(img.getFormat(),
                 linearizeSrgbImages ? img.getColorSpace() : ColorSpace.Linear);
-        Image cpuMipmapImage = null;
+        Image imageForUpload = img;
+        boolean cpuMipmapsGenerated = false;
         if (imageSamples <= 1) {
             boolean cpuMipmapFallbackFailed = false;
-            if (needsMipmaps && !hwMipmapSupported
-                    && allowCpuMipmapFallback
-                    && textureWasUnuploaded
-                    && !scaleToPot
-                    && MipMapGenerator.canGenerateMipmaps(img)) {
-                try {
-                    cpuMipmapImage = createCpuMipmapImage(img);
-                    MipMapGenerator.generateMipMaps(cpuMipmapImage, linearizeSrgbImages,
-                            img.getColorSpace() == ColorSpace.sRGB);
-                    img.setMipmapsGenerated(true);
-                } catch (RuntimeException exception) {
-                    cpuMipmapImage = null;
-                    cpuMipmapFallbackFailed = true;
-                    logger.log(Level.WARNING,
-                            "Texture " + img + " requires mipmaps, but hardware mipmap generation is not supported"
-                                    + " and CPU mipmap generation failed. Mipmaps will not be generated.",
-                            exception);
+            if (needsMipmaps) {
+                /*
+                 * Some formats cannot use glGenerateMipmap because they are not both
+                 * renderable and filterable. On a first upload, fall back to a CPU-built
+                 * mip chain when the image data is suitable. If NPOT scaling is also
+                 * required, build the CPU mips from the resized upload image.
+                 */
+                boolean needsCpuMipmapFallback = !hwMipmapSupported
+                        && allowCpuMipmapFallback
+                        && textureWasUnuploaded
+                        && MipMapGenerator.canGenerateMipmaps(img);
+                if (needsCpuMipmapFallback) {
+                    try {
+                        Image cpuMipmapUploadImage = cloneImageForUpload(img, scaleToPot);
+                        if (cpuMipmapUploadImage != null) {
+                            MipMapGenerator.generateMipMaps(cpuMipmapUploadImage, linearizeSrgbImages,
+                                    img.getColorSpace() == ColorSpace.sRGB);
+                            imageForUpload = cpuMipmapUploadImage;
+                            cpuMipmapsGenerated = true;
+                            scaleToPot = false;
+                            img.setMipmapsGenerated(true);
+                        }
+                    } catch (RuntimeException exception) {
+                        cpuMipmapFallbackFailed = true;
+                        logger.log(Level.WARNING,
+                                "Texture " + img + " requires mipmaps, but hardware mipmap generation is not supported"
+                                        + " and CPU mipmap generation failed. Mipmaps will not be generated.",
+                                exception);
+                    }
                 }
-            }
-
-            if (needsMipmaps && hwMipmapSupported) {
-                // Image does not have mipmaps, but they are required.
-                // Generate from base level.
 
-                if (!caps.contains(Caps.FrameBuffer) && gl2 != null) {
+                /*
+                 * Old desktop GL without FBO support can auto-generate mipmaps during
+                 * texture upload. Newer paths generate explicitly after upload below.
+                 */
+                if (hwMipmapSupported && !caps.contains(Caps.FrameBuffer) && gl2 != null) {
                     gl2.glTexParameteri(target, GL2.GL_GENERATE_MIPMAP, GL.GL_TRUE);
                     img.setMipmapsGenerated(true);
-                } else {
-                    // For OpenGL3 and up.
-                    // We'll generate mipmaps via glGenerateMipmapEXT (see below)
                 }
-            } else if (caps.contains(Caps.OpenGL20) || caps.contains(Caps.OpenGLES30)) {
-                if (img.hasMipmaps() || cpuMipmapImage != null) {
-                    int mipCount = cpuMipmapImage != null
-                            ? cpuMipmapImage.getMipMapSizes().length
-                            : img.getMipMapSizes().length;
-                    // Image already has mipmaps, set the max level based on the
-                    // number of mipmaps we have.
-                    gl.glTexParameteri(target, GL2.GL_TEXTURE_MAX_LEVEL, mipCount - 1);
-                } else {
-                    // Image does not have mipmaps, and they are not required.
-                    // Specify that the texture has no mipmaps.
-                    gl.glTexParameteri(target, GL2.GL_TEXTURE_MAX_LEVEL, 0);
-                }
-            }
-            if (needsMipmaps && !hwMipmapSupported) {
-                if (!img.hasMipmaps() && cpuMipmapImage == null && !cpuMipmapFallbackFailed) {
+
+                if (!hwMipmapSupported
+                        && !sourceMipmapsUsable
+                        && !cpuMipmapsGenerated
+                        && !cpuMipmapFallbackFailed) {
                     logger.log(Level.WARNING, "Texture " + img + " requires mipmaps, but hardware mipmaps generation is not supported. Mipmaps will not be generated.");
                 }
             }
+
+            /*
+             * Clamp the mip range to the levels actually uploaded. This is still
+             * needed when mipmaps are not requested, otherwise GL may sample
+             * missing levels left from a previous texture state. When hardware
+             * mipmap generation is pending, reopen the full generated range in
+             * case an earlier upload clamped this texture to the base level.
+             */
+            boolean canSetTextureMaxLevel = caps.contains(Caps.OpenGL20) || caps.contains(Caps.OpenGLES30);
+            boolean hasUploadMipmaps = sourceMipmapsUsable || cpuMipmapsGenerated;
+            int uploadWidth = scaleToPot ? FastMath.nearestPowerOfTwo(img.getWidth()) : imageForUpload.getWidth();
+            int uploadHeight = scaleToPot ? FastMath.nearestPowerOfTwo(img.getHeight()) : imageForUpload.getHeight();
+            int maxLevel = textureMaxLevelForUpload(canSetTextureMaxLevel, needsMipmaps, hwMipmapSupported,
+                    hasUploadMipmaps, cpuMipmapsGenerated ? imageForUpload.getMipMapSizes() : img.getMipMapSizes(),
+                    generatedMipMaxLevel(uploadWidth, uploadHeight, imageForUpload.getDepth()));
+            if (maxLevel >= 0) {
+                gl.glTexParameteri(target, GL2.GL_TEXTURE_MAX_LEVEL, maxLevel);
+            }
         } else {
             // Check if graphics card doesn't support multisample textures
             if (!caps.contains(Caps.TextureMultisample)) {
@@ -2906,13 +2923,8 @@ private void updateTexImageData(Image img, Texture.Type type, int unit, boolean
             }
         }
 
-        Image imageForUpload;
-        if (cpuMipmapImage != null) {
-            imageForUpload = cpuMipmapImage;
-        } else if (scaleToPot) {
+        if (scaleToPot) {
             imageForUpload = MipMapGenerator.resizeToPowerOf2(img);
-        } else {
-            imageForUpload = img;
         }
         if (target == GL.GL_TEXTURE_CUBE_MAP) {
             List<ByteBuffer> data = imageForUpload.getData();
@@ -2947,14 +2959,12 @@ private void updateTexImageData(Image img, Texture.Type type, int unit, boolean
             img.setMultiSamples(imageSamples);
         }
 
-        if (caps.contains(Caps.FrameBuffer) || gl2 == null) {
-            if (needsMipmaps && img.getData(0) != null
-                    && !img.isMipmapsGenerated()) {
-                if (hwMipmapSupported) {
-                    glfbo.glGenerateMipmapEXT(target);
-                    img.setMipmapsGenerated(true);
-                }
-            }
+        if (needsMipmaps && hwMipmapSupported
+                && (caps.contains(Caps.FrameBuffer) || gl2 == null)
+                && img.getData(0) != null
+                && !img.isMipmapsGenerated()) {
+            glfbo.glGenerateMipmapEXT(target);
+            img.setMipmapsGenerated(true);
         }
 
         img.clearUpdateNeeded();
@@ -2965,9 +2975,44 @@ private boolean isMipmapGenerationSupported(Image.Format format, ColorSpace colo
         return gf != null && gf.colorRenderable && gf.filterable;
     }
 
-    private Image createCpuMipmapImage(Image image) {
+    static int textureMaxLevelForUpload(boolean canSetTextureMaxLevel,
+                                        boolean needsMipmaps,
+                                        boolean hwMipmapSupported,
+                                        boolean hasUploadMipmaps,
+                                        int[] uploadMipMapSizes,
+                                        int generatedMipMaxLevel) {
+        if (!canSetTextureMaxLevel) {
+            return -1;
+        }
+        if (needsMipmaps && hwMipmapSupported) {
+            return generatedMipMaxLevel;
+        }
+        if (!hasUploadMipmaps) {
+            return 0;
+        }
+        return uploadMipMapSizes.length - 1;
+    }
+
+    static int generatedMipMaxLevel(int width, int height, int depth) {
+        int maxDimension = Math.max(Math.max(width, height), Math.max(1, depth));
+        int maxLevel = 0;
+        while (maxDimension > 1) {
+            maxDimension >>= 1;
+            maxLevel++;
+        }
+        return maxLevel;
+    }
+
+    private Image cloneImageForUpload(Image image, boolean scaleToPot) {
+        if (scaleToPot) {
+            return MipMapGenerator.resizeToPowerOf2(image);
+        }
+
         ArrayList<ByteBuffer> data = new ArrayList<>(image.getData().size());
         for (ByteBuffer buffer : image.getData()) {
+            if (buffer == null) {
+                return null;
+            }
             data.add(buffer.duplicate());
         }
         return new Image(image.getFormat(), image.getWidth(), image.getHeight(), image.getDepth(),
diff --git a/jme3-core/src/main/java/com/jme3/util/MipMapGenerator.java b/jme3-core/src/main/java/com/jme3/util/MipMapGenerator.java
@@ -452,6 +452,22 @@ private static void validateImage(Image image) {
                             + " bits per pixel"
             );
         }
+
+        int baseLevelSize = levelSize(image.getFormat(), image.getWidth(), image.getHeight());
+        for (int dataIndex = 0; dataIndex < image.getData().size(); dataIndex++) {
+            ByteBuffer data = image.getData(dataIndex);
+            if (data == null) {
+                throw new IllegalArgumentException("Image data buffer " + dataIndex + " is null");
+            }
+            if (data.capacity() < baseLevelSize) {
+                throw new IllegalArgumentException(
+                        "Image data buffer " + dataIndex + " is smaller than expected base level size. Data capacity="
+                                + data.capacity()
+                                + ", expected="
+                                + baseLevelSize
+                );
+            }
+        }
     }
 
     private static int levelSize(Image.Format format, int width, int height) {
@@ -491,6 +507,9 @@ private static int levelSize(Image.Format format, int width, int height) {
      * For rebuilding mipmaps, we only want the base level.
      */
     private static ByteBuffer copyBaseLevel(ByteBuffer source, int baseLevelSize) {
+        if (source == null) {
+            throw new IllegalArgumentException("Image data buffer is null");
+        }
         if (source.capacity() < baseLevelSize) {
             throw new IllegalArgumentException(
                     "Image data is smaller than expected base level size. Data capacity="
diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GLRendererMipmapPolicyTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GLRendererMipmapPolicyTest.java
@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2009-2026 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.opengl;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class GLRendererMipmapPolicyTest {
+
+    @Test
+    public void testDoesNotClampWhenHardwareMipmapGenerationIsPending() {
+        int maxLevel = GLRenderer.textureMaxLevelForUpload(
+                true,
+                true,
+                true,
+                false,
+                null,
+                7);
+
+        assertEquals(7, maxLevel);
+    }
+
+    @Test
+    public void testClampsToBaseLevelWhenNoMipmapsAreUploaded() {
+        int maxLevel = GLRenderer.textureMaxLevelForUpload(
+                true,
+                false,
+                false,
+                false,
+                null,
+                4);
+
+        assertEquals(0, maxLevel);
+    }
+
+    @Test
+    public void testUsesUploadedMipCountForExistingOrCpuMipmaps() {
+        int maxLevel = GLRenderer.textureMaxLevelForUpload(
+                true,
+                true,
+                false,
+                true,
+                new int[] {64, 16, 4, 1},
+                7);
+
+        assertEquals(3, maxLevel);
+    }
+
+    @Test
+    public void testSkipsMaxLevelWhenCapabilityIsUnavailable() {
+        int maxLevel = GLRenderer.textureMaxLevelForUpload(
+                false,
+                false,
+                false,
+                true,
+                new int[] {64, 16},
+                7);
+
+        assertEquals(-1, maxLevel);
+    }
+
+    @Test
+    public void testClampsToBaseLevelWhenRequestedMipmapsCannotBeGeneratedOrUploaded() {
+        int maxLevel = GLRenderer.textureMaxLevelForUpload(
+                true,
+                true,
+                false,
+                false,
+                null,
+                7);
+
+        assertEquals(0, maxLevel);
+    }
+
+    @Test
+    public void testGeneratedMipMaxLevelUsesLargestUploadDimension() {
+        assertEquals(0, GLRenderer.generatedMipMaxLevel(1, 1, 1));
+        assertEquals(2, GLRenderer.generatedMipMaxLevel(3, 5, 1));
+        assertEquals(3, GLRenderer.generatedMipMaxLevel(4, 8, 1));
+        assertEquals(4, GLRenderer.generatedMipMaxLevel(4, 8, 16));
+    }
+}
diff --git a/jme3-core/src/test/java/com/jme3/util/MipMapGeneratorTest.java b/jme3-core/src/test/java/com/jme3/util/MipMapGeneratorTest.java
