javafx shapes, bind coordinate values, performance issue, how to remove from scene

Viewed 184

Here is a boiled down version of my intended application. Run this code on JavaFX 15.0.1, you will see a window displaying a bunch of blue lines and dots, and a reload button. Rotate the mouse wheel to zoom in and out of the picture.

Problem is: Click the "reload" button about 20 times, then rotate the mouse wheel. Observe how the application freezes for seconds, sometimes cpu activitiy will reach 90%. If nothing suspicious, click for some more reloads.

So I believe the problem is how to remove all the shapes and bindings from the scene, this must be the reason of degrading performance, right? I do a pane.getChildren().clear(); Am I missing something here?

package application;
    
import java.util.List;
import java.util.Random;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javafx.application.Application;
import javafx.beans.property.DoubleProperty;
import javafx.beans.property.SimpleDoubleProperty;
import javafx.stage.Stage;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.layout.Pane;
import javafx.scene.layout.VBox;
import javafx.scene.paint.Color;
import javafx.scene.shape.Circle;
import javafx.scene.shape.Line;

public class Main extends Application {
    
    private DoubleProperty zoom = new SimpleDoubleProperty(1.0);
    private int w = 800;
    private int h = 500;

    @Override
    public void start(Stage stage) {
        try {
            Button reload = new Button("reload");
            Pane pane = new Pane();
            pane.setManaged(false);
            
            reload.setOnAction(event -> {
                pane.getChildren().clear();
                List<Displacement> disList = Stream.generate(Displacement::new).limit(1200).collect(Collectors.toList());;
                disList.forEach(dis -> pane.getChildren().add(new DisplacementLine(dis, w, h, pane)));
                disList.forEach(dis -> pane.getChildren().add(new DisplacementCircle(dis, w, h, pane)));
            });
            
            VBox vbox = new VBox(reload, pane);
            Pane root = new Pane();
            Scene scene = new Scene(root);
            root.getChildren().add(vbox);
            stage.setScene(scene);
            stage.setWidth(1000);
            stage.setHeight(600);
            stage.show();
            
            pane.prefWidthProperty().bind(zoom.multiply(w));
            pane.prefHeightProperty().bind(zoom.multiply(h));
            pane.layoutXProperty().bind(root.widthProperty().subtract(pane.prefWidthProperty()).divide(2.0));
            pane.layoutYProperty().bind(root.heightProperty().subtract(pane.prefHeightProperty()).divide(2.0));
            
            pane.setOnScroll(scrollEvent -> {
                double delta = 0.1;
                double f = scrollEvent.getDeltaY() > 0 ? 1+delta : 1/(1+delta);
                zoom.set(zoom.get() * f);
            });
            
            reload.fire();
            
        } catch(Exception e) {
            e.printStackTrace();
        }
    }

    
    public static void main(String[] args) {
        launch(args);
    }
}

class DisplacementLine extends Line {

    final int w, h;
    final double x1, y1, x2, y2;
    
    public DisplacementLine(Displacement dis, int w, int h, Pane pane) {
        this.w = w;
        this.h = h;
        x1 = dis.x;
        y1 = dis.y;
        x2 = dis.x + dis.u;
        y2 = dis.y + dis.v;
        startXProperty().bind(pane.prefWidthProperty().multiply(x1).divide(w));
        startYProperty().bind(pane.prefHeightProperty().multiply(y1).divide(h));
        endXProperty().bind(pane.prefWidthProperty().multiply(x2).divide(w));
        endYProperty().bind(pane.prefHeightProperty().multiply(y2).divide(h));
        setStroke(Color.BLUE);
        setVisible(dis.valid);
    }
}

class DisplacementCircle extends Circle {

    final int w, h;
    final double x, y;
    
    public DisplacementCircle(Displacement dis, int w, int h, Pane pane) {
        this.x = dis.x;
        this.y = dis.y;
        this.w = w;
        this.h = h;
        centerXProperty().bind(pane.prefWidthProperty().multiply(x).divide(w));
        centerYProperty().bind(pane.prefHeightProperty().multiply(y).divide(h));
        setRadius(1.5);
        setFill(Color.BLUE);
        setVisible(dis.valid);
    }
}

class Displacement {

    final static Random rand = new Random();
    final double x, y, u, v;
    final boolean valid;
    
    public Displacement() {
        x = rand.nextDouble() * 1000;
        y = rand.nextDouble() * 600;
        u = rand.nextDouble() * 30;
        v = rand.nextDouble() * 15;
        valid = rand.nextBoolean();
    }
}

app window

2 Answers

Based on the comment by geometrical I put together this variation. That looks very nice to mee, I can sill have immutable objects this way.

Here I introduced functions to listen for a change to pane width / height properties and then set the coordinates of each shape, so no thousands of bindings are created.

Will adapt this to my final project

public class Main extends Application {
    
    private DoubleProperty zoom = new SimpleDoubleProperty(1.0);
    private int w = 800;
    private int h = 500;

