Skip to content

Commit

Permalink
Fix memory leaks in render context #1311 (#1408)
Browse files Browse the repository at this point in the history
* Fix memory leaks in render context

* NativeObject.java: Codacy says "fields should be declared at the top"

* NativeObject.java: style correction (if statement without braces)

* NativeObject.java: use diamond notation (redundant type argument)

* NativeObject.java: provide javadoc for the new method

Co-authored-by: Stephen Gold <sgold@sonic.net>
  • Loading branch information
riccardobl and stephengold committed Nov 10, 2020
1 parent 5ea2611 commit a96eb08
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 13 deletions.
6 changes: 4 additions & 2 deletions jme3-core/src/main/java/com/jme3/renderer/RenderContext.java
Expand Up @@ -31,6 +31,8 @@
*/
package com.jme3.renderer;

import java.lang.ref.WeakReference;

import com.jme3.material.RenderState;
import com.jme3.math.ColorRGBA;
import com.jme3.scene.Mesh;
Expand Down Expand Up @@ -219,7 +221,7 @@ public class RenderContext {
*
* @see Renderer#setTexture(int, com.jme3.texture.Texture)
*/
public final Image[] boundTextures = new Image[16];
public final WeakReference<Image> boundTextures[] = new WeakReference[16];

/**
* IDList for texture units
Expand Down Expand Up @@ -252,7 +254,7 @@ public class RenderContext {
* Vertex attribs currently bound and enabled. If a slot is null, then
* it is disabled.
*/
public final VertexBuffer[] boundAttribs = new VertexBuffer[16];
public final WeakReference<VertexBuffer> [] boundAttribs = new WeakReference[16];

/**
* IDList for vertex attributes
Expand Down
21 changes: 11 additions & 10 deletions jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java
Expand Up @@ -62,6 +62,7 @@
import com.jme3.util.NativeObjectManager;
import jme3tools.shader.ShaderDebug;

import java.lang.ref.WeakReference;
import java.nio.ByteBuffer;
import java.nio.FloatBuffer;
import java.nio.IntBuffer;
Expand Down Expand Up @@ -2387,9 +2388,9 @@ private void bindTextureAndUnit(int target, Image img, int unit) {
gl.glActiveTexture(GL.GL_TEXTURE0 + unit);
context.boundTextureUnit = unit;
}
if (context.boundTextures[unit] != img) {
if (context.boundTextures[unit]==null||context.boundTextures[unit].get() != img.getWeakRef().get()) {
gl.glBindTexture(target, img.getId());
context.boundTextures[unit] = img;
context.boundTextures[unit] = img.getWeakRef();
statistics.onTextureUse(img, true);
} else {
statistics.onTextureUse(img, false);
Expand All @@ -2405,13 +2406,13 @@ private void bindTextureAndUnit(int target, Image img, int unit) {
* @param unit At what unit to bind the texture.
*/
private void bindTextureOnly(int target, Image img, int unit) {
if (context.boundTextures[unit] != img) {
if (context.boundTextures[unit] == null || context.boundTextures[unit].get() != img.getWeakRef().get()) {
if (context.boundTextureUnit != unit) {
gl.glActiveTexture(GL.GL_TEXTURE0 + unit);
context.boundTextureUnit = unit;
}
gl.glBindTexture(target, img.getId());
context.boundTextures[unit] = img;
context.boundTextures[unit] = img.getWeakRef();
statistics.onTextureUse(img, true);
} else {
statistics.onTextureUse(img, false);
Expand Down Expand Up @@ -2826,7 +2827,7 @@ public void clearVertexAttribs() {
for (int i = 0; i < attribList.oldLen; i++) {
int idx = attribList.oldList[i];
gl.glDisableVertexAttribArray(idx);
if (context.boundAttribs[idx].isInstanced()) {
if (context.boundAttribs[idx].get().isInstanced()) {
glext.glVertexAttribDivisorARB(idx, 0);
}
context.boundAttribs[idx] = null;
Expand Down Expand Up @@ -2881,13 +2882,13 @@ public void setVertexAttrib(VertexBuffer vb, VertexBuffer idb) {
updateBufferData(vb);
}

VertexBuffer[] attribs = context.boundAttribs;
WeakReference<VertexBuffer>[] attribs = context.boundAttribs;
for (int i = 0; i < slotsRequired; i++) {
if (!context.attribIndexList.moveToNew(loc + i)) {
gl.glEnableVertexAttribArray(loc + i);
}
}
if (attribs[loc] != vb) {
if (attribs[loc]==null||attribs[loc].get() != vb) {
// NOTE: Use id from interleaved buffer if specified
int bufId = idb != null ? idb.getId() : vb.getId();
assert bufId != -1;
Expand Down Expand Up @@ -2927,14 +2928,14 @@ public void setVertexAttrib(VertexBuffer vb, VertexBuffer idb) {

for (int i = 0; i < slotsRequired; i++) {
int slot = loc + i;
if (vb.isInstanced() && (attribs[slot] == null || !attribs[slot].isInstanced())) {
if (vb.isInstanced() && (attribs[slot] == null || attribs[slot].get() == null || !attribs[slot].get().isInstanced())) {
// non-instanced -> instanced
glext.glVertexAttribDivisorARB(slot, vb.getInstanceSpan());
} else if (!vb.isInstanced() && attribs[slot] != null && attribs[slot].isInstanced()) {
} else if (!vb.isInstanced() && attribs[slot] != null && attribs[slot].get() != null && attribs[slot].get().isInstanced()) {
// instanced -> non-instanced
glext.glVertexAttribDivisorARB(slot, 0);
}
attribs[slot] = vb;
attribs[slot] = vb.getWeakRef();
}
}
}
Expand Down
18 changes: 17 additions & 1 deletion jme3-core/src/main/java/com/jme3/util/NativeObject.java
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2009-2019 jMonkeyEngine
* Copyright (c) 2009-2020 jMonkeyEngine
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -31,6 +31,7 @@
*/
package com.jme3.util;

import java.lang.ref.WeakReference;
import java.nio.Buffer;

/**
Expand Down Expand Up @@ -79,6 +80,8 @@ public abstract class NativeObject implements Cloneable {
*/
protected boolean updateNeeded = true;

private WeakReference<NativeObject> weakRef;

/**
* Creates a new GLObject. Should be
* called by the subclasses.
Expand Down Expand Up @@ -227,4 +230,17 @@ public void dispose() {
objectManager.enqueueUnusedObject(this);
}
}

/**
* Acquire a weak reference to this NativeObject.
*
* @param <T> the type
* @return a weak reference (possibly a pre-existing one)
*/
public <T> WeakReference<T> getWeakRef() {
if (weakRef == null) {
weakRef = new WeakReference<>(this);
}
return (WeakReference<T>) weakRef;
}
}

0 comments on commit a96eb08

Please sign in to comment.