From b805bb5f231f8262475753a10f2a2217cca6b189 Mon Sep 17 00:00:00 2001 From: Mr Robot Date: Fri, 2 Oct 2026 12:23:12 +0200 Subject: [PATCH 1/6] Fix framebuffer attachment updates and resource cleanup Track native framebuffer attachments and all owned renderbuffers across target mutation, allocation failure, explicit deletion, managed cleanup, and context reset. Preserve existing storage and guard stale references when native names are reused. Use discovered presentation framebuffer names for copies, restoration, and overrides. Build directly on upstream master without the mipmap/null-storage or GLES3 capability changes from PR #2983. Add 34 recording-GL regressions in three dedicated test classes. Local core/effects validation: 522 passed, one existing test skipped, zero failures. --- .../com/jme3/renderer/opengl/GLRenderer.java | 117 +++++-- .../java/com/jme3/texture/FrameBuffer.java | 19 +- .../com/jme3/util/NativeObjectManager.java | 19 ++ .../opengl/FrameBufferTestSupport.java | 306 ++++++++++++++++++ .../GlRendererDefaultFrameBufferTest.java | 108 +++++++ .../GlRendererFrameBufferAttachmentTest.java | 225 +++++++++++++ .../GlRendererFrameBufferLifecycleTest.java | 225 +++++++++++++ 7 files changed, 990 insertions(+), 29 deletions(-) create mode 100644 jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java create mode 100644 jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererDefaultFrameBufferTest.java create mode 100644 jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferAttachmentTest.java create mode 100644 jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java 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 index 62d5d52e04..46b1860f42 100644 --- a/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java +++ b/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java @@ -102,6 +102,17 @@ public final class GLRenderer implements Renderer { private FrameBuffer mainFbOverride = null; private int defaultFBO = 0; + + /** Native attachment state, without a reference to the owning FrameBuffer. */ + private static final class FrameBufferState { + final Set attachmentSlots = new HashSet<>(); + final Map renderBuffers = new HashMap<>(); + final ArrayList colorTextures = new ArrayList<>(); + } + + // Destructible framebuffer clones contain only an ID. Keep their resources + // here so allocations and target changes after registration are also tracked. + private final Map frameBufferStates = new HashMap<>(); private final Statistics statistics = new Statistics(); private int vpX, vpY, vpW, vpH; private int clipX, clipY, clipW, clipH; @@ -895,11 +906,10 @@ public void initialize() { gl.glGetInteger(GL.GL_FRAMEBUFFER_BINDING, tmp); tmp.rewind(); int fbOnLoad = tmp.get(); - if(fbOnLoad > 0) - { - // Override default FB to fbOnLoad. Mostly an iOS fix for scene processors and filters. - defaultFBO = fbOnLoad; - } + // The presentation framebuffer can be nonzero, notably on iOS. + defaultFBO = fbOnLoad; + context.boundFBO = fbOnLoad; + context.boundFB = null; } @Override @@ -915,6 +925,12 @@ public void invalidateState() { public void resetGLObjects() { logger.log(Level.FINE, "Resetting objects and invalidating state"); objManager.resetObjects(); + for (FrameBufferState state : frameBufferStates.values()) { + for (RenderBuffer rb : state.renderBuffers.keySet()) { + rb.resetObject(); + } + } + frameBufferStates.clear(); statistics.clearMemory(); invalidateState(); } @@ -2026,7 +2042,7 @@ public void copyFrameBuffer(FrameBuffer src, FrameBuffer dst, boolean copyColor, } if (src == null) { - glfbo.glBindFramebufferEXT(GLFbo.GL_READ_FRAMEBUFFER_EXT, 0); + glfbo.glBindFramebufferEXT(GLFbo.GL_READ_FRAMEBUFFER_EXT, defaultFBO); srcX0 = vpX; srcY0 = vpY; srcX1 = vpX + vpW; @@ -2037,7 +2053,7 @@ public void copyFrameBuffer(FrameBuffer src, FrameBuffer dst, boolean copyColor, srcY1 = src.getHeight(); } if (dst == null) { - glfbo.glBindFramebufferEXT(GLFbo.GL_DRAW_FRAMEBUFFER_EXT, 0); + glfbo.glBindFramebufferEXT(GLFbo.GL_DRAW_FRAMEBUFFER_EXT, defaultFBO); dstX0 = vpX; dstY0 = vpY; dstX1 = vpX + vpW; @@ -2107,6 +2123,8 @@ private void checkFrameBufferError() { private void updateRenderBuffer(FrameBuffer fb, RenderBuffer rb) { int id = rb.getId(); + FrameBufferState state = frameBufferStates.get(fb.getId()); + state.renderBuffers.putIfAbsent(rb, 0); if (id == -1) { glfbo.glGenRenderbuffersEXT(intBuf1); id = intBuf1.get(0); @@ -2125,6 +2143,10 @@ private void updateRenderBuffer(FrameBuffer fb, RenderBuffer rb) { } GLImageFormat glFmt = texUtil.getImageFormatWithError(rb.getFormat(), fb.isSrgb()); + if (state.renderBuffers.get(rb) == glFmt.internalFormat) { + // Reattaching an unchanged buffer must not discard its contents. + return; + } if (fb.getSamples() > 1 && caps.contains(Caps.FrameBufferMultisample)) { int samples = fb.getSamples(); @@ -2143,6 +2165,7 @@ private void updateRenderBuffer(FrameBuffer fb, RenderBuffer rb) { fb.getWidth(), fb.getHeight()); } + state.renderBuffers.put(rb, glFmt.internalFormat); } private int convertAttachmentSlot(int attachmentSlot) { @@ -2204,21 +2227,17 @@ public void updateFrameBufferAttachment(FrameBuffer fb, RenderBuffer rb) { + " is not depth-renderable and cannot be used as a depth attachment."); } - boolean needAttach; if (rb.getTexture() == null) { - // if it hasn't been created yet, then attach is required. - needAttach = rb.getId() == -1; updateRenderBuffer(fb, rb); - } else { - needAttach = false; - updateRenderTexture(fb, rb); - } - if (needAttach) { + // A surviving renderbuffer may have moved to a different color slot. glfbo.glFramebufferRenderbufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, convertAttachmentSlot(rb.getSlot()), GLFbo.GL_RENDERBUFFER_EXT, rb.getId()); + } else { + updateRenderTexture(fb, rb); } + frameBufferStates.get(fb.getId()).attachmentSlots.add(rb.getSlot()); } private void bindFrameBuffer(FrameBuffer fb) { @@ -2268,11 +2287,40 @@ public void updateFrameBuffer(FrameBuffer fb) { glfbo.glGenFramebuffersEXT(intBuf1); id = intBuf1.get(0); fb.setId(id); + frameBufferStates.put(id, new FrameBufferState()); objManager.registerObject(fb); statistics.onNewFrameBuffer(); } bindFrameBuffer(fb); + FrameBufferState state = frameBufferStates.get(id); + Set slots = new HashSet<>(); + Set buffers = new HashSet<>(); + if (fb.getDepthTarget() != null) { + slots.add(fb.getDepthTarget().getSlot()); + buffers.add(fb.getDepthTarget()); + } + for (int i = 0; i < fb.getNumColorTargets(); i++) { + RenderBuffer rb = fb.getColorTarget(i); + slots.add(rb.getSlot()); + buffers.add(rb); + } + for (Iterator it = state.attachmentSlots.iterator(); it.hasNext();) { + int slot = it.next(); + if (!slots.contains(slot)) { + glfbo.glFramebufferRenderbufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, + convertAttachmentSlot(slot), GLFbo.GL_RENDERBUFFER_EXT, 0); + it.remove(); + } + } + for (Iterator it = state.renderBuffers.keySet().iterator(); it.hasNext();) { + RenderBuffer rb = it.next(); + if (!buffers.contains(rb)) { + deleteRenderBuffer(rb); + it.remove(); + } + } + state.colorTextures.clear(); FrameBuffer.RenderBuffer depthBuf = fb.getDepthTarget(); if (depthBuf != null) { @@ -2282,6 +2330,9 @@ public void updateFrameBuffer(FrameBuffer fb) { for (int i = 0; i < fb.getNumColorTargets(); i++) { FrameBuffer.RenderBuffer colorBuf = fb.getColorTarget(i); updateFrameBufferAttachment(fb, colorBuf); + if (colorBuf.getTexture() != null) { + state.colorTextures.add(colorBuf.getTexture()); + } } setReadDrawBuffers(fb); @@ -2314,7 +2365,7 @@ public Vector2f[] getFrameBufferSamplePositions(FrameBuffer fb) { @Override public void setMainFrameBufferOverride(FrameBuffer fb) { mainFbOverride = null; - if (context.boundFBO == 0) { + if (context.boundFBO == defaultFBO) { // Main FB is now set to fb, make sure its bound setFrameBuffer(fb); } @@ -2405,9 +2456,10 @@ public void setFrameBuffer(FrameBuffer fb) { if (boundFB != null && (boundFB.getMipMapsGenerationHint() != null ? boundFB.getMipMapsGenerationHint() : generateMipmapsForFramebuffers)) { - for (int i = 0; i < boundFB.getNumColorTargets(); i++) { - RenderBuffer rb = boundFB.getColorTarget(i); - Texture tex = rb.getTexture(); + // Use the native attachment snapshot: the Java targets may already + // have been replaced with textures that have not been allocated yet. + FrameBufferState state = frameBufferStates.get(boundFB.getId()); + for (Texture tex : state.colorTextures) { if (tex != null && tex.getMinFilter().usesMipMapLevels() && isMipmapGenerationSupported(tex.getImage().getFormat(), linearizeSrgbImages && boundFB.isSrgb() @@ -2489,28 +2541,37 @@ public void readFrameBufferWithFormat(FrameBuffer fb, ByteBuffer byteBuf, Image. readFrameBufferWithGLFormat(fb, byteBuf, glFormat.format, glFormat.dataType); } - private void deleteRenderBuffer(FrameBuffer fb, RenderBuffer rb) { - intBuf1.put(0, rb.getId()); + private void deleteRenderBuffer(RenderBuffer rb) { + int id = rb.getId(); + if (id == -1 || rb.getTexture() != null) { + return; + } + intBuf1.put(0, id); glfbo.glDeleteRenderbuffersEXT(intBuf1); + if (context.boundRB == id) { + context.boundRB = 0; + } + rb.resetObject(); } @Override public void deleteFrameBuffer(FrameBuffer fb) { if (fb.getId() != -1) { if (context.boundFBO == fb.getId()) { - glfbo.glBindFramebufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, 0); - context.boundFBO = 0; + bindFrameBuffer(null); + toggleFramebufferSrgb(null); } - if (fb.getDepthTarget() != null) { - deleteRenderBuffer(fb, fb.getDepthTarget()); - } - if (fb.getColorTarget() != null) { - deleteRenderBuffer(fb, fb.getColorTarget()); + FrameBufferState state = frameBufferStates.remove(fb.getId()); + if (state != null) { + for (RenderBuffer rb : state.renderBuffers.keySet()) { + deleteRenderBuffer(rb); + } } intBuf1.put(0, fb.getId()); glfbo.glDeleteFramebuffersEXT(intBuf1); + objManager.unregisterObject(fb); fb.resetObject(); statistics.onDeleteFrameBuffer(); diff --git a/jme3-core/src/main/java/com/jme3/texture/FrameBuffer.java b/jme3-core/src/main/java/com/jme3/texture/FrameBuffer.java index 8f696f4b49..5d1c8c4302 100644 --- a/jme3-core/src/main/java/com/jme3/texture/FrameBuffer.java +++ b/jme3-core/src/main/java/com/jme3/texture/FrameBuffer.java @@ -258,12 +258,14 @@ private FrameBuffer() { public void addColorTarget(FrameBufferBufferTarget colorBuf){ colorBuf.slot=colorBufs.size(); colorBufs.add(colorBuf); + setUpdateNeeded(); } public void addColorTarget(FrameBufferTextureTarget colorBuf){ // checkSetTexture(colorBuf.getTexture(), false); // TODO: this won't work for levels. colorBuf.slot=colorBufs.size(); colorBufs.add(colorBuf); + setUpdateNeeded(); } /** @@ -281,6 +283,7 @@ public void replaceColorTarget(int i, FrameBufferTextureTarget colorBuf) { } colorBuf.slot = i; colorBufs.set(i, colorBuf); + setUpdateNeeded(); } /** @@ -293,9 +296,15 @@ public void replaceColorTarget(int i, FrameBufferTextureTarget colorBuf) { */ public void removeColorTarget(int i) { colorBufs.remove(i); + if (colorBufIndex > i) { + colorBufIndex--; + } else if (colorBufIndex == i && colorBufIndex == colorBufs.size()) { + colorBufIndex = Math.max(0, colorBufs.size() - 1); + } for (; i < colorBufs.size(); i++) { colorBufs.get(i).slot = i; } + setUpdateNeeded(); } /** @@ -310,6 +319,7 @@ public void addColorTarget(FrameBufferTextureTarget colorBuf, TextureCubeMap.Fac colorBuf.slot = colorBufs.size(); colorBuf.face = face.ordinal(); colorBufs.add(colorBuf); + setUpdateNeeded(); } public void setDepthTarget(FrameBufferBufferTarget depthBuf){ @@ -317,12 +327,14 @@ public void setDepthTarget(FrameBufferBufferTarget depthBuf){ throw new IllegalArgumentException("Depth buffer format must be depth."); this.depthBuf = depthBuf; this.depthBuf.slot = this.depthBuf.getFormat().isDepthStencilFormat() ? SLOT_DEPTH_STENCIL : SLOT_DEPTH; + setUpdateNeeded(); } public void setDepthTarget(FrameBufferTextureTarget depthBuf){ checkSetTexture(depthBuf.getTexture(), true); this.depthBuf = depthBuf; this.depthBuf.slot = depthBuf.getTexture().getImage().getFormat().isDepthStencilFormat() ? SLOT_DEPTH_STENCIL : SLOT_DEPTH; + setUpdateNeeded(); } public int getNumColorTargets(){ @@ -474,6 +486,7 @@ public void setMultiTarget(boolean enabled) { } else { colorBufIndex = 0; } + setUpdateNeeded(); } /** @@ -499,7 +512,7 @@ public void setTargetIndex(int index) { throw new IllegalArgumentException("Target index must be between 0 and 16"); } - if (colorBufs.size() < index) { + if (colorBufs.size() <= index) { throw new IllegalArgumentException("The target at " + index + " is not set!"); } @@ -568,6 +581,10 @@ public void setColorTexture(TextureCubeMap tex, TextureCubeMap.Face face) { */ public void clearColorTargets() { colorBufs.clear(); + if (colorBufIndex >= 0) { + colorBufIndex = 0; + } + setUpdateNeeded(); } /** diff --git a/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java b/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java index fed89068f6..0f976a9077 100644 --- a/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java +++ b/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java @@ -116,6 +116,20 @@ public void registerObject(NativeObject obj) { } } + /** + * Unregisters an object explicitly deleted by a renderer. This is also safe + * when the manager has already unregistered it before calling the renderer. + * + * @param obj the deleted native object (not null, with its original ID) + */ + public void unregisterObject(NativeObject obj) { + NativeObjectRef ref = refMap.remove(obj.getUniqueId()); + if (ref != null) { + ref.clear(); + } + userDeletionQueue.removeIf(queued -> queued == obj); + } + private void deleteNativeObject(Object rendererObject, NativeObject obj, NativeObjectRef ref, boolean deleteGL, boolean deleteBufs) { assert rendererObject != null; @@ -187,6 +201,11 @@ public void deleteUnused(Object rendererObject){ if (ref == null) { break; } + // Explicit deletion may have unregistered an already queued reference. + // Its native name can now belong to a different object. + if (refMap.get(ref.objClone.getUniqueId()) != ref) { + continue; + } deleteNativeObject(rendererObject, ref.objClone, ref, true, false); removed++; diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java new file mode 100644 index 0000000000..fc214e50cb --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java @@ -0,0 +1,306 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertTrue; + +import com.jme3.renderer.RendererException; +import com.jme3.texture.FrameBuffer; +import com.jme3.texture.Image; +import com.jme3.texture.Texture2D; +import com.jme3.util.NativeObjectManager; +import java.lang.ref.Reference; +import java.lang.reflect.Field; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.nio.IntBuffer; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; + +/** Shared recording-GL fixture; does not require or emulate a native GPU. */ +abstract class FrameBufferTestSupport { + static final int GL_STENCIL_ATTACHMENT = 0x8D20; + RecordingGL driver; + GLRenderer renderer; + + @BeforeEach + void initializeRenderer() { + initializeRenderer(0); + } + + void initializeRenderer(int defaultFramebuffer) { + driver = new RecordingGL(defaultFramebuffer); + renderer = new GLRenderer(driver.proxy(GL2.class), + driver.proxy(GLExt.class), driver.proxy(GLFbo.class)); + renderer.initialize(); + } + + static Texture2D texture() { + return new Texture2D(8, 8, Image.Format.RGBA8); + } + + static FrameBuffer framebuffer(Texture2D... textures) { + FrameBuffer fb = new FrameBuffer(8, 8, 1); + for (Texture2D texture : textures) { + fb.addColorTarget(FrameBuffer.FrameBufferTarget.newTarget(texture)); + } + return fb; + } + + static FrameBuffer bufferedFramebuffer(int colorCount) { + FrameBuffer fb = new FrameBuffer(8, 8, 1); + for (int i = 0; i < colorCount; ++i) { + fb.addColorTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.RGBA8)); + } + fb.setDepthTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.Depth)); + return fb; + } + + Reference registeredReference(FrameBuffer framebuffer) throws ReflectiveOperationException { + Field managerField = GLRenderer.class.getDeclaredField("objManager"); + managerField.setAccessible(true); + NativeObjectManager manager = (NativeObjectManager) managerField.get(renderer); + Field mapField = NativeObjectManager.class.getDeclaredField("refMap"); + mapField.setAccessible(true); + Map map = (Map) mapField.get(manager); + return (Reference) map.get(framebuffer.getUniqueId()); + } + + static final class RecordingGL implements InvocationHandler { + final int defaultFramebuffer; + final Map> attachments = new HashMap<>(); + final Set liveRenderbuffers = new HashSet<>(); + final Set liveFramebuffers = new HashSet<>(); + final Set liveTextures = new HashSet<>(); + final List deletedRenderbuffers = new ArrayList<>(); + final List renderbufferStorageCalls = new ArrayList<>(); + final Map renderbufferFormats = new HashMap<>(); + final Map> drawBuffers = new HashMap<>(); + final List mipmapGenerations = new ArrayList<>(); + final Map boundTextures = new HashMap<>(); + int nextObjectId = 100; + int reuseRenderbufferId = -1; + int reuseFramebufferId = -1; + int boundRenderbuffer; + int drawFramebuffer; + int readFramebuffer; + int blitDrawFramebuffer = -1; + int blitReadFramebuffer = -1; + int activeTextureUnit; + int framebufferStatus = GLFbo.GL_FRAMEBUFFER_COMPLETE_EXT; + boolean failNextStorage; + + RecordingGL(int defaultFramebuffer) { + this.defaultFramebuffer = defaultFramebuffer; + drawFramebuffer = defaultFramebuffer; + readFramebuffer = defaultFramebuffer; + } + + T proxy(Class api) { + return api.cast(Proxy.newProxyInstance(api.getClassLoader(), new Class[] {api}, this)); + } + + int attachment(FrameBuffer fb, int colorSlot) { + return attachmentAt(fb, GLFbo.GL_COLOR_ATTACHMENT0_EXT + colorSlot); + } + + int attachmentAt(FrameBuffer fb, int glSlot) { + return attachments.get(fb.getId()).getOrDefault(glSlot, 0); + } + + @Override + public Object invoke(Object proxy, Method method, Object[] args) { + switch (method.getName()) { + case "glGetString": + return getString((Integer) args[0]); + case "glGetInteger": + ((IntBuffer) args[1]).put(0, getInteger((Integer) args[0])); + return null; + case "glGenTextures": + int texture = nextObjectId++; + liveTextures.add(texture); + ((IntBuffer) args[0]).put(0, texture); + return null; + case "glDeleteTextures": + liveTextures.remove(((IntBuffer) args[0]).get(0)); + return null; + case "glActiveTexture": + activeTextureUnit = (Integer) args[0] - GL.GL_TEXTURE0; + return null; + case "glBindTexture": + int textureName = (Integer) args[1]; + assertTrue(textureName >= 0, "Cannot bind an unallocated image name"); + boundTextures.put(activeTextureUnit, textureName); + return null; + case "glGenerateMipmapEXT": + int generated = boundTextures.getOrDefault(activeTextureUnit, 0); + assertTrue(liveTextures.contains(generated), "Mipmap generation needs allocated storage"); + mipmapGenerations.add(generated); + return null; + case "glGenFramebuffersEXT": + int framebuffer = reuseFramebufferId >= 0 ? reuseFramebufferId : nextObjectId++; + reuseFramebufferId = -1; + liveFramebuffers.add(framebuffer); + attachments.put(framebuffer, new HashMap<>()); + ((IntBuffer) args[0]).put(0, framebuffer); + return null; + case "glGenRenderbuffersEXT": + int renderbuffer = reuseRenderbufferId >= 0 ? reuseRenderbufferId : nextObjectId++; + reuseRenderbufferId = -1; + liveRenderbuffers.add(renderbuffer); + ((IntBuffer) args[0]).put(0, renderbuffer); + return null; + case "glBindRenderbufferEXT": + boundRenderbuffer = (Integer) args[1]; + return null; + case "glRenderbufferStorageEXT": + case "glRenderbufferStorageMultisampleEXT": + assertTrue(liveRenderbuffers.contains(boundRenderbuffer), + "Storage requires a live, bound renderbuffer"); + if (failNextStorage) { + failNextStorage = false; + throw new RendererException("Simulated storage allocation failure"); + } + renderbufferStorageCalls.add(boundRenderbuffer); + int formatIndex = method.getName().equals("glRenderbufferStorageEXT") ? 1 : 2; + renderbufferFormats.put(boundRenderbuffer, (Integer) args[formatIndex]); + return null; + case "glDeleteRenderbuffersEXT": + int deleted = ((IntBuffer) args[0]).get(0); + assertTrue(liveRenderbuffers.remove(deleted), + "Deleting a stale renderbuffer name " + deleted); + deletedRenderbuffers.add(deleted); + if (boundRenderbuffer == deleted) { + boundRenderbuffer = 0; + } + return null; + case "glDeleteFramebuffersEXT": + int deletedFramebuffer = ((IntBuffer) args[0]).get(0); + assertTrue(liveFramebuffers.remove(deletedFramebuffer), + "Deleting a stale framebuffer name"); + attachments.remove(deletedFramebuffer); + return null; + case "glBindFramebufferEXT": + int target = (Integer) args[0]; + int id = (Integer) args[1]; + if (target == GLFbo.GL_FRAMEBUFFER_EXT || target == GLFbo.GL_DRAW_FRAMEBUFFER_EXT) { + drawFramebuffer = id; + } + if (target == GLFbo.GL_FRAMEBUFFER_EXT || target == GLFbo.GL_READ_FRAMEBUFFER_EXT) { + readFramebuffer = id; + } + return null; + case "glFramebufferTexture2DEXT": + case "glFramebufferRenderbufferEXT": + int slot = (Integer) args[1]; + int attachment = (Integer) args[3]; + if (slot == GL3.GL_DEPTH_STENCIL_ATTACHMENT) { + attachments.get(drawFramebuffer).put(GLFbo.GL_DEPTH_ATTACHMENT_EXT, attachment); + attachments.get(drawFramebuffer).put(GL_STENCIL_ATTACHMENT, attachment); + } else { + attachments.get(drawFramebuffer).put(slot, attachment); + } + return null; + case "glDrawBuffer": + drawBuffers.put(drawFramebuffer, java.util.Collections.singletonList((Integer) args[0])); + return null; + case "glDrawBuffers": + List targets = new ArrayList<>(); + IntBuffer source = ((IntBuffer) args[0]).duplicate(); + while (source.hasRemaining()) { + targets.add(source.get()); + } + drawBuffers.put(drawFramebuffer, targets); + return null; + case "glCheckFramebufferStatusEXT": + return framebufferStatus; + case "glBlitFramebufferEXT": + blitDrawFramebuffer = drawFramebuffer; + blitReadFramebuffer = readFramebuffer; + return null; + default: + if (method.getReturnType() == void.class) { + return null; + } + if (method.getReturnType() == boolean.class) { + return false; + } + throw new AssertionError("Unexpected GL query: " + method.getName()); + } + } + + private static String getString(int name) { + switch (name) { + case GL.GL_VERSION: + return "2.1"; + case GL.GL_SHADING_LANGUAGE_VERSION: + return "1.20"; + case GL.GL_EXTENSIONS: + return "GL_EXT_framebuffer_object GL_EXT_framebuffer_blit GL_EXT_packed_depth_stencil " + + "GL_EXT_texture_sRGB GL_ARB_framebuffer_sRGB"; + case GL.GL_VENDOR: + case GL.GL_RENDERER: + return "Recording GL"; + default: + throw new AssertionError("Unexpected GL string: " + name); + } + } + + private int getInteger(int name) { + switch (name) { + case GL.GL_FRAMEBUFFER_BINDING: + return defaultFramebuffer; + case GL.GL_MAX_TEXTURE_SIZE: + case GL.GL_MAX_CUBE_MAP_TEXTURE_SIZE: + case GLFbo.GL_MAX_RENDERBUFFER_SIZE_EXT: + return 1024; + case GL.GL_MAX_TEXTURE_IMAGE_UNITS: + case GL.GL_MAX_VERTEX_ATTRIBS: + return 16; + case GLFbo.GL_MAX_COLOR_ATTACHMENTS_EXT: + case GLExt.GL_MAX_DRAW_BUFFERS_ARB: + return 4; + case GL2.GL_DRAW_BUFFER: + case GL2.GL_READ_BUFFER: + return GL.GL_BACK; + default: + return 0; + } + } + } +} diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererDefaultFrameBufferTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererDefaultFrameBufferTest.java new file mode 100644 index 0000000000..7956a67b82 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererDefaultFrameBufferTest.java @@ -0,0 +1,108 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +import com.jme3.texture.FrameBuffer; +import org.junit.jupiter.api.Test; + +/** Tests the platform presentation framebuffer when its name is nonzero. */ +public class GlRendererDefaultFrameBufferTest extends FrameBufferTestSupport { + @Test + void copyingToScreenUsesDiscoveredFramebuffer() { + initializeRenderer(77); + FrameBuffer fb = framebuffer(texture()); + renderer.setFrameBuffer(fb); + renderer.setFrameBuffer(null); + assertEquals(77, driver.drawFramebuffer); + renderer.copyFrameBuffer(fb, null, true, false); + assertEquals(77, driver.blitDrawFramebuffer); + } + + @Test + void copyingFromScreenUsesDiscoveredFramebuffer() { + initializeRenderer(77); + FrameBuffer fb = framebuffer(texture()); + renderer.setFrameBuffer(fb); + renderer.setFrameBuffer(null); + assertEquals(77, driver.readFramebuffer); + renderer.copyFrameBuffer(null, fb, true, false); + assertEquals(77, driver.blitReadFramebuffer); + } + + @Test + void zeroDefaultCopyRestoresPreviousFramebuffer() { + FrameBuffer fb = framebuffer(texture()); + renderer.setFrameBuffer(fb); + renderer.copyFrameBuffer(fb, null, true, false); + assertEquals(0, driver.blitDrawFramebuffer); + assertEquals(fb.getId(), driver.drawFramebuffer); + assertEquals(fb.getId(), driver.readFramebuffer); + } + + @Test + void copyImmediatelyAfterInitializationRestoresDiscoveredFramebuffer() { + initializeRenderer(77); + FrameBuffer fb = framebuffer(texture()); + renderer.copyFrameBuffer(fb, null, true, false); + assertEquals(77, driver.blitDrawFramebuffer); + assertEquals(77, driver.drawFramebuffer); + assertEquals(77, driver.readFramebuffer); + assertNull(renderer.getCurrentFrameBuffer()); + } + + @Test + void deletingBoundFramebufferRestoresScreenAndClearsObjectCache() { + initializeRenderer(77); + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + renderer.deleteFrameBuffer(fb); + assertEquals(77, driver.drawFramebuffer); + assertEquals(77, driver.readFramebuffer); + assertNull(renderer.getCurrentFrameBuffer()); + renderer.setFrameBuffer(null); + assertEquals(77, driver.drawFramebuffer); + } + + @Test + void mainOverrideIsBoundWhenDiscoveredDefaultIsCurrent() { + initializeRenderer(77); + FrameBuffer override = framebuffer(texture()); + renderer.setMainFrameBufferOverride(override); + assertEquals(override.getId(), driver.drawFramebuffer); + renderer.copyFrameBuffer(null, null, true, false); + assertEquals(override.getId(), driver.blitReadFramebuffer); + assertEquals(override.getId(), driver.blitDrawFramebuffer); + } +} diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferAttachmentTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferAttachmentTest.java new file mode 100644 index 0000000000..b19ff5124e --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferAttachmentTest.java @@ -0,0 +1,225 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.jme3.texture.FrameBuffer; +import com.jme3.texture.Image; +import com.jme3.texture.Texture; +import com.jme3.texture.Texture2D; +import java.util.Arrays; +import org.junit.jupiter.api.Test; + +/** Tests updates to already initialized framebuffer attachments. */ +public class GlRendererFrameBufferAttachmentTest extends FrameBufferTestSupport { + @Test + void initialAttachmentAndExplicitDirtyReplacement() { + Texture2D first = texture(); + Texture2D replacement = texture(); + FrameBuffer fb = framebuffer(first); + renderer.setFrameBuffer(fb); + assertEquals(first.getImage().getId(), driver.attachment(fb, 0)); + fb.replaceColorTarget(0, FrameBuffer.FrameBufferTarget.newTarget(replacement)); + fb.setUpdateNeeded(); + renderer.setFrameBuffer(fb); + assertEquals(replacement.getImage().getId(), driver.attachment(fb, 0)); + assertNotEquals(first.getImage().getId(), replacement.getImage().getId()); + } + + @Test + void replacementWhileBoundUpdatesNativeAttachment() throws Exception { + Texture2D replacement = texture(); + renderer.setTexture(0, replacement); + FrameBuffer fb = framebuffer(texture()); + renderer.setFrameBuffer(fb); + fb.replaceColorTarget(0, FrameBuffer.FrameBufferTarget.newTarget(replacement)); + renderer.setFrameBuffer(fb); + assertEquals(replacement.getImage().getId(), driver.attachment(fb, 0)); + } + + @Test + void replacementAfterSwitchingAwayUpdatesNativeAttachment() throws Exception { + Texture2D replacement = texture(); + renderer.setTexture(0, replacement); + FrameBuffer fb = framebuffer(texture()); + renderer.setFrameBuffer(fb); + renderer.setFrameBuffer(null); + fb.replaceColorTarget(0, FrameBuffer.FrameBufferTarget.newTarget(replacement)); + renderer.setFrameBuffer(fb); + assertEquals(replacement.getImage().getId(), driver.attachment(fb, 0)); + } + + @Test + void removedTextureIsDetached() { + FrameBuffer fb = framebuffer(texture(), texture()); + renderer.setFrameBuffer(fb); + fb.removeColorTarget(1); + renderer.setFrameBuffer(fb); + assertEquals(0, driver.attachment(fb, 1)); + } + + @Test + void removalReattachesShiftedRenderbufferWithoutReallocatingIt() { + FrameBuffer fb = bufferedFramebuffer(2); + renderer.setFrameBuffer(fb); + FrameBuffer.RenderBuffer removed = fb.getColorTarget(0); + final int survivor = fb.getColorTarget(1).getId(); + final int allocations = driver.renderbufferStorageCalls.size(); + fb.removeColorTarget(0); + renderer.setFrameBuffer(fb); + assertEquals(-1, removed.getId()); + assertEquals(survivor, driver.attachment(fb, 0)); + assertEquals(0, driver.attachment(fb, 1)); + assertEquals(allocations, driver.renderbufferStorageCalls.size()); + } + + @Test + void addingColorTargetAllocatesOnlyNewStorage() { + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + fb.addColorTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.RGBA8)); + renderer.setFrameBuffer(fb); + assertEquals(3, driver.renderbufferStorageCalls.size()); + assertEquals(fb.getColorTarget(1).getId(), driver.attachment(fb, 1)); + } + + @Test + void clearingColorsLeavesDepthOnlyFramebuffer() { + FrameBuffer fb = bufferedFramebuffer(2); + renderer.setFrameBuffer(fb); + fb.clearColorTargets(); + renderer.setFrameBuffer(fb); + assertEquals(0, driver.attachment(fb, 0)); + assertEquals(0, driver.attachment(fb, 1)); + assertEquals(1, driver.liveRenderbuffers.size()); + assertEquals(Arrays.asList(GL.GL_NONE), driver.drawBuffers.get(fb.getId())); + } + + @Test + void removingSelectedLastTargetKeepsSelectionValid() { + FrameBuffer fb = framebuffer(texture(), texture()); + fb.setTargetIndex(1); + renderer.setFrameBuffer(fb); + fb.removeColorTarget(1); + renderer.setFrameBuffer(fb); + assertEquals(0, fb.getTargetIndex()); + assertEquals(Arrays.asList(GLFbo.GL_COLOR_ATTACHMENT0_EXT), driver.drawBuffers.get(fb.getId())); + assertThrows(IllegalArgumentException.class, () -> fb.setTargetIndex(1)); + } + + @Test + void removingEarlierTargetKeepsSelectedSurvivor() { + FrameBuffer fb = framebuffer(texture(), texture(), texture()); + fb.setTargetIndex(2); + renderer.setFrameBuffer(fb); + final int selected = driver.attachment(fb, 2); + fb.removeColorTarget(0); + renderer.setFrameBuffer(fb); + assertEquals(1, fb.getTargetIndex()); + assertEquals(selected, driver.attachment(fb, 1)); + assertEquals(0, driver.attachment(fb, 2)); + } + + @Test + void mrtChangesUpdateDrawBuffersWithoutDiscardingStorage() { + FrameBuffer fb = bufferedFramebuffer(2); + renderer.setFrameBuffer(fb); + final int allocations = driver.renderbufferStorageCalls.size(); + fb.setMultiTarget(true); + renderer.setFrameBuffer(fb); + assertEquals(Arrays.asList(GLFbo.GL_COLOR_ATTACHMENT0_EXT, GLFbo.GL_COLOR_ATTACHMENT0_EXT + 1), + driver.drawBuffers.get(fb.getId())); + fb.setMultiTarget(false); + renderer.setFrameBuffer(fb); + assertEquals(Arrays.asList(GLFbo.GL_COLOR_ATTACHMENT0_EXT), driver.drawBuffers.get(fb.getId())); + assertEquals(allocations, driver.renderbufferStorageCalls.size()); + } + + @Test + void replacingPackedDepthStencilDetachesOldStencil() { + FrameBuffer fb = framebuffer(texture()); + fb.setDepthTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.Depth24Stencil8)); + renderer.setFrameBuffer(fb); + int oldDepth = fb.getDepthTarget().getId(); + assertEquals(oldDepth, driver.attachmentAt(fb, GL_STENCIL_ATTACHMENT)); + fb.setDepthTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.Depth)); + renderer.setFrameBuffer(fb); + assertFalse(driver.liveRenderbuffers.contains(oldDepth)); + assertEquals(0, driver.attachmentAt(fb, GL_STENCIL_ATTACHMENT)); + assertEquals(fb.getDepthTarget().getId(), driver.attachmentAt(fb, GLFbo.GL_DEPTH_ATTACHMENT_EXT)); + } + + @Test + void replacingDepthBufferWithTextureUpdatesAttachment() { + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + Texture2D depth = new Texture2D(8, 8, Image.Format.Depth); + fb.setDepthTarget(FrameBuffer.FrameBufferTarget.newTarget(depth)); + renderer.setFrameBuffer(fb); + assertEquals(depth.getImage().getId(), driver.attachmentAt(fb, GLFbo.GL_DEPTH_ATTACHMENT_EXT)); + assertEquals(1, driver.liveRenderbuffers.size()); + } + + @Test + void mipmappedReplacementGeneratesOldRenderedTextureBeforeAllocatingNewOne() { + Texture2D old = texture(); + old.setMinFilter(Texture.MinFilter.Trilinear); + Texture2D replacement = texture(); + replacement.setMinFilter(Texture.MinFilter.Trilinear); + FrameBuffer fb = framebuffer(old); + renderer.setFrameBuffer(fb); + fb.replaceColorTarget(0, FrameBuffer.FrameBufferTarget.newTarget(replacement)); + renderer.setFrameBuffer(fb); + assertEquals(Arrays.asList(old.getImage().getId()), driver.mipmapGenerations); + assertEquals(replacement.getImage().getId(), driver.attachment(fb, 0)); + renderer.setFrameBuffer(null); + assertEquals(Arrays.asList(old.getImage().getId(), replacement.getImage().getId()), + driver.mipmapGenerations); + } + + @Test + void explicitSrgbStorageChangeStillReallocatesInternalFormat() { + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + int id = fb.getColorTarget(0).getId(); + final int oldFormat = driver.renderbufferFormats.get(id); + fb.setSrgb(true); + fb.setUpdateNeeded(); + renderer.setFrameBuffer(fb); + assertNotEquals(oldFormat, driver.renderbufferFormats.get(id)); + assertEquals(GLExt.GL_SRGB8_ALPHA8_EXT, driver.renderbufferFormats.get(id)); + } +} diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java new file mode 100644 index 0000000000..b3c0ad00a5 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java @@ -0,0 +1,225 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.jme3.renderer.RendererException; +import com.jme3.texture.FrameBuffer; +import com.jme3.texture.Image; +import java.lang.ref.Reference; +import org.junit.jupiter.api.Test; + +/** Tests ownership, deletion, native-name reuse, and context reset. */ +public class GlRendererFrameBufferLifecycleTest extends FrameBufferTestSupport { + @Test + void disposalDeletesEveryColorAndDepthRenderbuffer() { + FrameBuffer fb = bufferedFramebuffer(2); + renderer.setFrameBuffer(fb); + assertEquals(3, driver.liveRenderbuffers.size()); + fb.dispose(); + renderer.postFrame(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertTrue(driver.liveFramebuffers.isEmpty()); + } + + @Test + void managedCleanupDeletesOwnedRenderbuffersFromDestructibleClone() { + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + assertEquals(2, driver.liveRenderbuffers.size()); + renderer.cleanup(); + assertTrue(driver.liveFramebuffers.isEmpty()); + assertTrue(driver.liveRenderbuffers.isEmpty()); + } + + @Test + void singleTargetDisposalDeletesBothBuffers() { + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + fb.dispose(); + renderer.postFrame(); + assertEquals(2, driver.deletedRenderbuffers.size()); + } + + @Test + void disposalBeforeRebindStillDeletesRemovedBuffers() { + FrameBuffer fb = bufferedFramebuffer(2); + renderer.setFrameBuffer(fb); + fb.removeColorTarget(0); + fb.setDepthTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.Depth)); + fb.dispose(); + renderer.postFrame(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertEquals(3, driver.deletedRenderbuffers.size()); + } + + @Test + void managedCleanupIncludesRenderbuffersAddedAfterRegistration() { + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + fb.addColorTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.RGBA8)); + renderer.setFrameBuffer(fb); + renderer.cleanup(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertEquals(3, driver.deletedRenderbuffers.size()); + } + + @Test + void removedBufferCanBeAddedAgainWithFreshNativeStorage() { + FrameBuffer fb = bufferedFramebuffer(1); + FrameBuffer.FrameBufferBufferTarget color + = (FrameBuffer.FrameBufferBufferTarget) fb.getColorTarget(0); + renderer.setFrameBuffer(fb); + final int original = color.getId(); + fb.clearColorTargets(); + renderer.setFrameBuffer(fb); + assertEquals(-1, color.getId()); + fb.addColorTarget(color); + renderer.setFrameBuffer(fb); + assertNotEquals(original, color.getId()); + assertEquals(color.getId(), driver.attachment(fb, 0)); + assertEquals(3, driver.renderbufferStorageCalls.size()); + } + + @Test + void resetClearsRemovedWrappersAndDoesNotDeleteOldContextNames() { + FrameBuffer fb = bufferedFramebuffer(2); + renderer.setFrameBuffer(fb); + FrameBuffer.RenderBuffer removed = fb.getColorTarget(0); + fb.removeColorTarget(0); + renderer.resetGLObjects(); + assertEquals(-1, removed.getId()); + assertEquals(-1, fb.getId()); + // The old driver context and all of its names no longer exist. + driver.liveRenderbuffers.clear(); + driver.liveFramebuffers.clear(); + driver.attachments.clear(); + driver.boundRenderbuffer = 0; + renderer.setFrameBuffer(fb); + fb.dispose(); + renderer.postFrame(); + assertEquals(2, driver.deletedRenderbuffers.size()); + assertTrue(driver.liveRenderbuffers.isEmpty()); + } + + @Test + void deletingBoundRenderbufferInvalidatesCacheBeforeNameReuse() { + FrameBuffer first = bufferedFramebuffer(1); + renderer.setFrameBuffer(first); + final int reused = driver.boundRenderbuffer; + first.dispose(); + renderer.postFrame(); + assertEquals(0, driver.boundRenderbuffer); + FrameBuffer second = new FrameBuffer(8, 8, 1); + second.addColorTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.RGBA8)); + driver.reuseRenderbufferId = reused; + renderer.setFrameBuffer(second); + assertEquals(reused, driver.boundRenderbuffer); + assertEquals(reused, second.getColorTarget(0).getId()); + } + + @Test + void directDeletionUnregistersOldCloneBeforeFramebufferNameReuse() throws Exception { + FrameBuffer first = bufferedFramebuffer(1); + renderer.setFrameBuffer(first); + final int oldName = first.getId(); + Reference staleReference = registeredReference(first); + // Queue the old reference before direct deletion to exercise the queue guard. + assertTrue(staleReference.enqueue()); + first.dispose(); + renderer.deleteFrameBuffer(first); + driver.reuseFramebufferId = oldName; + FrameBuffer second = bufferedFramebuffer(2); + renderer.setFrameBuffer(second); + renderer.postFrame(); + assertEquals(oldName, second.getId()); + assertEquals(3, driver.liveRenderbuffers.size()); + renderer.cleanup(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertTrue(driver.liveFramebuffers.isEmpty()); + } + + @Test + void simulatedGcQueueDeletesAllOwnedBuffers() throws Exception { + FrameBuffer fb = bufferedFramebuffer(2); + renderer.setFrameBuffer(fb); + renderer.setFrameBuffer(null); + assertTrue(registeredReference(fb).enqueue()); + renderer.postFrame(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertTrue(driver.liveFramebuffers.isEmpty()); + } + + @Test + void storageAllocationFailureKeepsNameTrackedForCleanup() { + FrameBuffer fb = bufferedFramebuffer(1); + driver.failNextStorage = true; + assertThrows(RendererException.class, () -> renderer.setFrameBuffer(fb)); + assertEquals(1, driver.liveRenderbuffers.size()); + renderer.cleanup(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertTrue(driver.liveFramebuffers.isEmpty()); + } + + @Test + void storageAllocationFailureCanRetryExistingName() { + FrameBuffer fb = bufferedFramebuffer(1); + driver.failNextStorage = true; + assertThrows(RendererException.class, () -> renderer.setFrameBuffer(fb)); + renderer.setFrameBuffer(fb); + assertEquals(2, driver.renderbufferStorageCalls.size()); + assertEquals(2, driver.liveRenderbuffers.size()); + } + + @Test + void incompleteFramebufferFailureStillCleansEveryAllocation() { + FrameBuffer fb = bufferedFramebuffer(2); + driver.framebufferStatus = GLFbo.GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT_EXT; + assertThrows(IllegalStateException.class, () -> renderer.setFrameBuffer(fb)); + renderer.cleanup(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertTrue(driver.liveFramebuffers.isEmpty()); + } + + @Test + void textureAttachmentsAreNotDeletedAsRenderbufferNames() { + FrameBuffer fb = framebuffer(texture()); + renderer.setFrameBuffer(fb); + renderer.deleteFrameBuffer(fb); + assertTrue(driver.deletedRenderbuffers.isEmpty()); + assertEquals(1, driver.liveTextures.size()); + } +} From c185a0f06423d214ac3111428d9d67582cdf0c80 Mon Sep 17 00:00:00 2001 From: Mr Robot Date: Sat, 3 Oct 2026 00:02:50 +0200 Subject: [PATCH 2/6] Remove redundant framebuffer disposal test --- .../opengl/GlRendererFrameBufferLifecycleTest.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java index b3c0ad00a5..b61d3a534a 100644 --- a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java @@ -65,15 +65,6 @@ void managedCleanupDeletesOwnedRenderbuffersFromDestructibleClone() { assertTrue(driver.liveRenderbuffers.isEmpty()); } - @Test - void singleTargetDisposalDeletesBothBuffers() { - FrameBuffer fb = bufferedFramebuffer(1); - renderer.setFrameBuffer(fb); - fb.dispose(); - renderer.postFrame(); - assertEquals(2, driver.deletedRenderbuffers.size()); - } - @Test void disposalBeforeRebindStillDeletesRemovedBuffers() { FrameBuffer fb = bufferedFramebuffer(2); From 7e86c0fd7a3fe4469088b6ce4a6d436be521c0d4 Mon Sep 17 00:00:00 2001 From: toaster0123 <261194501+toaster0123@users.noreply.github.com> Date: Mon, 26 Oct 2026 13:49:46 +0000 Subject: [PATCH 3/6] Enforce framebuffer target ownership and reduce lifecycle overhead Reject shared target instances before mutation while allowing distinct targets on the same texture. Retain ownership after removal/disposal using an opaque token independent of the framebuffer and its phantom-reference handle. Replace boxed framebuffer maps and temporary sets with IntMap, a slot mask, and reusable primitive renderbuffer state. Pass state through attachment updates and use indexed mipmap iteration. Deduplicate disposal requests by identity and avoid repeated queue scans in managed deletion. Clear stale requests during context reset and full cleanup so they cannot delete a subsequently reinitialized resource. Add 23 regressions covering ownership, shared-texture survival, real owner GC, highest color slots, duplicate disposal, batch limits, reset and cleanup. Validation: core 605 passed / 1 existing skipped; effects 6 passed. Core main/test checkstyle and git diff --check pass; no checkstyle warnings on added/modified lines. No native GPU or frame-rate benchmark claimed. --- .../com/jme3/renderer/opengl/GLRenderer.java | 120 ++++++--- .../java/com/jme3/texture/FrameBuffer.java | 80 +++++- .../com/jme3/util/NativeObjectManager.java | 19 +- .../GlRendererFrameBufferAttachmentTest.java | 21 ++ .../GlRendererFrameBufferLifecycleTest.java | 156 ++++++++++++ .../FrameBufferTargetOwnershipTest.java | 240 ++++++++++++++++++ 6 files changed, 597 insertions(+), 39 deletions(-) create mode 100644 jme3-core/src/test/java/com/jme3/texture/FrameBufferTargetOwnershipTest.java 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 index 46b1860f42..1c01ac57fb 100644 --- a/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java +++ b/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java @@ -65,6 +65,7 @@ import com.jme3.texture.image.LastTextureState; import com.jme3.util.ByteBufferUtils; import com.jme3.util.BufferUtils; +import com.jme3.util.IntMap; import com.jme3.util.ListMap; import com.jme3.util.MipMapGenerator; import com.jme3.util.NativeObject; @@ -103,16 +104,39 @@ public final class GLRenderer implements Renderer { private FrameBuffer mainFbOverride = null; private int defaultFBO = 0; + /** Native storage state, allocated once per owned renderbuffer. */ + private static final class RenderBufferState { + final RenderBuffer target; + int internalFormat; + + RenderBufferState(RenderBuffer target) { + this.target = target; + } + } + /** Native attachment state, without a reference to the owning FrameBuffer. */ private static final class FrameBufferState { - final Set attachmentSlots = new HashSet<>(); - final Map renderBuffers = new HashMap<>(); + // Bits 0..15 are color slots; 16 and 17 are depth and depth-stencil. + int attachmentSlots; + final ArrayList renderBuffers = new ArrayList<>(); final ArrayList colorTextures = new ArrayList<>(); + + RenderBufferState getRenderBuffer(RenderBuffer target) { + for (int i = 0; i < renderBuffers.size(); i++) { + RenderBufferState buffer = renderBuffers.get(i); + if (buffer.target == target) { + return buffer; + } + } + RenderBufferState buffer = new RenderBufferState(target); + renderBuffers.add(buffer); + return buffer; + } } // Destructible framebuffer clones contain only an ID. Keep their resources // here so allocations and target changes after registration are also tracked. - private final Map frameBufferStates = new HashMap<>(); + private final IntMap frameBufferStates = new IntMap<>(); private final Statistics statistics = new Statistics(); private int vpX, vpY, vpW, vpH; private int clipX, clipY, clipW, clipH; @@ -925,9 +949,10 @@ public void invalidateState() { public void resetGLObjects() { logger.log(Level.FINE, "Resetting objects and invalidating state"); objManager.resetObjects(); - for (FrameBufferState state : frameBufferStates.values()) { - for (RenderBuffer rb : state.renderBuffers.keySet()) { - rb.resetObject(); + for (IntMap.Entry entry : frameBufferStates) { + FrameBufferState state = entry.getValue(); + for (int i = 0; i < state.renderBuffers.size(); i++) { + state.renderBuffers.get(i).target.resetObject(); } } frameBufferStates.clear(); @@ -2121,14 +2146,15 @@ private void checkFrameBufferError() { } } - private void updateRenderBuffer(FrameBuffer fb, RenderBuffer rb) { + private void updateRenderBuffer(FrameBuffer fb, RenderBuffer rb, FrameBufferState state) { int id = rb.getId(); - FrameBufferState state = frameBufferStates.get(fb.getId()); - state.renderBuffers.putIfAbsent(rb, 0); + // Track before allocating so failures still leave a cleanup path. + RenderBufferState buffer = state.getRenderBuffer(rb); if (id == -1) { glfbo.glGenRenderbuffersEXT(intBuf1); id = intBuf1.get(0); rb.setId(id); + buffer.internalFormat = 0; } if (context.boundRB != id) { @@ -2143,7 +2169,7 @@ private void updateRenderBuffer(FrameBuffer fb, RenderBuffer rb) { } GLImageFormat glFmt = texUtil.getImageFormatWithError(rb.getFormat(), fb.isSrgb()); - if (state.renderBuffers.get(rb) == glFmt.internalFormat) { + if (buffer.internalFormat == glFmt.internalFormat) { // Reattaching an unchanged buffer must not discard its contents. return; } @@ -2165,7 +2191,7 @@ private void updateRenderBuffer(FrameBuffer fb, RenderBuffer rb) { fb.getWidth(), fb.getHeight()); } - state.renderBuffers.put(rb, glFmt.internalFormat); + buffer.internalFormat = glFmt.internalFormat; } private int convertAttachmentSlot(int attachmentSlot) { @@ -2214,6 +2240,10 @@ public void updateRenderTexture(FrameBuffer fb, RenderBuffer rb) { } public void updateFrameBufferAttachment(FrameBuffer fb, RenderBuffer rb) { + updateFrameBufferAttachment(fb, rb, frameBufferStates.get(fb.getId())); + } + + private void updateFrameBufferAttachment(FrameBuffer fb, RenderBuffer rb, FrameBufferState state) { Image.Format format = rb.getFormat(); boolean depthTarget = rb.getSlot() == FrameBuffer.SLOT_DEPTH || rb.getSlot() == FrameBuffer.SLOT_DEPTH_STENCIL; @@ -2228,7 +2258,7 @@ public void updateFrameBufferAttachment(FrameBuffer fb, RenderBuffer rb) { } if (rb.getTexture() == null) { - updateRenderBuffer(fb, rb); + updateRenderBuffer(fb, rb, state); // A surviving renderbuffer may have moved to a different color slot. glfbo.glFramebufferRenderbufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, convertAttachmentSlot(rb.getSlot()), @@ -2237,7 +2267,7 @@ public void updateFrameBufferAttachment(FrameBuffer fb, RenderBuffer rb) { } else { updateRenderTexture(fb, rb); } - frameBufferStates.get(fb.getId()).attachmentSlots.add(rb.getSlot()); + state.attachmentSlots |= attachmentSlotMask(rb.getSlot()); } private void bindFrameBuffer(FrameBuffer fb) { @@ -2294,42 +2324,40 @@ public void updateFrameBuffer(FrameBuffer fb) { bindFrameBuffer(fb); FrameBufferState state = frameBufferStates.get(id); - Set slots = new HashSet<>(); - Set buffers = new HashSet<>(); + int slots = 0; if (fb.getDepthTarget() != null) { - slots.add(fb.getDepthTarget().getSlot()); - buffers.add(fb.getDepthTarget()); + slots |= attachmentSlotMask(fb.getDepthTarget().getSlot()); } for (int i = 0; i < fb.getNumColorTargets(); i++) { - RenderBuffer rb = fb.getColorTarget(i); - slots.add(rb.getSlot()); - buffers.add(rb); + slots |= attachmentSlotMask(fb.getColorTarget(i).getSlot()); } - for (Iterator it = state.attachmentSlots.iterator(); it.hasNext();) { - int slot = it.next(); - if (!slots.contains(slot)) { - glfbo.glFramebufferRenderbufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, - convertAttachmentSlot(slot), GLFbo.GL_RENDERBUFFER_EXT, 0); - it.remove(); - } + int removedSlots = state.attachmentSlots & ~slots; + while (removedSlots != 0) { + int index = Integer.numberOfTrailingZeros(removedSlots); + int slot = index == 16 ? FrameBuffer.SLOT_DEPTH + : index == 17 ? FrameBuffer.SLOT_DEPTH_STENCIL : index; + glfbo.glFramebufferRenderbufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, + convertAttachmentSlot(slot), GLFbo.GL_RENDERBUFFER_EXT, 0); + state.attachmentSlots &= ~(1 << index); + removedSlots &= removedSlots - 1; } - for (Iterator it = state.renderBuffers.keySet().iterator(); it.hasNext();) { - RenderBuffer rb = it.next(); - if (!buffers.contains(rb)) { + for (int i = state.renderBuffers.size() - 1; i >= 0; i--) { + RenderBuffer rb = state.renderBuffers.get(i).target; + if (!isFrameBufferTarget(fb, rb)) { deleteRenderBuffer(rb); - it.remove(); + state.renderBuffers.remove(i); } } state.colorTextures.clear(); FrameBuffer.RenderBuffer depthBuf = fb.getDepthTarget(); if (depthBuf != null) { - updateFrameBufferAttachment(fb, depthBuf); + updateFrameBufferAttachment(fb, depthBuf, state); } for (int i = 0; i < fb.getNumColorTargets(); i++) { FrameBuffer.RenderBuffer colorBuf = fb.getColorTarget(i); - updateFrameBufferAttachment(fb, colorBuf); + updateFrameBufferAttachment(fb, colorBuf, state); if (colorBuf.getTexture() != null) { state.colorTextures.add(colorBuf.getTexture()); } @@ -2341,6 +2369,25 @@ public void updateFrameBuffer(FrameBuffer fb) { fb.clearUpdateNeeded(); } + private static int attachmentSlotMask(int slot) { + if (slot == FrameBuffer.SLOT_DEPTH) { + return 1 << 16; + } else if (slot == FrameBuffer.SLOT_DEPTH_STENCIL) { + return 1 << 17; + } else if (slot < 0 || slot >= 16) { + throw new UnsupportedOperationException("Invalid FBO attachment slot: " + slot); + } + return 1 << slot; + } + + private static boolean isFrameBufferTarget(FrameBuffer fb, RenderBuffer rb) { + if (fb.getDepthTarget() == rb) { + return true; + } + int slot = rb.getSlot(); + return slot >= 0 && slot < fb.getNumColorTargets() && fb.getColorTarget(slot) == rb; + } + public Vector2f[] getFrameBufferSamplePositions(FrameBuffer fb) { if (fb.getSamples() <= 1) { throw new IllegalArgumentException("Framebuffer must be multisampled"); @@ -2459,7 +2506,8 @@ public void setFrameBuffer(FrameBuffer fb) { // Use the native attachment snapshot: the Java targets may already // have been replaced with textures that have not been allocated yet. FrameBufferState state = frameBufferStates.get(boundFB.getId()); - for (Texture tex : state.colorTextures) { + for (int i = 0; i < state.colorTextures.size(); i++) { + Texture tex = state.colorTextures.get(i); if (tex != null && tex.getMinFilter().usesMipMapLevels() && isMipmapGenerationSupported(tex.getImage().getFormat(), linearizeSrgbImages && boundFB.isSrgb() @@ -2564,8 +2612,8 @@ public void deleteFrameBuffer(FrameBuffer fb) { FrameBufferState state = frameBufferStates.remove(fb.getId()); if (state != null) { - for (RenderBuffer rb : state.renderBuffers.keySet()) { - deleteRenderBuffer(rb); + for (int i = 0; i < state.renderBuffers.size(); i++) { + deleteRenderBuffer(state.renderBuffers.get(i).target); } } diff --git a/jme3-core/src/main/java/com/jme3/texture/FrameBuffer.java b/jme3-core/src/main/java/com/jme3/texture/FrameBuffer.java index 5d1c8c4302..a49b8a70e2 100644 --- a/jme3-core/src/main/java/com/jme3/texture/FrameBuffer.java +++ b/jme3-core/src/main/java/com/jme3/texture/FrameBuffer.java @@ -87,10 +87,23 @@ public class FrameBuffer extends NativeObject { private String name; private Boolean mipMapsGenerationHint = null; + // Not this FrameBuffer or NativeObject.handleRef: renderer-held targets must + // not keep the owner (or its phantom-reference handle) alive. + private final Object targetOwner = new Object(); + /** * RenderBuffer represents either a texture or a * buffer that will be rendered to. RenderBuffers * are attached to an attachment slot on a FrameBuffer. + *