    @Override
    public void start(Stage stage) {
        try {
            Button reload = new Button("reload");
            Pane pane = new Pane();
            pane.setManaged(false);
            
            Consumer<Number> setX = value -> {
                double pw = value.doubleValue();
                for (Node node : pane.getChildren()) {
                    if (node instanceof DisplacementLine) {
                        DisplacementLine dl = (DisplacementLine) node;
                        dl.setStartX(pw * dl.x1 / w);
                        dl.setEndX(pw * dl.x2 / w);
                    }
                    if (node instanceof DisplacementCircle) {
                        DisplacementCircle dc = (DisplacementCircle) node;
                        dc.setCenterX(pw * dc.x / w);
                    }
                }
            };
            Consumer<Number> setY = value -> {
                double ph = value.doubleValue();
                for (Node node : pane.getChildren()) {
                    if (node instanceof DisplacementLine) {
                        DisplacementLine dl = (DisplacementLine) node;
                        dl.setStartY(ph * dl.y1 / h);
                        dl.setEndY(ph * dl.y2 / h);
                    }
                    if (node instanceof DisplacementCircle) {
                        DisplacementCircle dc = (DisplacementCircle) node;
                        dc.setCenterY(ph * dc.y / h);
                    }
                }
            };
            pane.prefWidthProperty().addListener((obs, oldval, newval) -> setX.accept(newval));
            pane.prefHeightProperty().addListener((obs, oldval, newval) -> setY.accept(newval));
            
            reload.setOnAction(event -> {
                pane.getChildren().clear();
                List<Displacement> disList = Stream.generate(Displacement::new).limit(1200).collect(Collectors.toList());;
                disList.forEach(dis -> pane.getChildren().add(new DisplacementLine(dis, pane)));
                disList.forEach(dis -> pane.getChildren().add(new DisplacementCircle(dis, pane)));
                setX.accept(pane.getPrefWidth());
                setY.accept(pane.getPrefHeight());
            });
            
            VBox vbox = new VBox(reload, pane);
            Pane root = new Pane();
            Scene scene = new Scene(root);
            root.getChildren().add(vbox);
            stage.setScene(scene);
            stage.setWidth(1000);
            stage.setHeight(600);
            stage.show();
            
            pane.prefWidthProperty().bind(zoom.multiply(w));
            pane.prefHeightProperty().bind(zoom.multiply(h));
            pane.layoutXProperty().bind(root.widthProperty().subtract(pane.prefWidthProperty()).divide(2.0));
            pane.layoutYProperty().bind(root.heightProperty().subtract(pane.prefHeightProperty()).divide(2.0));
            
            pane.setOnScroll(scrollEvent -> {
                double delta = 0.1;
                double f = scrollEvent.getDeltaY() > 0 ? 1+delta : 1/(1+delta);
                zoom.set(zoom.get() * f);
            });
            
            reload.fire();
            
        } catch(Exception e) {
            e.printStackTrace();
        }
    }

    
    public static void main(String[] args) {
        launch(args);
    }
}

class DisplacementLine extends Line {

    final double x1, y1, x2, y2;
    
    public DisplacementLine(Displacement dis, Pane pane) {
        x1 = dis.x;
        y1 = dis.y;
        x2 = dis.x + dis.u;
        y2 = dis.y + dis.v;
        setStroke(Color.BLUE);
        setVisible(dis.valid);
    }
}

class DisplacementCircle extends Circle {

    final double x, y;
    
    public DisplacementCircle(Displacement dis, Pane pane) {
        this.x = dis.x;
        this.y = dis.y;
        setRadius(1.5);
        setFill(Color.BLUE);
        setVisible(dis.valid);
    }
}

class Displacement {

    final static Random rand = new Random();
    final double x, y, u, v;
    final boolean valid;
    
    public Displacement() {
        x = rand.nextDouble() * 1000;
        y = rand.nextDouble() * 600;
        u = rand.nextDouble() * 30;
        v = rand.nextDouble() * 15;
        valid = rand.nextBoolean();
    }
}

To avoid GUI freezing take the long process off the application thread. To demonstrate it try this (demo only, not the best implementation):

        reload.setOnAction(event -> {
            pane.getChildren().clear();
            new Thread(()->{
                List<Displacement> disList = Stream.generate(Displacement::new).limit(1200).collect(Collectors.toList());
                disList.forEach(dis ->Platform.runLater(()-> pane.getChildren().add(new DisplacementLine(dis, w, h, pane))));
                disList.forEach(dis ->Platform.runLater(()-> pane.getChildren().add(new DisplacementCircle(dis, w, h, pane))));
            }).start();

        });

Consider using a Model class to hold all the information used by the view.
Also consider applying new displacements without reconstructing the lines and circles.
Implementation ofDisplacementLine with mutable displacement:

class DisplacementLine extends Line {

    private final SimpleDoubleProperty x1, y1, x2, y2;

    public DisplacementLine(Displacement dis, int w, int h, Pane pane) {

        x1 = new SimpleDoubleProperty();     y1 = new SimpleDoubleProperty();
        x2 = new SimpleDoubleProperty();     y2 = new SimpleDoubleProperty();
        setDisplacement(dis);
        startXProperty().bind(pane.prefWidthProperty().multiply(x1).divide(w));
        startYProperty().bind(pane.prefHeightProperty().multiply(y1).divide(h));
        endXProperty().bind(pane.prefWidthProperty().multiply(x2).divide(w));
        endYProperty().bind(pane.prefHeightProperty().multiply(y2).divide(h));
        setStroke(Color.BLUE);
        setVisible(dis.valid);
    }

    public void setDisplacement(Displacement dis) {
        x1.set(dis.x);
        y1.set(dis.y);
        x2.set(dis.x + dis.u);
        y2.set(dis.y + dis.v);
    }
}
Related