husker-dev / openglfx

OpenGL implementation for JavaFX
Apache License 2.0
80 stars 10 forks source link

Safely closing OpenGLCanvas #19

Closed jameshball closed 2 years ago

jameshball commented 2 years ago

First of all, thanks for making this library as I believe it's the only way I can get JavaFX working at all with LWJGL on mac.

Unfortunately, I'm running into a few stability issues, particularly around disposing of the OpenGLCanvas.

When I start the program I have two windows

If I close (using the X on Windows) the Stage associated with the OpenGLCanvas window, it works fine, but when I reopen it with a button from the other window (which calls Stage.show()) and then close it again it consistently causes a JVM native crash.

I am assuming this is because I am somehow closing some LWJGL resource twice. Below is an example error log generated after the crash that shows it's an issue with GLFW.glfwDestroyWindow():

hs_err_pid4056.log

An alternative method I've tried which doesn't crash is to call close() on the Stage associated with the OpenGLCanvas window and completely re-initialise the Stage and OpenGLCanvas whenever I want to reopen the window. The problem with this method is that no resources are freed, which I can verify by repeatedly opening and closing the window and seeing the performance get worse and worse.

Snippet of the second method below:

public static void startOscilloscope(MainController controller) {
  // separate window for oscilloscope
  StackPane oscilloscopeRoot = new StackPane();
  oscilloscopeStage = new Stage();

  OpenGLCanvas oscilloscopeCanvas = OpenGLCanvas.create(new LWJGLInitializer(), DirectDrawPolicy.IF_AVAILABLE);
  oscilloscopeCanvas.setAnimator(new GLCanvasAnimator(GLCanvasAnimator.getUNLIMITED_FPS(), true));
  oscilloscopeCanvas.onInitialize(event -> {
    renderable = new Oscilloscope<>(audioPlayer, 2);
  });
  oscilloscopeCanvas.onRender(event ->
    renderable.onRender(oscilloscopeCanvas.getWidth(), oscilloscopeCanvas.getHeight(), event.getDelta())
  );

  oscilloscopeRoot.getChildren().add(oscilloscopeCanvas);

  oscilloscopeStage.setScene(new Scene(oscilloscopeRoot, 320, 320));
  oscilloscopeStage.show();
  oscilloscopeStage.showingProperty().addListener((o, old, showing) -> {
    if (!showing) {
      controller.oscilloscopeClosed();
    }
  });
}

public static void stopOscilloscope() {
  oscilloscopeStage.close();
  oscilloscopeStage = null;
}

public static void main(String[] args) {
  System.setProperty("prism.order", "es2,d3d,sw");
  System.setProperty("prism.vsync", "true");
  launch(args);
}

The launch(args) calls startOscilloscope() after initialisation of the main JavaFX Stage.

Oscilloscope class referenced in above code:

package sh.ball.oscilloscope;

import org.lwjgl.*;
import org.lwjgl.glfw.*;
import sh.ball.audio.AudioPlayer;

import java.nio.*;

import static org.lwjgl.opengl.GL11.*;
import static org.lwjgl.opengl.GL15.*;

public class Oscilloscope<S> implements Renderable {

  private static final int NUM_VERTICES = 4000;
  private static final int VERTEX_SIZE = 2;

  private final AudioPlayer<S> audioPlayer;
  private final byte[] buffer;
  private final int frameSize;
  private final int vbo_vertex_handle;

  public Oscilloscope(AudioPlayer<S> audioPlayer, int frameSize) {
    this.audioPlayer = audioPlayer;
    this.buffer = new byte[frameSize * NUM_VERTICES * VERTEX_SIZE];
    this.frameSize = frameSize;

    GLFWErrorCallback.createPrint(System.err).set();

    FloatBuffer vertex_data = BufferUtils.createFloatBuffer(NUM_VERTICES * VERTEX_SIZE);
    for (int i = 0; i < NUM_VERTICES; i++) {
      vertex_data.put(new float[] { 0, 0 });
    }
    vertex_data.flip();

    vbo_vertex_handle = glGenBuffers();
    glBindBuffer(GL_ARRAY_BUFFER, vbo_vertex_handle);
    glBufferData(GL_ARRAY_BUFFER, vertex_data, GL_STATIC_DRAW);
    glBindBuffer(GL_ARRAY_BUFFER, 0);
  }

  private float[] decode(final byte[] buf) {
    float[] fbuf = new float[buf.length / frameSize];
    ByteBuffer wrapped = ByteBuffer.wrap(buf).order(ByteOrder.LITTLE_ENDIAN);
    ShortBuffer shortBuffer = wrapped.asShortBuffer();
    boolean even = true;
    for (int i = 0; i < fbuf.length; i++) {
      fbuf[i] = (even ? 1 : -1) * shortBuffer.get(i) / (float) Short.MAX_VALUE;
      even = !even;
    }
    return fbuf;
  }

  @Override
  public void onRender(double width, double height, double deltaTime) {
    glClearColor(0, 0, 0, 1);

    try {
      audioPlayer.read(buffer);
    } catch (InterruptedException e) {
      e.printStackTrace();
    }
    float[] floatBuffer = decode(buffer);
    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT); // clear the framebuffer

    glBufferSubData(GL_ARRAY_BUFFER, 0, floatBuffer);

    glColor4f(0, 1, 0, 1);
    glBindBuffer(GL_ARRAY_BUFFER, vbo_vertex_handle);
    glVertexPointer(VERTEX_SIZE, GL_FLOAT, 0, 0L);
    glEnableClientState(GL_VERTEX_ARRAY);
    glDrawArrays(GL_POINTS, 0, NUM_VERTICES);
    glDisableClientState(GL_VERTEX_ARRAY);
  }
}

Let me know if there's any other info you need!

husker-dev commented 2 years ago

Hi!

Initially, it was not planned that the window would be closed and then openned. So the "lifecycle" of the Node was based on that. I removed it in the last commit and will release a new version later.

Some suggestions to your code

jameshball commented 2 years ago

Thanks so much for the quick response and code suggestions!!