On Tue, 1 Oct 2024 17:37:15 GMT, Thiago Milczarek Sayao <tsa...@openjdk.org>
wrote:
> When creating a Scene, a `DrawableInfo` is allocated with `malloc`. When
> scene changes, this is called on `WindowStage.java`:
>
> `QuantumRenderer.getInstance().disposePresentable(painter.presentable); //
> latched on RT`
>
> But the underlying `DrawableInfo` is never freed.
>
> I also think this should be done when the Stage is closed.
>
> To test:
>
> import javafx.animation.Animation;
> import javafx.animation.KeyFrame;
> import javafx.animation.KeyValue;
> import javafx.animation.Timeline;
> import javafx.application.Application;
> import javafx.scene.Scene;
> import javafx.scene.control.TextField;
> import javafx.scene.control.Label;
> import javafx.scene.layout.Pane;
> import javafx.scene.layout.StackPane;
> import javafx.scene.layout.VBox;
> import javafx.scene.paint.Color;
> import javafx.stage.Stage;
> import javafx.util.Duration;
>
> public class TestScenes extends Application {
>
> @Override
> public void start(Stage stage) {
> Timeline timeline = new Timeline(
> new KeyFrame(Duration.millis(100), e ->
> stage.setScene(createScene("Scene 1", Color.RED))),
> new KeyFrame(Duration.millis(200), e ->
> stage.setScene(createScene("Scene 2", Color.BLUE))),
> new KeyFrame(Duration.millis(300), e ->
> stage.setScene(createScene("Scene 3", Color.GREEN)))
> );
>
> timeline.setCycleCount(Animation.INDEFINITE);
> timeline.play();
>
> stage.show();
> }
>
> private Scene createScene(String text, Color color) {
> return new Scene(new StackPane(), 400, 300, color);
> }
>
> public static void main(String[] args) {
> launch(TestScenes.class, args);
> }
> }
The fix works on Windows and macOS. I've left some additional comments.
modules/javafx.graphics/src/main/java/com/sun/prism/es2/ES2SwapChain.java line
193:
> 191: @Override
> 192: public ES2Graphics createGraphics() {
> 193: if (drawable == null || drawable.getNativeWindow() !=
> pState.getNativeWindow()) {
The null check wasn't here before, and it doesn't seem necessary as
`createGLDrawable()` does not return `null`.
modules/javafx.graphics/src/main/java/com/sun/prism/es2/GLDrawable.java line 56:
> 54: abstract boolean swapBuffers(GLContext glCtx);
> 55:
> 56: abstract void dispose();
`GLDrawable` could also get the dispose method by implementing
`GraphicsResource`.
modules/javafx.graphics/src/main/java/com/sun/prism/es2/IOSGLDrawable.java line
57:
> 55: @Override
> 56: void dispose() {
> 57: nDestroyDrawable(getNativeDrawableInfo());
You should set `nativeDrawableInfo` to zero in this method (and check this
before calling the native method), as otherwise we run the risk of freeing a
stale pointer when the `dispose()` method is called repeatedly. The same should
be done in all other implementations.
modules/javafx.graphics/src/main/native-prism-es2/GLDrawable.c line 44:
> 42: }
> 43:
> 44: void deleteDrawableInfo(DrawableInfo *dInfo)
Maybe delete this whole file, as its only content is now a function that just
calls `memset()`.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1586#pullrequestreview-2350517717
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1789053938
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1789054227
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1789054946
PR Review Comment: https://git.openjdk.org/jfx/pull/1586#discussion_r1789055920