+ * A target instance belongs to the first framebuffer to which it is added, + * even after removal or disposal. It cannot be transferred to another + * framebuffer or attached to multiple slots at once. A removed target may + * be added again to its original framebuffer. + *

+ * To share a texture, create a separate target for each framebuffer using + * {@link FrameBufferTarget#newTarget(Texture)} with the same texture. + * Renderbuffer storage is owned exclusively by its framebuffer. */ public static class RenderBuffer { @@ -101,6 +114,7 @@ public static class RenderBuffer { int face = -1; int layer = -1; int level = 0; + private Object owner; public int getLevel() { @@ -255,14 +269,31 @@ public static FrameBufferTextureTarget newTarget(Texture tx, TextureCubeMap.Face private FrameBuffer() { } + /** + * Adds an exclusively owned color renderbuffer target. + * + * @param colorBuf the target (not null) + * @throws IllegalArgumentException if the target belongs to another + * framebuffer or is already attached to this framebuffer + */ public void addColorTarget(FrameBufferBufferTarget colorBuf){ + claimTarget(colorBuf, null); colorBuf.slot=colorBufs.size(); colorBufs.add(colorBuf); setUpdateNeeded(); } + /** + * Adds an exclusively owned color texture target. The underlying texture + * may be shared through distinct target instances. + * + * @param colorBuf the target (not null) + * @throws IllegalArgumentException if the target belongs to another + * framebuffer or is already attached to this framebuffer + */ public void addColorTarget(FrameBufferTextureTarget colorBuf){ // checkSetTexture(colorBuf.getTexture(), false); // TODO: this won't work for levels. + claimTarget(colorBuf, null); colorBuf.slot=colorBufs.size(); colorBufs.add(colorBuf); setUpdateNeeded(); @@ -276,11 +307,14 @@ public void addColorTarget(FrameBufferTextureTarget colorBuf){ * * @param i index of color target to replace * @param colorBuf color target to replace with + * @throws IllegalArgumentException if the target belongs to another + * framebuffer or is already attached to a different slot */ public void replaceColorTarget(int i, FrameBufferTextureTarget colorBuf) { if (i < 0 || i >= colorBufs.size()) { throw new IndexOutOfBoundsException("No color target exists to replace at index=" + i); } + claimTarget(colorBuf, colorBufs.get(i)); colorBuf.slot = i; colorBufs.set(i, colorBuf); setUpdateNeeded(); @@ -313,30 +347,74 @@ public void removeColorTarget(int i) { * * @param colorBuf texture to add to the color Buffer * @param face position to add to the color buffer + * @throws IllegalArgumentException if the target belongs to another + * framebuffer or is already attached to this framebuffer */ public void addColorTarget(FrameBufferTextureTarget colorBuf, TextureCubeMap.Face face) { // checkSetTexture(colorBuf.getTexture(), false); // TODO: this won't work for levels. + int faceIndex = face.ordinal(); + claimTarget(colorBuf, null); colorBuf.slot = colorBufs.size(); - colorBuf.face = face.ordinal(); + colorBuf.face = faceIndex; colorBufs.add(colorBuf); setUpdateNeeded(); } + /** + * Sets an exclusively owned depth renderbuffer target. + * + * @param depthBuf the depth target (not null) + * @throws IllegalArgumentException if the target is not a depth buffer, + * belongs to another framebuffer, or is already attached to another slot + */ public void setDepthTarget(FrameBufferBufferTarget depthBuf){ if (!depthBuf.getFormat().isDepthFormat()) throw new IllegalArgumentException("Depth buffer format must be depth."); + claimTarget(depthBuf, this.depthBuf); this.depthBuf = depthBuf; this.depthBuf.slot = this.depthBuf.getFormat().isDepthStencilFormat() ? SLOT_DEPTH_STENCIL : SLOT_DEPTH; setUpdateNeeded(); } + /** + * Sets an exclusively owned depth texture target. The texture itself may + * be shared through distinct target instances. + * + * @param depthBuf the depth target (not null) + * @throws IllegalArgumentException if the texture is not a valid depth + * target, the target belongs to another framebuffer, or it is already + * attached to another slot + */ public void setDepthTarget(FrameBufferTextureTarget depthBuf){ checkSetTexture(depthBuf.getTexture(), true); + claimTarget(depthBuf, this.depthBuf); this.depthBuf = depthBuf; this.depthBuf.slot = depthBuf.getTexture().getImage().getFormat().isDepthStencilFormat() ? SLOT_DEPTH_STENCIL : SLOT_DEPTH; setUpdateNeeded(); } + private void claimTarget(RenderBuffer target, RenderBuffer replaced) { + if (target == null) { + throw new IllegalArgumentException("Framebuffer target cannot be null."); + } + if (target.owner != null && target.owner != targetOwner) { + throw new IllegalArgumentException("Framebuffer target belongs to another framebuffer. " + + "Create a separate target for each framebuffer."); + } + if (target != replaced) { + if (target == depthBuf) { + throw new IllegalArgumentException("Framebuffer target is already attached."); + } + for (int i = 0; i < colorBufs.size(); i++) { + if (colorBufs.get(i) == target) { + throw new IllegalArgumentException("Framebuffer target is already attached."); + } + } + } + // Keep the identity after removal: native detachment/deletion is deferred. + target.owner = targetOwner; + } + public int getNumColorTargets(){ return colorBufs.size(); } diff --git a/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java b/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java index 0f976a9077..fece79c3f2 100644 --- a/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java +++ b/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java @@ -38,6 +38,7 @@ import java.util.ArrayDeque; import java.util.ArrayList; import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.logging.Level; import java.util.logging.Logger; @@ -80,6 +81,7 @@ public class NativeObjectManager { * List of real objects requested by user for deletion. */ private final ArrayDeque userDeletionQueue = new ArrayDeque<>(); + private final IdentityHashMap pendingUserDeletions = new IdentityHashMap<>(); private static class NativeObjectRef extends PhantomReference { @@ -127,7 +129,11 @@ public void unregisterObject(NativeObject obj) { if (ref != null) { ref.clear(); } - userDeletionQueue.removeIf(queued -> queued == obj); + // Managed deletion already removed the pending request before calling + // the renderer. Only a still-queued object needs this identity scan. + if (pendingUserDeletions.remove(obj) != null) { + userDeletionQueue.removeIf(queued -> queued == obj); + } } private void deleteNativeObject(Object rendererObject, NativeObject obj, NativeObjectRef ref, @@ -192,6 +198,7 @@ public void deleteUnused(Object rendererObject){ while (removed < MAX_REMOVES_PER_FRAME && !userDeletionQueue.isEmpty()) { // Remove user requested objects. NativeObject obj = userDeletionQueue.pop(); + pendingUserDeletions.remove(obj); deleteNativeObject(rendererObject, obj, null, true, true); removed++; } @@ -228,6 +235,9 @@ public void deleteAllObjects(Object rendererObject){ deleteNativeObject(rendererObject, ref.objClone, ref, true, false); } assert refMap.size() == 0; + // deleteUnused is bounded; the remaining objects were deleted via clones. + userDeletionQueue.clear(); + pendingUserDeletions.clear(); } /** @@ -240,7 +250,9 @@ public void deleteAllObjects(Object rendererObject){ * @param obj The object to mark as unused. */ void enqueueUnusedObject(NativeObject obj) { - userDeletionQueue.push(obj); + if (pendingUserDeletions.put(obj, Boolean.TRUE) == null) { + userDeletionQueue.push(obj); + } } /** @@ -260,6 +272,9 @@ public void resetObjects(){ } refMap.clear(); refQueue = new ReferenceQueue(); + // Queued deletions belong to the lost context, not to recreated objects. + userDeletionQueue.clear(); + pendingUserDeletions.clear(); } // public void printObjects(){ diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferAttachmentTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferAttachmentTest.java index b19ff5124e..577dd65590 100644 --- a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferAttachmentTest.java +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferAttachmentTest.java @@ -36,6 +36,7 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import com.jme3.renderer.Limits; import com.jme3.texture.FrameBuffer; import com.jme3.texture.Image; import com.jme3.texture.Texture; @@ -222,4 +223,24 @@ void explicitSrgbStorageChangeStillReallocatesInternalFormat() { assertNotEquals(oldFormat, driver.renderbufferFormats.get(id)); assertEquals(GLExt.GL_SRGB8_ALPHA8_EXT, driver.renderbufferFormats.get(id)); } + + @Test + void highestColorSlotDoesNotOverlapDepthStencilBookkeeping() { + renderer.getLimits().put(Limits.FrameBufferAttachments, 16); + FrameBuffer fb = bufferedFramebuffer(16); + fb.setDepthTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.Depth24Stencil8)); + renderer.setFrameBuffer(fb); + final int depthId = fb.getDepthTarget().getId(); + fb.removeColorTarget(15); + renderer.setFrameBuffer(fb); + assertEquals(0, driver.attachment(fb, 15)); + assertEquals(depthId, driver.attachmentAt(fb, GLFbo.GL_DEPTH_ATTACHMENT_EXT)); + assertEquals(depthId, driver.attachmentAt(fb, GL_STENCIL_ATTACHMENT)); + fb.setDepthTarget(FrameBuffer.FrameBufferTarget.newTarget(Image.Format.Depth)); + renderer.setFrameBuffer(fb); + assertEquals(0, driver.attachmentAt(fb, GL_STENCIL_ATTACHMENT)); + assertEquals(16, driver.liveRenderbuffers.size()); + renderer.cleanup(); + assertEquals(0, driver.liveRenderbuffers.size()); + } } diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java index b61d3a534a..d3ff47ea49 100644 --- a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleTest.java @@ -33,13 +33,16 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.jme3.renderer.RendererException; import com.jme3.texture.FrameBuffer; import com.jme3.texture.Image; +import com.jme3.texture.Texture2D; import java.lang.ref.Reference; +import java.lang.ref.WeakReference; import org.junit.jupiter.api.Test; /** Tests ownership, deletion, native-name reuse, and context reset. */ @@ -213,4 +216,157 @@ void textureAttachmentsAreNotDeletedAsRenderbufferNames() { assertTrue(driver.deletedRenderbuffers.isEmpty()); assertEquals(1, driver.liveTextures.size()); } + + @Test + void disposingOneFramebufferPreservesSharedColorAndDepthTextures() { + Texture2D color = texture(); + Texture2D depth = new Texture2D(8, 8, Image.Format.Depth); + FrameBuffer first = framebuffer(color); + FrameBuffer second = framebuffer(color); + first.setDepthTarget(FrameBuffer.FrameBufferTarget.newTarget(depth)); + second.setDepthTarget(FrameBuffer.FrameBufferTarget.newTarget(depth)); + renderer.setFrameBuffer(first); + renderer.setFrameBuffer(second); + final int colorId = color.getImage().getId(); + final int depthId = depth.getImage().getId(); + first.dispose(); + renderer.postFrame(); + assertEquals(colorId, driver.attachment(second, 0)); + assertEquals(depthId, driver.attachmentAt(second, GLFbo.GL_DEPTH_ATTACHMENT_EXT)); + assertEquals(2, driver.liveTextures.size()); + assertTrue(driver.deletedRenderbuffers.isEmpty()); + second.setUpdateNeeded(); + renderer.setFrameBuffer(second); + second.dispose(); + renderer.postFrame(); + assertTrue(driver.liveFramebuffers.isEmpty()); + assertEquals(2, driver.liveTextures.size()); + assertEquals(colorId, color.getImage().getId()); + assertEquals(depthId, depth.getImage().getId()); + } + + @Test + void disposalDoesNotAllowTargetTransferButOriginalFramebufferCanBeReused() { + FrameBuffer first = bufferedFramebuffer(1); + final FrameBuffer.FrameBufferBufferTarget target + = (FrameBuffer.FrameBufferBufferTarget) first.getColorTarget(0); + renderer.setFrameBuffer(first); + first.dispose(); + renderer.postFrame(); + FrameBuffer second = framebuffer(texture()); + assertThrows(IllegalArgumentException.class, () -> second.addColorTarget(target)); + renderer.setFrameBuffer(first); + assertEquals(target.getId(), driver.attachment(first, 0)); + renderer.cleanup(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + } + + @Test + void duplicateDisposalRequestsCannotDeleteAReinitializedFramebuffer() { + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + for (int i = 0; i < 150; i++) { + fb.dispose(); + } + renderer.postFrame(); + assertTrue(driver.liveFramebuffers.isEmpty()); + renderer.setFrameBuffer(fb); + int newId = fb.getId(); + renderer.postFrame(); + assertEquals(newId, fb.getId()); + assertTrue(driver.liveFramebuffers.contains(newId)); + assertEquals(2, driver.liveRenderbuffers.size()); + renderer.cleanup(); + } + + @Test + void batchDisposalStillHonorsThePerFrameLimit() { + for (int i = 0; i < 250; i++) { + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + fb.dispose(); + fb.dispose(); + } + renderer.postFrame(); + assertEquals(150, driver.liveFramebuffers.size()); + assertEquals(300, driver.liveRenderbuffers.size()); + renderer.postFrame(); + assertEquals(50, driver.liveFramebuffers.size()); + renderer.postFrame(); + assertTrue(driver.liveFramebuffers.isEmpty()); + assertTrue(driver.liveRenderbuffers.isEmpty()); + } + + @Test + void retainedTargetDoesNotPreventOwnerGcOrNativeCleanup() throws InterruptedException { + final FrameBuffer.FrameBufferBufferTarget target + = FrameBuffer.FrameBufferTarget.newTarget(Image.Format.RGBA8); + WeakReference owner = initializeCollectibleFramebuffer(target); + try { + // Keep the target strongly reachable, but not its owning framebuffer. + for (int i = 0; i < 100 && !driver.liveFramebuffers.isEmpty(); i++) { + System.gc(); + Thread.sleep(25); + renderer.postFrame(); + } + assertNull(owner.get(), "The target must not retain its owner"); + assertTrue(driver.liveFramebuffers.isEmpty(), "The phantom handle must also be collectible"); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertEquals(-1, target.getId()); + } finally { + Reference.reachabilityFence(target); + } + } + + private WeakReference initializeCollectibleFramebuffer( + FrameBuffer.FrameBufferBufferTarget target) { + FrameBuffer fb = new FrameBuffer(8, 8, 1); + fb.addColorTarget(target); + renderer.setFrameBuffer(fb); + renderer.setFrameBuffer(null); + return new WeakReference<>(fb); + } + + @Test + void contextResetDiscardsPendingDisposalOfOldNativeNames() { + FrameBuffer fb = bufferedFramebuffer(1); + renderer.setFrameBuffer(fb); + fb.dispose(); + renderer.resetGLObjects(); + driver.liveRenderbuffers.clear(); + driver.liveFramebuffers.clear(); + driver.attachments.clear(); + driver.boundRenderbuffer = 0; + renderer.setFrameBuffer(fb); + int newId = fb.getId(); + renderer.postFrame(); + assertEquals(newId, fb.getId()); + assertTrue(driver.liveFramebuffers.contains(newId)); + assertEquals(2, driver.liveRenderbuffers.size()); + fb.dispose(); + renderer.postFrame(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + } + + @Test + void fullCleanupDiscardsRequestsBeyondThePerFrameLimit() { + FrameBuffer[] buffers = new FrameBuffer[150]; + for (int i = 0; i < buffers.length; i++) { + buffers[i] = bufferedFramebuffer(1); + renderer.setFrameBuffer(buffers[i]); + buffers[i].dispose(); + } + renderer.cleanup(); + assertTrue(driver.liveFramebuffers.isEmpty()); + assertTrue(driver.liveRenderbuffers.isEmpty()); + FrameBuffer reused = buffers[0]; + renderer.setFrameBuffer(reused); + int newId = reused.getId(); + renderer.postFrame(); + assertEquals(newId, reused.getId()); + assertTrue(driver.liveFramebuffers.contains(newId)); + reused.dispose(); + renderer.postFrame(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + } } diff --git a/jme3-core/src/test/java/com/jme3/texture/FrameBufferTargetOwnershipTest.java b/jme3-core/src/test/java/com/jme3/texture/FrameBufferTargetOwnershipTest.java new file mode 100644 index 0000000000..1fc2d332b5 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/texture/FrameBufferTargetOwnershipTest.java @@ -0,0 +1,240 @@ +/* + * 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.texture; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.jme3.texture.FrameBuffer.FrameBufferBufferTarget; +import com.jme3.texture.FrameBuffer.FrameBufferTarget; +import com.jme3.texture.FrameBuffer.FrameBufferTextureTarget; +import org.junit.jupiter.api.Test; + +/** Target instances are exclusive; underlying textures may be shared. */ +public class FrameBufferTargetOwnershipTest { + private static FrameBuffer framebuffer() { + return new FrameBuffer(8, 8, 1); + } + + private static FrameBufferTextureTarget colorTarget() { + return FrameBufferTarget.newTarget(new Texture2D(8, 8, Image.Format.RGBA8)); + } + + @Test + void colorRenderbufferCannotBeSharedAndFailureDoesNotChangeItsSlot() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferBufferTarget target = FrameBufferTarget.newTarget(Image.Format.RGBA8); + first.addColorTarget(target); + second.addColorTarget(colorTarget()); + second.clearUpdateNeeded(); + assertThrows(IllegalArgumentException.class, () -> second.addColorTarget(target)); + assertEquals(0, target.getSlot()); + assertEquals(1, second.getNumColorTargets()); + assertSame(target, first.getColorTarget(0)); + assertFalse(second.isUpdateNeeded()); + } + + @Test + void colorTextureTargetCannotBeShared() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferTextureTarget target = colorTarget(); + first.addColorTarget(target); + assertThrows(IllegalArgumentException.class, () -> second.addColorTarget(target)); + assertEquals(0, second.getNumColorTargets()); + } + + @Test + void depthRenderbufferCannotBeShared() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferBufferTarget target = FrameBufferTarget.newTarget(Image.Format.Depth); + first.setDepthTarget(target); + assertThrows(IllegalArgumentException.class, () -> second.setDepthTarget(target)); + assertNull(second.getDepthTarget()); + assertSame(target, first.getDepthTarget()); + } + + @Test + void depthTextureTargetCannotBeShared() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferTextureTarget target = FrameBufferTarget.newTarget( + new Texture2D(8, 8, Image.Format.Depth)); + first.setDepthTarget(target); + assertThrows(IllegalArgumentException.class, () -> second.setDepthTarget(target)); + assertNull(second.getDepthTarget()); + } + + @Test + void rejectedReplacementPreservesBothFramebuffers() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferTextureTarget shared = colorTarget(); + FrameBufferTextureTarget original = colorTarget(); + first.addColorTarget(shared); + second.addColorTarget(colorTarget()); + second.addColorTarget(original); + second.clearUpdateNeeded(); + assertThrows(IllegalArgumentException.class, () -> second.replaceColorTarget(1, shared)); + assertSame(original, second.getColorTarget(1)); + assertEquals(0, shared.getSlot()); + assertFalse(second.isUpdateNeeded()); + } + + @Test + void cubemapOverloadRejectsSharingBeforeChangingFaceOrSlot() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferTextureTarget target = FrameBufferTarget.newTarget( + new TextureCubeMap(8, 8, Image.Format.RGBA8)); + first.addColorTarget(target, TextureCubeMap.Face.PositiveX); + second.addColorTarget(colorTarget()); + assertThrows(IllegalArgumentException.class, + () -> second.addColorTarget(target, TextureCubeMap.Face.NegativeX)); + assertEquals(TextureCubeMap.Face.PositiveX.ordinal(), target.getFace()); + assertEquals(0, target.getSlot()); + assertEquals(1, second.getNumColorTargets()); + } + + @Test + void duplicateRenderbufferSlotsAreRejected() { + FrameBuffer fb = framebuffer(); + FrameBufferBufferTarget target = FrameBufferTarget.newTarget(Image.Format.RGBA8); + fb.addColorTarget(target); + assertThrows(IllegalArgumentException.class, () -> fb.addColorTarget(target)); + assertEquals(1, fb.getNumColorTargets()); + assertEquals(0, target.getSlot()); + } + + @Test + void duplicateTextureSlotsAreRejectedIncludingReplacement() { + FrameBuffer fb = framebuffer(); + FrameBufferTextureTarget first = colorTarget(); + FrameBufferTextureTarget second = colorTarget(); + fb.addColorTarget(first); + fb.addColorTarget(second); + assertThrows(IllegalArgumentException.class, () -> fb.addColorTarget(first)); + assertThrows(IllegalArgumentException.class, () -> fb.replaceColorTarget(1, first)); + assertSame(second, fb.getColorTarget(1)); + assertEquals(0, first.getSlot()); + } + + @Test + void sameTargetCannotOccupyColorAndDepthSlots() { + FrameBuffer fb = framebuffer(); + FrameBufferBufferTarget target = FrameBufferTarget.newTarget(Image.Format.Depth); + fb.setDepthTarget(target); + assertThrows(IllegalArgumentException.class, () -> fb.addColorTarget(target)); + assertEquals(FrameBuffer.SLOT_DEPTH, target.getSlot()); + } + + @Test + void settingExistingTargetAgainIsAllowed() { + FrameBuffer fb = framebuffer(); + FrameBufferBufferTarget depth = FrameBufferTarget.newTarget(Image.Format.Depth); + FrameBufferTextureTarget color = colorTarget(); + fb.setDepthTarget(depth); + fb.addColorTarget(color); + assertDoesNotThrow(() -> fb.setDepthTarget(depth)); + assertDoesNotThrow(() -> fb.replaceColorTarget(0, color)); + assertSame(depth, fb.getDepthTarget()); + assertSame(color, fb.getColorTarget(0)); + } + + @Test + void removedTargetCanReturnToOriginalFramebufferButCannotBeTransferred() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferBufferTarget target = FrameBufferTarget.newTarget(Image.Format.RGBA8); + first.addColorTarget(target); + first.removeColorTarget(0); + assertThrows(IllegalArgumentException.class, () -> second.addColorTarget(target)); + assertDoesNotThrow(() -> first.addColorTarget(target)); + first.clearColorTargets(); + assertThrows(IllegalArgumentException.class, () -> second.addColorTarget(target)); + assertDoesNotThrow(() -> first.addColorTarget(target)); + } + + @Test + void replacedTextureTargetKeepsOriginalOwner() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferTextureTarget target = colorTarget(); + first.addColorTarget(target); + first.replaceColorTarget(0, colorTarget()); + assertThrows(IllegalArgumentException.class, () -> second.addColorTarget(target)); + assertDoesNotThrow(() -> first.replaceColorTarget(0, target)); + } + + @Test + void replacedDepthTargetKeepsOriginalOwner() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferBufferTarget target = FrameBufferTarget.newTarget(Image.Format.Depth); + first.setDepthTarget(target); + first.setDepthTarget(FrameBufferTarget.newTarget(Image.Format.Depth)); + assertThrows(IllegalArgumentException.class, () -> second.setDepthTarget(target)); + assertDoesNotThrow(() -> first.setDepthTarget(target)); + } + + @Test + void failedValidationDoesNotClaimTarget() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + FrameBufferBufferTarget color = FrameBufferTarget.newTarget(Image.Format.RGBA8); + assertThrows(IllegalArgumentException.class, () -> first.setDepthTarget(color)); + assertDoesNotThrow(() -> second.addColorTarget(color)); + FrameBufferTextureTarget texture = colorTarget(); + assertThrows(IndexOutOfBoundsException.class, () -> first.replaceColorTarget(0, texture)); + assertDoesNotThrow(() -> second.addColorTarget(texture)); + } + + @Test + void separateTargetsCanShareColorAndDepthTextures() { + FrameBuffer first = framebuffer(); + FrameBuffer second = framebuffer(); + Texture2D color = new Texture2D(8, 8, Image.Format.RGBA8); + Texture2D depth = new Texture2D(8, 8, Image.Format.Depth); + first.addColorTarget(FrameBufferTarget.newTarget(color)); + second.addColorTarget(FrameBufferTarget.newTarget(color)); + first.setDepthTarget(FrameBufferTarget.newTarget(depth)); + second.setDepthTarget(FrameBufferTarget.newTarget(depth)); + assertSame(first.getColorTarget(0).getTexture(), second.getColorTarget(0).getTexture()); + assertSame(first.getDepthTarget().getTexture(), second.getDepthTarget().getTexture()); + } +} From f0c3d4b2597473ca6e0671afd3dbbed69cfc07af Mon Sep 17 00:00:00 2001 From: toaster0123 <261194501+toaster0123@users.noreply.github.com> Date: Mon, 26 Oct 2026 15:56:44 +0000 Subject: [PATCH 4/6] Fix framebuffer snapshots and preserve pending native-buffer cleanup Snapshot the actual attached Image and native name per color slot, and invalidate snapshots and texture-unit bindings when an image is deleted. Avoid mipmap generation for deleted or recreated images, including native name reuse, while retaining the rendered image across Texture.setImage. Keep untouched attachment snapshots on partial update failures. Reconcile public attachment/texture update helpers through the full framebuffer update path, including initialization, binding and ownership validation. Retain allocation-free snapshot reuse for stable slots. Drain pending CPU-buffer disposal on context reset and full cleanup when UNSAFE is enabled, without deleting old GL names in a new context. Preserve queued CPU cleanup on direct unregistration without double-freeing during managed deletion. Keep ordinary per-frame deletion limits and deduplication. Treat invalidated framebuffer binding as unknown and restore the discovered presentation framebuffer for binding, copies and main overrides. Add 26 regressions. Validation: core 642 passed / 1 existing skipped; effects 6 passed. All 32 targeted cases pass with Java assertions disabled. Core main/test Checkstyle and git diff --check completed, with no warnings on changed lines. Recording-GL tests only; no native GPU/FPS claim. --- .../com/jme3/renderer/opengl/GLRenderer.java | 132 +++++++-- .../com/jme3/util/NativeObjectManager.java | 27 +- .../opengl/FrameBufferTestSupport.java | 15 +- .../GlRendererDefaultFrameBufferTest.java | 32 ++ .../GlRendererFrameBufferSnapshotTest.java | 277 +++++++++++++++++ .../util/NativeObjectManagerDisposalTest.java | 279 ++++++++++++++++++ 6 files changed, 735 insertions(+), 27 deletions(-) create mode 100644 jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferSnapshotTest.java create mode 100644 jme3-core/src/test/java/com/jme3/util/NativeObjectManagerDisposalTest.java 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 index 5da3315a7c..21a7805361 100644 --- a/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java +++ b/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java @@ -114,12 +114,55 @@ private static final class RenderBufferState { } } + /** Snapshot of the image actually attached, independent of Texture.setImage(). */ + private static final class ColorAttachmentState { + Texture texture; + Image image; + int imageId; + } + /** Native attachment state, without a reference to the owning FrameBuffer. */ private static final class FrameBufferState { // Bits 0..15 are color slots; 16 and 17 are depth and depth-stencil. int attachmentSlots; final ArrayList renderBuffers = new ArrayList<>(); - final ArrayList colorTextures = new ArrayList<>(); + final ColorAttachmentState[] colorTextures = new ColorAttachmentState[16]; + int colorTextureSlots; + + void setColorTexture(int slot, Texture texture) { + if (slot < 0) { + return; + } + ColorAttachmentState attachment = colorTextures[slot]; + if (texture == null) { + colorTextureSlots &= ~(1 << slot); + if (attachment != null) { + attachment.texture = null; + attachment.image = null; + attachment.imageId = NativeObject.INVALID_ID; + } + } else { + if (attachment == null) { + attachment = new ColorAttachmentState(); + colorTextures[slot] = attachment; + } + attachment.texture = texture; + attachment.image = texture.getImage(); + attachment.imageId = attachment.image.getId(); + colorTextureSlots |= 1 << slot; + } + } + + void invalidateImage(int imageId) { + int slots = colorTextureSlots; + while (slots != 0) { + int slot = Integer.numberOfTrailingZeros(slots); + slots &= slots - 1; + if (colorTextures[slot].imageId == imageId) { + setColorTexture(slot, null); + } + } + } RenderBufferState getRenderBuffer(RenderBuffer target) { for (int i = 0; i < renderBuffers.size(); i++) { @@ -939,6 +982,7 @@ public void initialize() { @Override public void invalidateState() { context.reset(); + context.boundFBO = -1; if (gl2 != null) { context.initialDrawBuf = getInteger(GL2.GL_DRAW_BUFFER); context.initialReadBuf = getInteger(GL2.GL_READ_BUFFER); @@ -2055,7 +2099,7 @@ public void copyFrameBuffer(FrameBuffer src, FrameBuffer dst, boolean copyColor, int dstX1; int dstY1; - int prevFBO = context.boundFBO; + final int previousFramebuffer = context.boundFBO < 0 ? defaultFBO : context.boundFBO; FrameBuffer prevFB = context.boundFB; if (mainFbOverride != null) { @@ -2115,8 +2159,8 @@ public void copyFrameBuffer(FrameBuffer src, FrameBuffer dst, boolean copyColor, GL.GL_NEAREST); - glfbo.glBindFramebufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, prevFBO); - context.boundFBO = prevFBO; + glfbo.glBindFramebufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, previousFramebuffer); + context.boundFBO = previousFramebuffer; context.boundFB = prevFB; toggleFramebufferSrgb(prevFB); } else { @@ -2218,7 +2262,23 @@ private int convertAttachmentSlot(int attachmentSlot) { return GLFbo.GL_COLOR_ATTACHMENT0_EXT + attachmentSlot; } + /** + * Synchronizes and binds the framebuffer containing this texture target. + * All pending attachment changes are applied, as in + * {@link #updateFrameBufferAttachment(FrameBuffer, RenderBuffer)}. + * + * @param fb the framebuffer (not null) + * @param rb one of its current texture targets (not null) + * @throws IllegalArgumentException if the target is not a texture target of {@code fb} + */ public void updateRenderTexture(FrameBuffer fb, RenderBuffer rb) { + if (rb == null || rb.getTexture() == null) { + throw new IllegalArgumentException("A texture target is required."); + } + updateFrameBufferAttachment(fb, rb); + } + + private void updateRenderTextureInternal(FrameBuffer fb, RenderBuffer rb) { Texture tex = rb.getTexture(); Image image = tex.getImage(); if (image.isUpdateNeeded()) { @@ -2248,8 +2308,21 @@ public void updateRenderTexture(FrameBuffer fb, RenderBuffer rb) { } } + /** + * Synchronizes and binds the framebuffer containing this target. Reconciles + * all pending target changes so native attachments and their cleanup/mipmap + * snapshots cannot diverge. An uninitialized framebuffer is initialized first. + * + * @param fb the framebuffer (not null) + * @param rb one of its current targets (not null) + * @throws IllegalArgumentException if the target is not attached to {@code fb} + */ public void updateFrameBufferAttachment(FrameBuffer fb, RenderBuffer rb) { - updateFrameBufferAttachment(fb, rb, frameBufferStates.get(fb.getId())); + if (fb == null || rb == null || !isFrameBufferTarget(fb, rb)) { + throw new IllegalArgumentException("The target must belong to this framebuffer."); + } + fb.setUpdateNeeded(); + setFrameBuffer(fb); } private void updateFrameBufferAttachment(FrameBuffer fb, RenderBuffer rb, FrameBufferState state) { @@ -2274,9 +2347,10 @@ private void updateFrameBufferAttachment(FrameBuffer fb, RenderBuffer rb, FrameB GLFbo.GL_RENDERBUFFER_EXT, rb.getId()); } else { - updateRenderTexture(fb, rb); + updateRenderTextureInternal(fb, rb); } state.attachmentSlots |= attachmentSlotMask(rb.getSlot()); + state.setColorTexture(rb.getSlot(), rb.getTexture()); } private void bindFrameBuffer(FrameBuffer fb) { @@ -2348,6 +2422,7 @@ public void updateFrameBuffer(FrameBuffer fb) { glfbo.glFramebufferRenderbufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, convertAttachmentSlot(slot), GLFbo.GL_RENDERBUFFER_EXT, 0); state.attachmentSlots &= ~(1 << index); + state.setColorTexture(slot, null); removedSlots &= removedSlots - 1; } for (int i = state.renderBuffers.size() - 1; i >= 0; i--) { @@ -2357,8 +2432,8 @@ public void updateFrameBuffer(FrameBuffer fb) { state.renderBuffers.remove(i); } } - state.colorTextures.clear(); - + // Successful attachments update their own snapshot. Preserve untouched + // slots if a later upload/attachment fails part-way through this update. FrameBuffer.RenderBuffer depthBuf = fb.getDepthTarget(); if (depthBuf != null) { updateFrameBufferAttachment(fb, depthBuf, state); @@ -2367,9 +2442,6 @@ public void updateFrameBuffer(FrameBuffer fb) { for (int i = 0; i < fb.getNumColorTargets(); i++) { FrameBuffer.RenderBuffer colorBuf = fb.getColorTarget(i); updateFrameBufferAttachment(fb, colorBuf, state); - if (colorBuf.getTexture() != null) { - state.colorTextures.add(colorBuf.getTexture()); - } } setReadDrawBuffers(fb); @@ -2421,7 +2493,7 @@ public Vector2f[] getFrameBufferSamplePositions(FrameBuffer fb) { @Override public void setMainFrameBufferOverride(FrameBuffer fb) { mainFbOverride = null; - if (context.boundFBO == defaultFBO) { + if (context.boundFB == null) { // Main FB is now set to fb, make sure its bound setFrameBuffer(fb); } @@ -2495,7 +2567,8 @@ public void setFrameBuffer(FrameBuffer fb) { fb = mainFbOverride; } - if (context.boundFB == fb) { + if (context.boundFB == fb + && context.boundFBO == (fb == null ? defaultFBO : fb.getId())) { if (fb == null || !fb.isUpdateNeeded()) { toggleFramebufferSrgb(fb); return; @@ -2515,13 +2588,21 @@ public void setFrameBuffer(FrameBuffer fb) { // Use the native attachment snapshot: the Java targets may already // have been replaced with textures that have not been allocated yet. FrameBufferState state = frameBufferStates.get(boundFB.getId()); - for (int i = 0; i < state.colorTextures.size(); i++) { - Texture tex = state.colorTextures.get(i); - if (tex != null && tex.getMinFilter().usesMipMapLevels() - && isMipmapGenerationSupported(tex.getImage().getFormat(), + int textureSlots = state == null ? 0 : state.colorTextureSlots; + while (textureSlots != 0) { + int slot = Integer.numberOfTrailingZeros(textureSlots); + textureSlots &= textureSlots - 1; + ColorAttachmentState attachment = state.colorTextures[slot]; + Texture tex = attachment.texture; + Image image = attachment.image; + if (image.getId() <= 0 || image.getId() != attachment.imageId) { + state.setColorTexture(slot, null); + continue; + } + if (tex.getMinFilter().usesMipMapLevels() + && isMipmapGenerationSupported(image.getFormat(), linearizeSrgbImages && boundFB.isSrgb() ? ColorSpace.sRGB : ColorSpace.Linear)) { - Image image = tex.getImage(); if (image.getMultiSamples() > 1) { throw new RendererException("Multisample textures do not support mipmaps"); } @@ -2541,7 +2622,7 @@ && isMipmapGenerationSupported(tex.getImage().getFormat(), image.setMipmapsGenerated(true); } else if (tex != null && tex.getMinFilter().usesMipMapLevels()) { logger.warning("Cannot generate mipmaps for framebuffer texture: " + tex - + " with image format: " + tex.getImage().getFormat()); + + " with image format: " + image.getFormat()); } } } @@ -3343,6 +3424,19 @@ public void deleteImage(Image image) { intBuf1.put(0, texId); intBuf1.position(0).limit(1); gl.glDeleteTextures(intBuf1); + // Clear native-name snapshots before the driver can reuse this name, + // even if the same Image is uploaded again before the next FBO bind. + for (IntMap.Entry entry : frameBufferStates) { + entry.getValue().invalidateImage(texId); + } + for (int unit = 0; unit < context.boundTextures.length; unit++) { + WeakReference reference = context.boundTextures[unit]; + Image boundImage = reference == null ? null : reference.get(); + if (boundImage != null && boundImage.getId() == texId) { + context.boundTextures[unit] = null; + } + } + objManager.unregisterObject(image); image.resetObject(); statistics.onDeleteTexture(); diff --git a/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java b/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java index fece79c3f2..fe95cbe8cd 100644 --- a/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java +++ b/jme3-core/src/main/java/com/jme3/util/NativeObjectManager.java @@ -133,6 +133,11 @@ public void unregisterObject(NativeObject obj) { // the renderer. Only a still-queued object needs this identity scan. if (pendingUserDeletions.remove(obj) != null) { userDeletionQueue.removeIf(queued -> queued == obj); + // The renderer deleted the native name, not the CPU buffers that + // the pending dispose() explicitly requested us to release. + if (UNSAFE) { + obj.deleteNativeBuffersInternal(); + } } } @@ -235,9 +240,9 @@ public void deleteAllObjects(Object rendererObject){ deleteNativeObject(rendererObject, ref.objClone, ref, true, false); } assert refMap.size() == 0; - // deleteUnused is bounded; the remaining objects were deleted via clones. - userDeletionQueue.clear(); - pendingUserDeletions.clear(); + // deleteUnused is bounded; the remaining GL objects were deleted via + // clones, but queued requests still own their CPU-buffer cleanup. + finishPendingDeletions(); } /** @@ -272,9 +277,19 @@ public void resetObjects(){ } refMap.clear(); refQueue = new ReferenceQueue(); - // Queued deletions belong to the lost context, not to recreated objects. - userDeletionQueue.clear(); - pendingUserDeletions.clear(); + // Old native names must never be deleted in the new context. Losing + // the context does not, however, release explicitly disposed CPU buffers. + finishPendingDeletions(); + } + + private void finishPendingDeletions() { + while (!userDeletionQueue.isEmpty()) { + NativeObject obj = userDeletionQueue.pop(); + pendingUserDeletions.remove(obj); + if (UNSAFE) { + obj.deleteNativeBuffersInternal(); + } + } } // public void printObjects(){ diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java index fc214e50cb..62bad75088 100644 --- a/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java @@ -116,6 +116,7 @@ static final class RecordingGL implements InvocationHandler { int nextObjectId = 100; int reuseRenderbufferId = -1; int reuseFramebufferId = -1; + int reuseTextureId = -1; int boundRenderbuffer; int drawFramebuffer; int readFramebuffer; @@ -124,6 +125,7 @@ static final class RecordingGL implements InvocationHandler { int activeTextureUnit; int framebufferStatus = GLFbo.GL_FRAMEBUFFER_COMPLETE_EXT; boolean failNextStorage; + int failTextureAttachmentSlot = -1; RecordingGL(int defaultFramebuffer) { this.defaultFramebuffer = defaultFramebuffer; @@ -152,12 +154,15 @@ public Object invoke(Object proxy, Method method, Object[] args) { ((IntBuffer) args[1]).put(0, getInteger((Integer) args[0])); return null; case "glGenTextures": - int texture = nextObjectId++; + int texture = reuseTextureId >= 0 ? reuseTextureId : nextObjectId++; + reuseTextureId = -1; liveTextures.add(texture); ((IntBuffer) args[0]).put(0, texture); return null; case "glDeleteTextures": - liveTextures.remove(((IntBuffer) args[0]).get(0)); + int deletedTexture = ((IntBuffer) args[0]).get(0); + liveTextures.remove(deletedTexture); + boundTextures.replaceAll((unit, name) -> name == deletedTexture ? 0 : name); return null; case "glActiveTexture": activeTextureUnit = (Integer) args[0] - GL.GL_TEXTURE0; @@ -218,6 +223,7 @@ public Object invoke(Object proxy, Method method, Object[] args) { case "glBindFramebufferEXT": int target = (Integer) args[0]; int id = (Integer) args[1]; + assertTrue(id >= 0, "Cannot bind an invalid framebuffer name"); if (target == GLFbo.GL_FRAMEBUFFER_EXT || target == GLFbo.GL_DRAW_FRAMEBUFFER_EXT) { drawFramebuffer = id; } @@ -229,6 +235,11 @@ public Object invoke(Object proxy, Method method, Object[] args) { case "glFramebufferRenderbufferEXT": int slot = (Integer) args[1]; int attachment = (Integer) args[3]; + if (method.getName().equals("glFramebufferTexture2DEXT") + && slot == GLFbo.GL_COLOR_ATTACHMENT0_EXT + failTextureAttachmentSlot) { + failTextureAttachmentSlot = -1; + throw new RendererException("Simulated texture attachment failure"); + } if (slot == GL3.GL_DEPTH_STENCIL_ATTACHMENT) { attachments.get(drawFramebuffer).put(GLFbo.GL_DEPTH_ATTACHMENT_EXT, attachment); attachments.get(drawFramebuffer).put(GL_STENCIL_ATTACHMENT, attachment); diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererDefaultFrameBufferTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererDefaultFrameBufferTest.java index 7956a67b82..817b405294 100644 --- a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererDefaultFrameBufferTest.java +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererDefaultFrameBufferTest.java @@ -105,4 +105,36 @@ void mainOverrideIsBoundWhenDiscoveredDefaultIsCurrent() { assertEquals(override.getId(), driver.blitReadFramebuffer); assertEquals(override.getId(), driver.blitDrawFramebuffer); } + + @Test + void overrideAfterInvalidationBindsNonzeroDefaultReplacement() { + initializeRenderer(77); + renderer.invalidateState(); + FrameBuffer override = framebuffer(texture()); + renderer.setMainFrameBufferOverride(override); + assertEquals(override.getId(), driver.drawFramebuffer); + assertEquals(override.getId(), driver.readFramebuffer); + } + + @Test + void screenBindAfterInvalidationRestoresDiscoveredDefault() { + initializeRenderer(77); + renderer.setFrameBuffer(framebuffer(texture())); + renderer.invalidateState(); + renderer.setFrameBuffer(null); + assertEquals(77, driver.drawFramebuffer); + assertEquals(77, driver.readFramebuffer); + } + + @Test + void copyAfterInvalidationRestoresDiscoveredDefault() { + initializeRenderer(77); + FrameBuffer fb = framebuffer(texture()); + renderer.setFrameBuffer(fb); + renderer.invalidateState(); + renderer.copyFrameBuffer(fb, null, true, false); + assertEquals(77, driver.blitDrawFramebuffer); + assertEquals(77, driver.drawFramebuffer); + assertEquals(77, driver.readFramebuffer); + } } diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferSnapshotTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferSnapshotTest.java new file mode 100644 index 0000000000..458b173620 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferSnapshotTest.java @@ -0,0 +1,277 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.jme3.renderer.RendererException; +import com.jme3.texture.FrameBuffer; +import com.jme3.texture.FrameBuffer.FrameBufferTarget; +import com.jme3.texture.Image; +import com.jme3.texture.Texture; +import com.jme3.texture.Texture2D; +import com.jme3.texture.image.ColorSpace; +import com.jme3.util.NativeObjectManager; +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.Collections; +import org.junit.jupiter.api.Test; + +/** Tests native attachment snapshots across deletion and low-level updates. */ +public class GlRendererFrameBufferSnapshotTest extends FrameBufferTestSupport { + private static Texture2D mipmappedTexture() { + Texture2D texture = texture(); + texture.setMinFilter(Texture.MinFilter.Trilinear); + return texture; + } + + @Test + void replacementThenDirectImageDeletionSkipsOldMipmaps() { + Texture2D old = mipmappedTexture(); + Texture2D replacement = texture(); + FrameBuffer fb = framebuffer(old); + renderer.setFrameBuffer(fb); + fb.replaceColorTarget(0, FrameBufferTarget.newTarget(replacement)); + renderer.deleteImage(old.getImage()); + assertDoesNotThrow(() -> renderer.setFrameBuffer(fb)); + assertEquals(replacement.getImage().getId(), driver.attachment(fb, 0)); + assertTrue(driver.mipmapGenerations.isEmpty()); + assertDoesNotThrow(() -> renderer.cleanup()); + } + + @Test + void replacementThenQueuedImageDisposalSkipsOldMipmaps() { + Texture2D old = mipmappedTexture(); + Texture2D replacement = texture(); + FrameBuffer fb = framebuffer(old); + renderer.setFrameBuffer(fb); + fb.replaceColorTarget(0, FrameBufferTarget.newTarget(replacement)); + old.getImage().dispose(); + renderer.postFrame(); + assertDoesNotThrow(() -> renderer.setFrameBuffer(fb)); + assertEquals(replacement.getImage().getId(), driver.attachment(fb, 0)); + assertTrue(driver.mipmapGenerations.isEmpty()); + renderer.cleanup(); + } + + @Test + void deletedImageRecreatedWithSameNameCannotReviveOldSnapshot() throws Exception { + Texture2D texture = mipmappedTexture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + final int originalId = texture.getImage().getId(); + texture.getImage().dispose(); + renderer.postFrame(); + driver.reuseTextureId = originalId; + renderer.setTexture(0, texture); + assertEquals(originalId, texture.getImage().getId()); + renderer.setFrameBuffer(null); + assertTrue(driver.mipmapGenerations.isEmpty(), "The recreated image was not rendered into"); + fb.setUpdateNeeded(); + renderer.setFrameBuffer(fb); + renderer.setFrameBuffer(null); + assertEquals(Collections.singletonList(originalId), driver.mipmapGenerations); + renderer.cleanup(); + } + + @Test + void textureImageMutationUsesImageActuallyAttached() { + Texture2D texture = mipmappedTexture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + final int originalId = texture.getImage().getId(); + Image replacement = texture().getImage(); + texture.setImage(replacement); + renderer.setFrameBuffer(null); + assertEquals(Collections.singletonList(originalId), driver.mipmapGenerations); + assertEquals(-1, replacement.getId()); + renderer.cleanup(); + } + + @Test + void directAttachmentUpdateRefreshesMipmapSnapshot() { + Texture2D replacement = mipmappedTexture(); + FrameBuffer fb = framebuffer(mipmappedTexture()); + renderer.setFrameBuffer(fb); + fb.replaceColorTarget(0, FrameBufferTarget.newTarget(replacement)); + renderer.updateFrameBufferAttachment(fb, fb.getColorTarget(0)); + assertEquals(replacement.getImage().getId(), driver.attachment(fb, 0)); + driver.mipmapGenerations.clear(); + renderer.setFrameBuffer(null); + assertEquals(Collections.singletonList(replacement.getImage().getId()), driver.mipmapGenerations); + renderer.cleanup(); + } + + @Test + void directRenderTextureUpdateRefreshesMipmapSnapshot() { + Texture2D replacement = mipmappedTexture(); + FrameBuffer fb = framebuffer(mipmappedTexture()); + renderer.setFrameBuffer(fb); + fb.replaceColorTarget(0, FrameBufferTarget.newTarget(replacement)); + renderer.updateRenderTexture(fb, fb.getColorTarget(0)); + assertEquals(replacement.getImage().getId(), driver.attachment(fb, 0)); + driver.mipmapGenerations.clear(); + renderer.setFrameBuffer(null); + assertEquals(Collections.singletonList(replacement.getImage().getId()), driver.mipmapGenerations); + renderer.cleanup(); + } + + @Test + void directAttachmentUpdateInitializesAndBindsNewFramebuffer() { + FrameBuffer previous = framebuffer(texture()); + renderer.setFrameBuffer(previous); + final int previousTexture = driver.attachment(previous, 0); + Texture2D texture = mipmappedTexture(); + FrameBuffer fb = framebuffer(texture); + renderer.updateFrameBufferAttachment(fb, fb.getColorTarget(0)); + assertTrue(fb.getId() > 0); + assertEquals(fb.getId(), driver.drawFramebuffer); + assertEquals(previousTexture, driver.attachment(previous, 0)); + assertEquals(texture.getImage().getId(), driver.attachment(fb, 0)); + renderer.setFrameBuffer(null); + assertEquals(Collections.singletonList(texture.getImage().getId()), driver.mipmapGenerations); + renderer.cleanup(); + } + + @Test + void directTextureUpdateInitializesNewFramebuffer() { + Texture2D texture = mipmappedTexture(); + FrameBuffer fb = framebuffer(texture); + renderer.updateRenderTexture(fb, fb.getColorTarget(0)); + assertEquals(fb.getId(), driver.drawFramebuffer); + assertEquals(texture.getImage().getId(), driver.attachment(fb, 0)); + renderer.setFrameBuffer(null); + assertEquals(Collections.singletonList(texture.getImage().getId()), driver.mipmapGenerations); + renderer.cleanup(); + } + + @Test + void directUpdateRejectsForeignTargetBeforeTouchingNativeState() { + FrameBuffer first = framebuffer(texture()); + FrameBuffer second = framebuffer(texture()); + renderer.setFrameBuffer(first); + int attachment = driver.attachment(first, 0); + assertThrows(IllegalArgumentException.class, + () -> renderer.updateFrameBufferAttachment(first, second.getColorTarget(0))); + assertEquals(attachment, driver.attachment(first, 0)); + assertEquals(-1, second.getId()); + renderer.cleanup(); + } + + @Test + void renderTextureUpdateRejectsRenderbufferBeforeTouchingNativeState() { + FrameBuffer fb = bufferedFramebuffer(1); + assertThrows(IllegalArgumentException.class, + () -> renderer.updateRenderTexture(fb, fb.getColorTarget(0))); + assertEquals(-1, fb.getId()); + assertTrue(driver.liveFramebuffers.isEmpty()); + } + + @Test + void directUpdateReconcilesRemovedSlotsToo() { + FrameBuffer fb = framebuffer(mipmappedTexture(), mipmappedTexture()); + renderer.setFrameBuffer(fb); + fb.removeColorTarget(1); + renderer.updateFrameBufferAttachment(fb, fb.getColorTarget(0)); + driver.mipmapGenerations.clear(); + renderer.setFrameBuffer(null); + assertEquals(0, driver.attachment(fb, 1)); + assertEquals(Collections.singletonList(fb.getColorTarget(0).getTexture().getImage().getId()), + driver.mipmapGenerations); + renderer.cleanup(); + } + + @Test + void deletingSharedImageInvalidatesEveryFramebufferSnapshot() { + Texture2D shared = mipmappedTexture(); + FrameBuffer first = framebuffer(shared); + FrameBuffer second = framebuffer(shared); + renderer.setFrameBuffer(first); + renderer.setFrameBuffer(second); + renderer.deleteImage(shared.getImage()); + driver.mipmapGenerations.clear(); + renderer.setFrameBuffer(null); + renderer.setFrameBuffer(first); + renderer.setFrameBuffer(null); + assertTrue(driver.mipmapGenerations.isEmpty()); + renderer.cleanup(); + } + + @Test + void failedUpdatePreservesSnapshotsForAttachmentsNotYetChanged() { + Texture2D oldFirst = mipmappedTexture(); + Texture2D oldSecond = mipmappedTexture(); + Texture2D newFirst = mipmappedTexture(); + Texture2D newSecond = mipmappedTexture(); + FrameBuffer fb = framebuffer(oldFirst, oldSecond); + renderer.setFrameBuffer(fb); + fb.replaceColorTarget(0, FrameBufferTarget.newTarget(newFirst)); + fb.replaceColorTarget(1, FrameBufferTarget.newTarget(newSecond)); + driver.failTextureAttachmentSlot = 1; + assertThrows(RendererException.class, () -> renderer.setFrameBuffer(fb)); + assertEquals(newFirst.getImage().getId(), driver.attachment(fb, 0)); + assertEquals(oldSecond.getImage().getId(), driver.attachment(fb, 1)); + driver.mipmapGenerations.clear(); + renderer.setFrameBuffer(null); + assertEquals(Arrays.asList(newFirst.getImage().getId(), oldSecond.getImage().getId()), + driver.mipmapGenerations); + renderer.setFrameBuffer(fb); + assertEquals(newSecond.getImage().getId(), driver.attachment(fb, 1)); + renderer.cleanup(); + } + + @Test + void directImageDeletionPreservesQueuedUnsafeCpuCleanup() { + boolean originalUnsafe = NativeObjectManager.UNSAFE; + int[] bufferDeletes = {0}; + Image image = new Image(Image.Format.RGBA8, 8, 8, (ByteBuffer) null, ColorSpace.Linear) { + @Override + protected void deleteNativeBuffers() { + bufferDeletes[0]++; + } + }; + try { + NativeObjectManager.UNSAFE = true; + renderer.setFrameBuffer(framebuffer(new Texture2D(image))); + image.dispose(); + renderer.deleteImage(image); + renderer.postFrame(); + renderer.cleanup(); + assertEquals(1, bufferDeletes[0]); + } finally { + NativeObjectManager.UNSAFE = originalUnsafe; + } + } +} diff --git a/jme3-core/src/test/java/com/jme3/util/NativeObjectManagerDisposalTest.java b/jme3-core/src/test/java/com/jme3/util/NativeObjectManagerDisposalTest.java new file mode 100644 index 0000000000..6f3e9902f3 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/util/NativeObjectManagerDisposalTest.java @@ -0,0 +1,279 @@ +/* + * 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.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** Explicit CPU-buffer disposal must survive removal of obsolete GL requests. */ +public class NativeObjectManagerDisposalTest { + private final Object renderer = new Object(); + private boolean originalUnsafe; + + @BeforeEach + void saveUnsafe() { + originalUnsafe = NativeObjectManager.UNSAFE; + } + + @AfterEach + void restoreUnsafe() { + NativeObjectManager.UNSAFE = originalUnsafe; + } + + private static final class Counts { + int nativeDeletes; + int bufferDeletes; + } + + private static class Resource extends NativeObject { + final Counts counts; + + Resource() { + counts = new Counts(); + } + + Resource(int id, Counts counts) { + super(id); + this.counts = counts; + } + + @Override + public void resetObject() { + id = INVALID_ID; + setUpdateNeeded(); + } + + @Override + public void deleteObject(Object rendererObject) { + counts.nativeDeletes++; + resetObject(); + } + + @Override + protected void deleteNativeBuffers() { + // No raw memory is allocated/freed by this fixture. + counts.bufferDeletes++; + } + + @Override + public NativeObject createDestructableClone() { + return new Resource(id, counts); + } + + @Override + public long getUniqueId() { + return ((long) OBJTYPE_VERTEXBUFFER << 32) | (id & 0xffffffffL); + } + } + + private static Resource register(NativeObjectManager manager, int id) { + Resource resource = new Resource(); + resource.setId(id); + manager.registerObject(resource); + return resource; + } + + @Test + void unsafeResetReleasesOnlyExplicitlyQueuedBuffersExactlyOnce() { + NativeObjectManager.UNSAFE = true; + NativeObjectManager manager = new NativeObjectManager(); + Resource disposed = register(manager, 1); + Resource retained = register(manager, 2); + disposed.dispose(); + disposed.dispose(); + manager.resetObjects(); + assertEquals(-1, disposed.getId()); + assertEquals(-1, retained.getId()); + assertEquals(1, disposed.counts.bufferDeletes); + assertEquals(0, retained.counts.bufferDeletes); + assertEquals(0, disposed.counts.nativeDeletes); + assertEquals(0, retained.counts.nativeDeletes); + manager.deleteUnused(renderer); + manager.resetObjects(); + assertEquals(1, disposed.counts.bufferDeletes); + assertEquals(0, disposed.counts.nativeDeletes); + } + + @Test + void safeResetDiscardsOldRequestsBeforeReinitialization() { + NativeObjectManager.UNSAFE = false; + NativeObjectManager manager = new NativeObjectManager(); + Resource resource = register(manager, 1); + resource.dispose(); + manager.resetObjects(); + resource.setId(2); + manager.registerObject(resource); + manager.deleteUnused(renderer); + assertEquals(2, resource.getId()); + assertEquals(0, resource.counts.nativeDeletes); + assertEquals(0, resource.counts.bufferDeletes); + resource.dispose(); + manager.deleteUnused(renderer); + assertEquals(1, resource.counts.nativeDeletes); + assertEquals(0, resource.counts.bufferDeletes); + } + + @Test + void fullCleanupReleasesAllQueuedBuffersBeyondFrameLimit() { + NativeObjectManager.UNSAFE = true; + NativeObjectManager manager = new NativeObjectManager(); + Resource[] queued = new Resource[150]; + for (int i = 0; i < queued.length; i++) { + queued[i] = register(manager, i + 1); + queued[i].dispose(); + queued[i].dispose(); + } + Resource retained = register(manager, 1000); + manager.deleteAllObjects(renderer); + for (Resource resource : queued) { + assertEquals(-1, resource.getId()); + assertEquals(1, resource.counts.nativeDeletes); + assertEquals(1, resource.counts.bufferDeletes); + } + assertEquals(1, retained.counts.nativeDeletes); + assertEquals(0, retained.counts.bufferDeletes); + manager.deleteUnused(renderer); + manager.deleteAllObjects(renderer); + assertEquals(1, queued[0].counts.bufferDeletes); + } + + @Test + void safeFullCleanupDoesNotFreeCpuBuffersOrLeaveDeletionRequests() { + NativeObjectManager.UNSAFE = false; + NativeObjectManager manager = new NativeObjectManager(); + Resource[] resources = new Resource[150]; + for (int i = 0; i < resources.length; i++) { + resources[i] = register(manager, i + 1); + resources[i].dispose(); + } + manager.deleteAllObjects(renderer); + for (Resource resource : resources) { + assertEquals(1, resource.counts.nativeDeletes); + assertEquals(0, resource.counts.bufferDeletes); + } + resources[0].setId(1); + manager.registerObject(resources[0]); + manager.deleteUnused(renderer); + assertEquals(1, resources[0].getId()); + assertEquals(1, resources[0].counts.nativeDeletes); + } + + @Test + void unsafeDirectUnregistrationHonorsQueuedCpuDisposalOnce() { + NativeObjectManager.UNSAFE = true; + NativeObjectManager manager = new NativeObjectManager(); + Resource resource = register(manager, 1); + resource.dispose(); + resource.dispose(); + manager.unregisterObject(resource); + resource.resetObject(); + manager.deleteUnused(renderer); + manager.resetObjects(); + assertEquals(1, resource.counts.bufferDeletes); + assertEquals(0, resource.counts.nativeDeletes); + } + + @Test + void safeDirectUnregistrationDoesNotDeleteReusedNativeName() { + NativeObjectManager.UNSAFE = false; + NativeObjectManager manager = new NativeObjectManager(); + Resource resource = register(manager, 1); + resource.dispose(); + manager.unregisterObject(resource); + resource.resetObject(); + resource.setId(1); + manager.registerObject(resource); + manager.deleteUnused(renderer); + assertEquals(1, resource.getId()); + assertEquals(0, resource.counts.nativeDeletes); + assertEquals(0, resource.counts.bufferDeletes); + } + + @Test + void managedDisposalDoesNotRunCpuCleanupTwiceViaUnregister() { + NativeObjectManager.UNSAFE = true; + NativeObjectManager manager = new NativeObjectManager(); + Resource resource = new Resource() { + @Override + public void deleteObject(Object rendererObject) { + manager.unregisterObject(this); + super.deleteObject(rendererObject); + } + }; + resource.setId(1); + manager.registerObject(resource); + resource.dispose(); + resource.dispose(); + manager.deleteUnused(renderer); + assertEquals(1, resource.counts.nativeDeletes); + assertEquals(1, resource.counts.bufferDeletes); + } + + @Test + void ordinaryCleanupRetainsItsFrameLimitAndDeduplicatesRequests() { + NativeObjectManager.UNSAFE = true; + NativeObjectManager manager = new NativeObjectManager(); + Resource[] resources = new Resource[150]; + for (int i = 0; i < resources.length; i++) { + resources[i] = register(manager, i + 1); + resources[i].dispose(); + resources[i].dispose(); + } + manager.deleteUnused(renderer); + int deleted = 0; + for (Resource resource : resources) { + deleted += resource.counts.nativeDeletes; + assertEquals(resource.counts.nativeDeletes, resource.counts.bufferDeletes); + } + assertEquals(100, deleted); + manager.deleteUnused(renderer); + for (Resource resource : resources) { + assertEquals(1, resource.counts.nativeDeletes); + assertEquals(1, resource.counts.bufferDeletes); + } + } + + @Test + void unqueuedDirectUnregistrationNeverFreesCpuBuffers() { + NativeObjectManager.UNSAFE = true; + NativeObjectManager manager = new NativeObjectManager(); + Resource resource = register(manager, 1); + manager.unregisterObject(resource); + resource.resetObject(); + manager.deleteUnused(renderer); + assertEquals(0, resource.counts.nativeDeletes); + assertEquals(0, resource.counts.bufferDeletes); + } +} From 56e53704be8e4fcbdfddc786cfe9a1e95f23852d Mon Sep 17 00:00:00 2001 From: toaster0123 <261194501+toaster0123@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:27:32 +0000 Subject: [PATCH 5/6] Preserve screen rendering on backends without framebuffer objects Handle the window-system framebuffer without unsupported FBO calls after invalidation, reset and cleanup. Reject actual offscreen targets as before. Avoid querying GL_FRAMEBUFFER_BINDING when the capability is absent. Seven new fallback cases failed before this fix and now pass. Integrate six lifecycle edge regressions covering shared Images, stale phantom references, 151-resource cleanup, reset, texture-to-buffer replacement and UNSAFE cleanup. Core/effects: 661 passed, one existing skipped; no Checkstyle warnings on modified lines. --- .../com/jme3/renderer/opengl/GLRenderer.java | 23 ++- ...lRendererFrameBufferLifecycleEdgeTest.java | 194 ++++++++++++++++++ .../opengl/GlRendererNoFrameBufferTest.java | 139 +++++++++++++ 3 files changed, 347 insertions(+), 9 deletions(-) create mode 100644 jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleEdgeTest.java create mode 100644 jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererNoFrameBufferTest.java 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 index 21a7805361..9fdec55e71 100644 --- a/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java +++ b/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java @@ -969,13 +969,10 @@ public void initialize() { } } - IntBuffer tmp = BufferUtils.createIntBuffer(16); - gl.glGetInteger(GL.GL_FRAMEBUFFER_BINDING, tmp); - tmp.rewind(); - int fbOnLoad = tmp.get(); - // The presentation framebuffer can be nonzero, notably on iOS. - defaultFBO = fbOnLoad; - context.boundFBO = fbOnLoad; + // Without FBO support, even querying GL_FRAMEBUFFER_BINDING is invalid. + // Otherwise the presentation framebuffer can be nonzero, notably on iOS. + defaultFBO = caps.contains(Caps.FrameBuffer) ? getInteger(GL.GL_FRAMEBUFFER_BINDING) : 0; + context.boundFBO = defaultFBO; context.boundFB = null; } @@ -2576,8 +2573,16 @@ public void setFrameBuffer(FrameBuffer fb) { } if (!caps.contains(Caps.FrameBuffer)) { - throw new RendererException("Framebuffer objects are not supported" - + " by the video hardware"); + if (fb != null) { + throw new RendererException("Framebuffer objects are not supported" + + " by the video hardware"); + } + // The window-system framebuffer is the only possible destination. + // Restore the cache after invalidation without an unsupported GL call. + context.boundFBO = 0; + context.boundFB = null; + toggleFramebufferSrgb(null); + return; } // generate mipmaps for last FB if needed diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleEdgeTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleEdgeTest.java new file mode 100644 index 0000000000..38d76b42d3 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferLifecycleEdgeTest.java @@ -0,0 +1,194 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.jme3.texture.FrameBuffer; +import com.jme3.texture.FrameBuffer.FrameBufferTarget; +import com.jme3.texture.Image; +import com.jme3.texture.Texture; +import com.jme3.texture.Texture2D; +import com.jme3.util.NativeObjectManager; +import java.lang.ref.Reference; +import java.lang.reflect.Field; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/** Exercises mixed resource cleanup, shared images, and stale reference edge cases. */ +public class GlRendererFrameBufferLifecycleEdgeTest extends FrameBufferTestSupport { + @Test + void distinctTextureWrappersCanShareTheSameImageAfterDisposal() { + Image image = texture().getImage(); + Texture2D a = new Texture2D(image); + Texture2D b = new Texture2D(image); + a.setMinFilter(Texture.MinFilter.Trilinear); + b.setMinFilter(Texture.MinFilter.Trilinear); + FrameBuffer first = framebuffer(a); + FrameBuffer second = framebuffer(b); + renderer.setFrameBuffer(first); + renderer.setFrameBuffer(second); + final int imageId = image.getId(); + first.dispose(); + renderer.postFrame(); + renderer.setFrameBuffer(null); + assertEquals(imageId, image.getId()); + assertTrue(driver.liveTextures.contains(imageId)); + assertEquals(imageId, driver.attachment(second, 0)); + renderer.cleanup(); + assertTrue(driver.liveTextures.isEmpty()); + assertTrue(driver.liveFramebuffers.isEmpty()); + } + + @Test + void staleImagePhantomReferenceCannotDeleteReusedNativeName() throws Exception { + Texture2D old = texture(); + renderer.setTexture(0, old); + final int id = old.getImage().getId(); + Field managerField = GLRenderer.class.getDeclaredField("objManager"); + managerField.setAccessible(true); + NativeObjectManager manager = (NativeObjectManager) managerField.get(renderer); + Field mapField = NativeObjectManager.class.getDeclaredField("refMap"); + mapField.setAccessible(true); + Reference ref = (Reference) ((Map) mapField.get(manager)) + .get(old.getImage().getUniqueId()); + assertTrue(ref.enqueue()); + renderer.deleteImage(old.getImage()); + driver.reuseTextureId = id; + Texture2D replacement = texture(); + renderer.setTexture(0, replacement); + renderer.postFrame(); + assertEquals(id, replacement.getImage().getId()); + assertTrue(driver.liveTextures.contains(id)); + renderer.cleanup(); + } + + @Test + void cleanupOfManyMixedResourcesLeavesNoNativeObjects() { + FrameBuffer[] buffers = new FrameBuffer[151]; + for (int i = 0; i < buffers.length; i++) { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + fb.setDepthTarget(FrameBufferTarget.newTarget(Image.Format.Depth24Stencil8)); + renderer.setFrameBuffer(fb); + buffers[i] = fb; + if ((i & 1) == 0) { + fb.dispose(); + texture.getImage().dispose(); + } + } + renderer.cleanup(); + assertTrue(driver.liveFramebuffers.isEmpty()); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertTrue(driver.liveTextures.isEmpty()); + for (FrameBuffer fb : buffers) { + assertEquals(-1, fb.getId()); + assertEquals(-1, fb.getDepthTarget().getId()); + } + } + + @Test + void contextResetReinitializesSharedImagesAndAttachmentsOnce() { + Texture2D shared = texture(); + FrameBuffer a = framebuffer(shared); + FrameBuffer b = framebuffer(shared); + renderer.setFrameBuffer(a); + renderer.setFrameBuffer(b); + a.dispose(); + shared.getImage().dispose(); + renderer.resetGLObjects(); + driver.liveTextures.clear(); + driver.liveRenderbuffers.clear(); + driver.liveFramebuffers.clear(); + driver.attachments.clear(); + driver.boundTextures.clear(); + driver.drawFramebuffer = 0; + driver.readFramebuffer = 0; + renderer.setFrameBuffer(a); + renderer.setFrameBuffer(b); + renderer.postFrame(); + assertEquals(1, driver.liveTextures.size()); + assertEquals(2, driver.liveFramebuffers.size()); + assertEquals(shared.getImage().getId(), driver.attachment(a, 0)); + assertEquals(shared.getImage().getId(), driver.attachment(b, 0)); + renderer.cleanup(); + } + + @Test + void textureToBufferReplacementDropsOnlyItsMipmapSnapshot() { + Texture2D texture = texture(); + texture.setMinFilter(Texture.MinFilter.Trilinear); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + fb.clearColorTargets(); + fb.addColorTarget(FrameBufferTarget.newTarget(Image.Format.RGBA8)); + renderer.setFrameBuffer(fb); + driver.mipmapGenerations.clear(); + renderer.setFrameBuffer(null); + assertTrue(driver.mipmapGenerations.isEmpty()); + assertTrue(driver.liveTextures.contains(texture.getImage().getId())); + fb.dispose(); + renderer.postFrame(); + assertTrue(driver.liveRenderbuffers.isEmpty()); + assertTrue(driver.liveTextures.contains(texture.getImage().getId())); + renderer.cleanup(); + } + + @Test + void explicitUnsafeDeletionWithImageMutationDoesNotDoubleFree() { + boolean before = NativeObjectManager.UNSAFE; + int[] frees = {0}; + Image image = new Image(Image.Format.RGBA8, 8, 8, (java.nio.ByteBuffer) null) { + @Override + protected void deleteNativeBuffers() { + frees[0]++; + } + }; + Texture2D texture = new Texture2D(image); + texture.setMinFilter(Texture.MinFilter.Trilinear); + try { + NativeObjectManager.UNSAFE = true; + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + texture.setImage(texture().getImage()); + image.dispose(); + renderer.deleteImage(image); + renderer.setFrameBuffer(null); + renderer.postFrame(); + renderer.cleanup(); + assertEquals(1, frees[0]); + } finally { + NativeObjectManager.UNSAFE = before; + } + } +} diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererNoFrameBufferTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererNoFrameBufferTest.java new file mode 100644 index 0000000000..a87ffd7f61 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererNoFrameBufferTest.java @@ -0,0 +1,139 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.jme3.renderer.Caps; +import com.jme3.renderer.RendererException; +import com.jme3.texture.FrameBuffer; +import com.jme3.util.BufferUtils; +import java.lang.reflect.Proxy; +import org.junit.jupiter.api.Test; + +/** Screen rendering remains available on OpenGL 2.1 without FBO extensions. */ +public class GlRendererNoFrameBufferTest extends FrameBufferTestSupport { + private int framebufferQueries; + + private void initializeWithoutFramebuffers(boolean rejectBindingQuery) { + driver = new RecordingGL(0); + GL2 legacyGl = (GL2) Proxy.newProxyInstance(GL2.class.getClassLoader(), + new Class[] {GL2.class}, (proxy, method, args) -> { + if (method.getName().equals("glGetString") && (Integer) args[0] == GL.GL_EXTENSIONS) { + return ""; + } + if (method.getName().equals("glGetInteger") + && (Integer) args[0] == GL.GL_FRAMEBUFFER_BINDING) { + framebufferQueries++; + if (rejectBindingQuery) { + throw new AssertionError("GL_FRAMEBUFFER_BINDING is not supported"); + } + } + return driver.invoke(proxy, method, args); + }); + GLFbo unsupportedFbo = (GLFbo) Proxy.newProxyInstance(GLFbo.class.getClassLoader(), + new Class[] {GLFbo.class}, (proxy, method, args) -> { + throw new AssertionError("Unsupported FBO entry point: " + method.getName()); + }); + renderer = new GLRenderer(legacyGl, driver.proxy(GLExt.class), unsupportedFbo); + renderer.initialize(); + assertFalse(renderer.getCaps().contains(Caps.FrameBuffer)); + } + + @Test + void initializationDoesNotQueryUnsupportedFramebufferBinding() { + assertDoesNotThrow(() -> initializeWithoutFramebuffers(true)); + assertEquals(0, framebufferQueries); + } + + @Test + void screenBindingWorksBeforeAndAfterInvalidation() { + initializeWithoutFramebuffers(false); + renderer.setFrameBuffer(null); + renderer.invalidateState(); + assertDoesNotThrow(() -> renderer.setFrameBuffer(null)); + assertDoesNotThrow(() -> renderer.setFrameBuffer(null)); + assertNull(renderer.getCurrentFrameBuffer()); + assertEquals(0, driver.drawFramebuffer); + } + + @Test + void screenBindingWorksAfterContextResetAndCleanup() { + initializeWithoutFramebuffers(false); + renderer.resetGLObjects(); + assertDoesNotThrow(() -> renderer.setFrameBuffer(null)); + renderer.cleanup(); + assertDoesNotThrow(() -> renderer.setFrameBuffer(null)); + assertNull(renderer.getCurrentFrameBuffer()); + } + + @Test + void screenReadbackWorksAfterInvalidation() { + initializeWithoutFramebuffers(false); + renderer.setViewPort(0, 0, 1, 1); + renderer.invalidateState(); + assertDoesNotThrow(() -> renderer.readFrameBuffer(null, BufferUtils.createByteBuffer(4))); + } + + @Test + void clearingMainOverrideDoesNotRequireFramebufferFunctions() { + initializeWithoutFramebuffers(false); + renderer.invalidateState(); + assertDoesNotThrow(() -> renderer.setMainFrameBufferOverride(null)); + assertDoesNotThrow(() -> renderer.setFrameBuffer(null)); + } + + @Test + void offscreenFramebufferRemainsUnsupportedWithoutAllocation() { + initializeWithoutFramebuffers(false); + FrameBuffer offscreen = framebuffer(texture()); + renderer.invalidateState(); + assertThrows(RendererException.class, () -> renderer.setFrameBuffer(offscreen)); + assertEquals(-1, offscreen.getId()); + assertDoesNotThrow(() -> renderer.setFrameBuffer(null)); + } + + @Test + void offscreenOverrideRemainsUnsupportedWithoutAllocation() { + initializeWithoutFramebuffers(false); + FrameBuffer offscreen = framebuffer(texture()); + renderer.invalidateState(); + assertThrows(RendererException.class, () -> renderer.setMainFrameBufferOverride(offscreen)); + assertEquals(-1, offscreen.getId()); + assertNull(renderer.getCurrentFrameBuffer()); + assertDoesNotThrow(() -> renderer.setFrameBuffer(null)); + } +} From 310942d1c5eac618b5f853968b12b800e894bb81 Mon Sep 17 00:00:00 2001 From: toaster0123 <261194501+toaster0123@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:57:59 +0000 Subject: [PATCH 6/6] Fix framebuffer reattachment after image disposal Track renderer-owned attachment invalidation for color and depth textures and reconcile it on bind, blit and readback without requiring callers to dirty the framebuffer. Preserve invalidation across failed updates and keep renderbuffer storage and GC ownership unchanged. Add 20 regression/control tests, strengthen recording GL with native texture generations and bound-FBO deletion semantics, and document the existing public attachment update methods' full-framebuffer/viewport side effects. --- .../com/jme3/renderer/opengl/GLRenderer.java | 49 ++- .../opengl/FrameBufferTestSupport.java | 55 ++- .../GlRendererFrameBufferImageRebindTest.java | 342 ++++++++++++++++++ .../GlRendererFrameBufferSnapshotTest.java | 9 +- 4 files changed, 438 insertions(+), 17 deletions(-) create mode 100644 jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferImageRebindTest.java 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 index 58a86191b6..1ccc14a4e5 100644 --- a/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java +++ b/jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java @@ -129,9 +129,13 @@ private static final class FrameBufferState { final ArrayList renderBuffers = new ArrayList<>(); final ColorAttachmentState[] colorTextures = new ColorAttachmentState[16]; int colorTextureSlots; + int depthTextureId = NativeObject.INVALID_ID; + // Native image deletion can invalidate an otherwise unchanged Java FBO. + boolean attachmentsDirty; - void setColorTexture(int slot, Texture texture) { + void setTextureAttachment(int slot, Texture texture) { if (slot < 0) { + depthTextureId = texture == null ? NativeObject.INVALID_ID : texture.getImage().getId(); return; } ColorAttachmentState attachment = colorTextures[slot]; @@ -160,9 +164,14 @@ void invalidateImage(int imageId) { int slot = Integer.numberOfTrailingZeros(slots); slots &= slots - 1; if (colorTextures[slot].imageId == imageId) { - setColorTexture(slot, null); + setTextureAttachment(slot, null); + attachmentsDirty = true; } } + if (depthTextureId == imageId) { + depthTextureId = NativeObject.INVALID_ID; + attachmentsDirty = true; + } } RenderBufferState getRenderBuffer(RenderBuffer target) { @@ -2110,11 +2119,11 @@ public void copyFrameBuffer(FrameBuffer src, FrameBuffer dst, boolean copyColor, } } - if (src != null && src.isUpdateNeeded()) { + if (src != null && isFrameBufferUpdateNeeded(src)) { updateFrameBuffer(src); } - if (dst != null && dst.isUpdateNeeded()) { + if (dst != null && isFrameBufferUpdateNeeded(dst)) { updateFrameBuffer(dst); } @@ -2263,8 +2272,8 @@ private int convertAttachmentSlot(int attachmentSlot) { /** * Synchronizes and binds the framebuffer containing this texture target. - * All pending attachment changes are applied, as in - * {@link #updateFrameBufferAttachment(FrameBuffer, RenderBuffer)}. + * All pending attachment changes and rendering-state side effects described + * by {@link #updateFrameBufferAttachment(FrameBuffer, RenderBuffer)} apply. * * @param fb the framebuffer (not null) * @param rb one of its current texture targets (not null) @@ -2311,6 +2320,12 @@ private void updateRenderTextureInternal(FrameBuffer fb, RenderBuffer rb) { * Synchronizes and binds the framebuffer containing this target. Reconciles * all pending target changes so native attachments and their cleanup/mipmap * snapshots cannot diverge. An uninitialized framebuffer is initialized first. + *

+ * Like {@link #setFrameBuffer(FrameBuffer)}, this leaves {@code fb} bound, + * resets the viewport to its full dimensions, applies its sRGB write state, + * and may generate mipmaps for the previous attachments. It updates all + * current targets, not only {@code rb}; the previous framebuffer and viewport + * are not restored. Set a custom viewport after calling this method. * * @param fb the framebuffer (not null) * @param rb one of its current targets (not null) @@ -2349,7 +2364,7 @@ private void updateFrameBufferAttachment(FrameBuffer fb, RenderBuffer rb, FrameB updateRenderTextureInternal(fb, rb); } state.attachmentSlots |= attachmentSlotMask(rb.getSlot()); - state.setColorTexture(rb.getSlot(), rb.getTexture()); + state.setTextureAttachment(rb.getSlot(), rb.getTexture()); } private void bindFrameBuffer(FrameBuffer fb) { @@ -2421,7 +2436,7 @@ public void updateFrameBuffer(FrameBuffer fb) { glfbo.glFramebufferRenderbufferEXT(GLFbo.GL_FRAMEBUFFER_EXT, convertAttachmentSlot(slot), GLFbo.GL_RENDERBUFFER_EXT, 0); state.attachmentSlots &= ~(1 << index); - state.setColorTexture(slot, null); + state.setTextureAttachment(slot, null); removedSlots &= removedSlots - 1; } for (int i = state.renderBuffers.size() - 1; i >= 0; i--) { @@ -2446,9 +2461,20 @@ public void updateFrameBuffer(FrameBuffer fb) { setReadDrawBuffers(fb); checkFrameBufferError(); + // Keep renderer-side invalidation pending through upload/attachment or + // completeness failures. The Java target list may never have changed. + state.attachmentsDirty = false; fb.clearUpdateNeeded(); } + private boolean isFrameBufferUpdateNeeded(FrameBuffer fb) { + if (fb.isUpdateNeeded()) { + return true; + } + FrameBufferState state = frameBufferStates.get(fb.getId()); + return state != null && state.attachmentsDirty; + } + private static int attachmentSlotMask(int slot) { if (slot == FrameBuffer.SLOT_DEPTH) { return 1 << 16; @@ -2566,9 +2592,10 @@ public void setFrameBuffer(FrameBuffer fb) { fb = mainFbOverride; } + boolean updateNeeded = fb != null && isFrameBufferUpdateNeeded(fb); if (context.boundFB == fb && context.boundFBO == (fb == null ? defaultFBO : fb.getId())) { - if (fb == null || !fb.isUpdateNeeded()) { + if (!updateNeeded) { toggleFramebufferSrgb(fb); return; } @@ -2603,7 +2630,7 @@ public void setFrameBuffer(FrameBuffer fb) { Texture tex = attachment.texture; Image image = attachment.image; if (image.getId() <= 0 || image.getId() != attachment.imageId) { - state.setColorTexture(slot, null); + state.setTextureAttachment(slot, null); continue; } if (tex.getMinFilter().usesMipMapLevels() @@ -2637,7 +2664,7 @@ && isMipmapGenerationSupported(image.getFormat(), if (fb == null) { bindFrameBuffer(null); } else { - if (fb.isUpdateNeeded()) { + if (updateNeeded) { updateFrameBuffer(fb); } else { bindFrameBuffer(fb); diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java index 62bad75088..72e4f7a136 100644 --- a/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/FrameBufferTestSupport.java @@ -113,6 +113,13 @@ static final class RecordingGL implements InvocationHandler { final Map> drawBuffers = new HashMap<>(); final List mipmapGenerations = new ArrayList<>(); final Map boundTextures = new HashMap<>(); + final Map textureVersions = new HashMap<>(); + final Map> attachedTextureVersions = new HashMap<>(); + int nextTextureVersion; + int textureAttachmentCalls; + int framebufferChecks; + int viewportWidth; + int viewportHeight; int nextObjectId = 100; int reuseRenderbufferId = -1; int reuseFramebufferId = -1; @@ -157,11 +164,15 @@ public Object invoke(Object proxy, Method method, Object[] args) { int texture = reuseTextureId >= 0 ? reuseTextureId : nextObjectId++; reuseTextureId = -1; liveTextures.add(texture); + textureVersions.put(texture, ++nextTextureVersion); ((IntBuffer) args[0]).put(0, texture); return null; case "glDeleteTextures": int deletedTexture = ((IntBuffer) args[0]).get(0); liveTextures.remove(deletedTexture); + Integer deletedVersion = textureVersions.remove(deletedTexture); + detachDeletedTexture(drawFramebuffer, deletedTexture, deletedVersion); + detachDeletedTexture(readFramebuffer, deletedTexture, deletedVersion); boundTextures.replaceAll((unit, name) -> name == deletedTexture ? 0 : name); return null; case "glActiveTexture": @@ -182,6 +193,7 @@ public Object invoke(Object proxy, Method method, Object[] args) { reuseFramebufferId = -1; liveFramebuffers.add(framebuffer); attachments.put(framebuffer, new HashMap<>()); + attachedTextureVersions.put(framebuffer, new HashMap<>()); ((IntBuffer) args[0]).put(0, framebuffer); return null; case "glGenRenderbuffersEXT": @@ -219,6 +231,7 @@ public Object invoke(Object proxy, Method method, Object[] args) { assertTrue(liveFramebuffers.remove(deletedFramebuffer), "Deleting a stale framebuffer name"); attachments.remove(deletedFramebuffer); + attachedTextureVersions.remove(deletedFramebuffer); return null; case "glBindFramebufferEXT": int target = (Integer) args[0]; @@ -240,11 +253,15 @@ public Object invoke(Object proxy, Method method, Object[] args) { failTextureAttachmentSlot = -1; throw new RendererException("Simulated texture attachment failure"); } + boolean textureAttachment = method.getName().equals("glFramebufferTexture2DEXT"); + if (textureAttachment) { + textureAttachmentCalls++; + } if (slot == GL3.GL_DEPTH_STENCIL_ATTACHMENT) { - attachments.get(drawFramebuffer).put(GLFbo.GL_DEPTH_ATTACHMENT_EXT, attachment); - attachments.get(drawFramebuffer).put(GL_STENCIL_ATTACHMENT, attachment); + setAttachment(GLFbo.GL_DEPTH_ATTACHMENT_EXT, attachment, textureAttachment); + setAttachment(GL_STENCIL_ATTACHMENT, attachment, textureAttachment); } else { - attachments.get(drawFramebuffer).put(slot, attachment); + setAttachment(slot, attachment, textureAttachment); } return null; case "glDrawBuffer": @@ -259,7 +276,12 @@ public Object invoke(Object proxy, Method method, Object[] args) { drawBuffers.put(drawFramebuffer, targets); return null; case "glCheckFramebufferStatusEXT": + framebufferChecks++; return framebufferStatus; + case "glViewport": + viewportWidth = (Integer) args[2]; + viewportHeight = (Integer) args[3]; + return null; case "glBlitFramebufferEXT": blitDrawFramebuffer = drawFramebuffer; blitReadFramebuffer = readFramebuffer; @@ -275,6 +297,33 @@ public Object invoke(Object proxy, Method method, Object[] args) { } } + private void setAttachment(int slot, int name, boolean texture) { + attachments.get(drawFramebuffer).put(slot, name); + Map versions = attachedTextureVersions.get(drawFramebuffer); + if (texture && name != 0) { + versions.put(slot, textureVersions.get(name)); + } else { + versions.remove(slot); + } + } + + private void detachDeletedTexture(int framebuffer, int name, Integer version) { + Map versions = attachedTextureVersions.get(framebuffer); + if (versions == null || version == null) { + return; + } + // Deletion only detaches from bound FBOs, and only this native object + // generation. An unbound FBO can retain an older object with this name. + versions.entrySet().removeIf(entry -> { + if (entry.getValue().equals(version) + && attachments.get(framebuffer).get(entry.getKey()) == name) { + attachments.get(framebuffer).put(entry.getKey(), 0); + return true; + } + return false; + }); + } + private static String getString(int name) { switch (name) { case GL.GL_VERSION: diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferImageRebindTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferImageRebindTest.java new file mode 100644 index 0000000000..0021767cd2 --- /dev/null +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferImageRebindTest.java @@ -0,0 +1,342 @@ +/* + * 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 static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.jme3.renderer.RendererException; +import com.jme3.renderer.TextureUnitException; +import com.jme3.texture.FrameBuffer; +import com.jme3.texture.FrameBuffer.FrameBufferTarget; +import com.jme3.texture.Image; +import com.jme3.texture.Texture2D; +import com.jme3.util.BufferUtils; +import org.junit.jupiter.api.Test; + +/** Deleted texture storage must be reattached without manually dirtying the FBO. */ +public class GlRendererFrameBufferImageRebindTest extends FrameBufferTestSupport { + private void assertCurrentTexture(FrameBuffer fb, Texture2D texture) { + assertTrue(texture.getImage().getId() > 0); + assertEquals(texture.getImage().getId(), driver.attachment(fb, 0)); + assertEquals(driver.textureVersions.get(texture.getImage().getId()), + driver.attachedTextureVersions.get(fb.getId()).get(GLFbo.GL_COLOR_ATTACHMENT0_EXT)); + } + + @Test + void boundColorImageIsRecreatedAndReattached() { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + renderer.deleteImage(texture.getImage()); + assertEquals(0, driver.attachment(fb, 0)); + assertFalse(fb.isUpdateNeeded()); + renderer.setFrameBuffer(fb); + assertCurrentTexture(fb, texture); + renderer.cleanup(); + } + + @Test + void unboundImageUploadedBeforeBindingIsReattached() throws TextureUnitException { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + final int oldId = texture.getImage().getId(); + renderer.setFrameBuffer(null); + renderer.deleteImage(texture.getImage()); + assertEquals(oldId, driver.attachment(fb, 0), "Unbound FBO retains the deleted object"); + renderer.setTexture(0, texture); + assertNotEquals(oldId, texture.getImage().getId()); + renderer.setFrameBuffer(fb); + assertCurrentTexture(fb, texture); + renderer.cleanup(); + } + + @Test + void sameNativeNameStillReattachesANewObjectToUnboundFramebuffer() throws TextureUnitException { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + final int oldId = texture.getImage().getId(); + renderer.setFrameBuffer(null); + renderer.deleteImage(texture.getImage()); + driver.reuseTextureId = oldId; + renderer.setTexture(0, texture); + assertEquals(oldId, texture.getImage().getId()); + renderer.setFrameBuffer(fb); + assertCurrentTexture(fb, texture); + renderer.cleanup(); + } + + @Test + void sameNativeNameStillReattachesToBoundFramebuffer() throws TextureUnitException { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + final int oldId = texture.getImage().getId(); + renderer.deleteImage(texture.getImage()); + driver.reuseTextureId = oldId; + renderer.setTexture(0, texture); + renderer.setFrameBuffer(fb); + assertCurrentTexture(fb, texture); + renderer.cleanup(); + } + + @Test + void queuedDisposalIsRecoveredOnNextBind() { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + texture.getImage().dispose(); + renderer.postFrame(); + renderer.setFrameBuffer(fb); + assertCurrentTexture(fb, texture); + renderer.cleanup(); + } + + @Test + void sharedImageReattachesEveryFramebufferWithDistinctWrappers() { + Texture2D a = texture(); + Texture2D b = new Texture2D(a.getImage()); + FrameBuffer first = framebuffer(a); + FrameBuffer second = framebuffer(b); + renderer.setFrameBuffer(first); + renderer.setFrameBuffer(second); + renderer.deleteImage(a.getImage()); + renderer.setFrameBuffer(first); + renderer.setFrameBuffer(second); + assertCurrentTexture(first, a); + assertCurrentTexture(second, b); + assertEquals(1, driver.liveTextures.size()); + renderer.cleanup(); + } + + private void checkDepthRecovery(Image.Format format) { + Texture2D depth = new Texture2D(8, 8, format); + FrameBuffer fb = new FrameBuffer(8, 8, 1); + fb.setDepthTarget(FrameBufferTarget.newTarget(depth)); + renderer.setFrameBuffer(fb); + renderer.deleteImage(depth.getImage()); + renderer.setFrameBuffer(fb); + assertTrue(depth.getImage().getId() > 0); + assertEquals(depth.getImage().getId(), driver.attachmentAt(fb, GLFbo.GL_DEPTH_ATTACHMENT_EXT)); + if (format == Image.Format.Depth24Stencil8) { + assertEquals(depth.getImage().getId(), driver.attachmentAt(fb, GL_STENCIL_ATTACHMENT)); + } + renderer.cleanup(); + } + + @Test + void depthOnlyImageIsRecreatedAndReattached() { + checkDepthRecovery(Image.Format.Depth); + } + + @Test + void packedDepthStencilImageIsRecreatedAndReattached() { + checkDepthRecovery(Image.Format.Depth24Stencil8); + } + + @Test + void blitSourceIsReattachedBeforeCopy() { + Texture2D texture = texture(); + FrameBuffer source = framebuffer(texture); + FrameBuffer destination = framebuffer(texture()); + renderer.setFrameBuffer(source); + renderer.setFrameBuffer(destination); + renderer.deleteImage(texture.getImage()); + renderer.copyFrameBuffer(source, destination, true, false); + assertCurrentTexture(source, texture); + assertEquals(destination.getId(), driver.drawFramebuffer); + renderer.cleanup(); + } + + @Test + void blitDestinationIsReattachedBeforeCopy() { + Texture2D texture = texture(); + FrameBuffer source = framebuffer(texture()); + FrameBuffer destination = framebuffer(texture); + renderer.setFrameBuffer(destination); + renderer.setFrameBuffer(source); + renderer.deleteImage(texture.getImage()); + renderer.copyFrameBuffer(source, destination, true, false); + assertCurrentTexture(destination, texture); + assertEquals(source.getId(), driver.drawFramebuffer); + renderer.cleanup(); + } + + @Test + void mainOverrideIsRepairedForNullBind() { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setMainFrameBufferOverride(fb); + renderer.deleteImage(texture.getImage()); + renderer.setFrameBuffer(null); + assertCurrentTexture(fb, texture); + renderer.cleanup(); + } + + @Test + void mainOverrideIsRepairedForNullBlitEndpoint() { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setMainFrameBufferOverride(fb); + FrameBuffer source = framebuffer(texture()); + renderer.setFrameBuffer(source); + renderer.deleteImage(texture.getImage()); + renderer.copyFrameBuffer(source, null, true, false); + assertCurrentTexture(fb, texture); + assertEquals(fb.getId(), driver.blitDrawFramebuffer); + renderer.cleanup(); + } + + @Test + void readbackRepairsDeletedAttachment() { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + renderer.deleteImage(texture.getImage()); + renderer.readFrameBuffer(fb, BufferUtils.createByteBuffer(8 * 8 * 4)); + assertCurrentTexture(fb, texture); + renderer.cleanup(); + } + + @Test + void failedReattachmentRemainsDirtyForRetry() { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + renderer.deleteImage(texture.getImage()); + driver.failTextureAttachmentSlot = 0; + assertThrows(RendererException.class, () -> renderer.setFrameBuffer(fb)); + assertFalse(fb.isUpdateNeeded(), "This retry relies on renderer-owned invalidation"); + renderer.setFrameBuffer(fb); + assertCurrentTexture(fb, texture); + renderer.cleanup(); + } + + @Test + void failedCompletenessCheckRemainsDirtyForRetry() { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + renderer.setFrameBuffer(fb); + renderer.deleteImage(texture.getImage()); + driver.framebufferStatus = GLFbo.GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT_EXT; + assertThrows(IllegalStateException.class, () -> renderer.setFrameBuffer(fb)); + final int checks = driver.framebufferChecks; + driver.framebufferStatus = GLFbo.GL_FRAMEBUFFER_COMPLETE_EXT; + renderer.setFrameBuffer(fb); + assertEquals(checks + 1, driver.framebufferChecks); + assertCurrentTexture(fb, texture); + renderer.cleanup(); + } + + @Test + void repairPreservesRenderbufferStorageAndReturnsToFastPath() { + Texture2D texture = texture(); + FrameBuffer fb = framebuffer(texture); + fb.setDepthTarget(FrameBufferTarget.newTarget(Image.Format.Depth)); + renderer.setFrameBuffer(fb); + final int allocations = driver.renderbufferStorageCalls.size(); + renderer.deleteImage(texture.getImage()); + renderer.setFrameBuffer(fb); + assertCurrentTexture(fb, texture); + final int checks = driver.framebufferChecks; + final int attachments = driver.textureAttachmentCalls; + renderer.setFrameBuffer(fb); + renderer.setFrameBuffer(null); + renderer.setFrameBuffer(fb); + assertEquals(checks, driver.framebufferChecks); + assertEquals(attachments, driver.textureAttachmentCalls); + assertEquals(allocations, driver.renderbufferStorageCalls.size()); + renderer.cleanup(); + } + + @Test + void unrelatedImageDeletionDoesNotDirtyFramebuffer() throws TextureUnitException { + FrameBuffer fb = framebuffer(texture()); + renderer.setFrameBuffer(fb); + Texture2D unrelated = texture(); + renderer.setTexture(0, unrelated); + final int checks = driver.framebufferChecks; + renderer.deleteImage(unrelated.getImage()); + renderer.setFrameBuffer(fb); + assertEquals(checks, driver.framebufferChecks); + renderer.cleanup(); + } + + @Test + void deletedOldDepthTextureDoesNotInvalidateItsRenderbufferReplacement() { + Texture2D depth = new Texture2D(8, 8, Image.Format.Depth24Stencil8); + FrameBuffer fb = framebuffer(texture()); + fb.setDepthTarget(FrameBufferTarget.newTarget(depth)); + renderer.setFrameBuffer(fb); + fb.setDepthTarget(FrameBufferTarget.newTarget(Image.Format.Depth)); + renderer.setFrameBuffer(fb); + final int checks = driver.framebufferChecks; + renderer.deleteImage(depth.getImage()); + renderer.setFrameBuffer(fb); + assertEquals(checks, driver.framebufferChecks); + assertEquals(fb.getDepthTarget().getId(), driver.attachmentAt(fb, GLFbo.GL_DEPTH_ATTACHMENT_EXT)); + assertEquals(0, driver.attachmentAt(fb, GL_STENCIL_ATTACHMENT)); + renderer.cleanup(); + } + + @Test + void publicTextureUpdateSelectsFramebufferAndResetsViewport() { + FrameBuffer previous = framebuffer(texture()); + renderer.setFrameBuffer(previous); + final int previousAttachment = driver.attachment(previous, 0); + renderer.setViewPort(2, 3, 4, 5); + FrameBuffer next = framebuffer(texture()); + renderer.updateRenderTexture(next, next.getColorTarget(0)); + assertEquals(next.getId(), driver.drawFramebuffer); + assertEquals(previousAttachment, driver.attachment(previous, 0)); + assertEquals(8, driver.viewportWidth); + assertEquals(8, driver.viewportHeight); + renderer.cleanup(); + } + + @Test + void publicAttachmentUpdateResetsViewportEvenOnSameFramebuffer() { + FrameBuffer fb = framebuffer(texture()); + renderer.setFrameBuffer(fb); + renderer.setViewPort(2, 3, 4, 5); + renderer.updateFrameBufferAttachment(fb, fb.getColorTarget(0)); + assertEquals(fb.getId(), driver.drawFramebuffer); + assertEquals(8, driver.viewportWidth); + assertEquals(8, driver.viewportHeight); + renderer.cleanup(); + } +} diff --git a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferSnapshotTest.java b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferSnapshotTest.java index 458b173620..d45d1d0dc2 100644 --- a/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferSnapshotTest.java +++ b/jme3-core/src/test/java/com/jme3/renderer/opengl/GlRendererFrameBufferSnapshotTest.java @@ -99,8 +99,8 @@ void deletedImageRecreatedWithSameNameCannotReviveOldSnapshot() throws Exception assertEquals(originalId, texture.getImage().getId()); renderer.setFrameBuffer(null); assertTrue(driver.mipmapGenerations.isEmpty(), "The recreated image was not rendered into"); - fb.setUpdateNeeded(); renderer.setFrameBuffer(fb); + assertEquals(originalId, driver.attachment(fb, 0)); renderer.setFrameBuffer(null); assertEquals(Collections.singletonList(originalId), driver.mipmapGenerations); renderer.cleanup(); @@ -214,7 +214,7 @@ void directUpdateReconcilesRemovedSlotsToo() { } @Test - void deletingSharedImageInvalidatesEveryFramebufferSnapshot() { + void deletingSharedImageRebuildsSnapshotsOnlyWhenFramebufferIsReused() { Texture2D shared = mipmappedTexture(); FrameBuffer first = framebuffer(shared); FrameBuffer second = framebuffer(shared); @@ -223,9 +223,12 @@ void deletingSharedImageInvalidatesEveryFramebufferSnapshot() { renderer.deleteImage(shared.getImage()); driver.mipmapGenerations.clear(); renderer.setFrameBuffer(null); + assertTrue(driver.mipmapGenerations.isEmpty(), "Deleted attachments must not generate mipmaps"); renderer.setFrameBuffer(first); + assertTrue(shared.getImage().getId() > 0); + assertEquals(shared.getImage().getId(), driver.attachment(first, 0)); renderer.setFrameBuffer(null); - assertTrue(driver.mipmapGenerations.isEmpty()); + assertEquals(Collections.singletonList(shared.getImage().getId()), driver.mipmapGenerations); renderer.cleanup(); }