0

I'm on Ubuntu 20.04 using OpenJavaFX. I want to have the user press the escape key to toggle the display of a menu. Due to the very high frame rate, I'm struggling to achieve this.

The simple program:

class 
AutoScalingCanvas extends Region {

  private final Canvas canvas;

  public AutoScalingCanvas(double canvasWidth, double canvasHeight) {
    this.canvas = new Canvas(canvasWidth, canvasHeight);
    getChildren().add(canvas);
  }

  public GraphicsContext getGraphicsContext2D() {
    return canvas.getGraphicsContext2D();
  }

  @Override
  protected void layoutChildren() {
    double x = getInsets().getLeft();
    double y = getInsets().getTop();
    double w = getWidth() - getInsets().getRight() - x;
    double h = getHeight() - getInsets().getBottom() - y;

    // preserve aspect ratio while also staying within the available space
    double sf = Math.min(w / canvas.getWidth(), h / canvas.getHeight());
    canvas.setScaleX(sf);
    canvas.setScaleY(sf);

    positionInArea(canvas, x, y, w, h, -1, HPos.CENTER, VPos.CENTER);
  }
}


public class 
Gui extends Application
{
  long target_ns_per_frame = 1_000_000_00 / 60;
  boolean in_menu;
  boolean esc_down;

  @Override
  public void
  start(Stage primary_stage) throws Exception
  {
    primary_stage.setTitle("GUI");

    AutoScalingCanvas canvas = new AutoScalingCanvas(1280, 720);
    Scene scene = new Scene(canvas);
    scene.setFill(Color.BLACK);
    primary_stage.setScene(scene);

    GraphicsContext gc = canvas.getGraphicsContext2D();

    scene.setOnKeyPressed(new EventHandler<KeyEvent>(){
      @Override
      public void handle(KeyEvent event)
      {
        esc_down = (event.getCode() == KeyCode.ESCAPE);
      }
    });

    scene.setOnKeyReleased(new EventHandler<KeyEvent>(){
      @Override
      public void handle(KeyEvent event)
      {
        if (event.getCode() == KeyCode.ESCAPE)
        {
          esc_down = false;
        }
      }
    });

    new AnimationTimer()
    {
      @Override
      public void
      handle(long total_elapsed_time_ns)
      {
        gc.setFill(Color.WHITE);
        gc.fillRect(0, 0, 1280, 720);

        if (esc_down)
        {
          in_menu = !in_menu;
        }

        if (in_menu)
        {
          gc.setFill(Color.BLUE);
          gc.fillRect(300, 300, 200, 200);
        }
        else
        {
          gc.setFill(Color.RED);
          gc.fillRect(100, 100, 100, 100);
        }

        long elapsed_time_ns = System.nanoTime() -
                               total_elapsed_time_ns;
        if (elapsed_time_ns < target_ns_per_frame)
        {
          long time_remaining_ms = (target_ns_per_frame - elapsed_time_ns)
                                    / 1000;
          try {
            Thread.sleep(time_remaining_ms);
          }
          catch (InterruptedException e)
          {

          }
        }
      }
    }.start();

    primary_stage.show();
  }
}

If run without Thread.sleep() the framerate is around 600fps. As a result, pressing the escape key once will be seen as down for a number of frames (due to the speed limit of my human finger) thereby triggering the toggle multiple times. This is obviously not intended. So, I tried to cap the framerate at 60fps. However, with the sleeping, the program runs very slow (perhaps I'm sleeping on the wrong thread?)

How best to keep track of the input to achieve this toggling behavior?

James_D
  • 201,275
  • 16
  • 291
  • 322
  • I think I'm missing something here. Is the desired behavior simply to have the menu show or hide each time the user presses escape? – James_D Jun 10 '21 at 12:31
  • (Removed the multithreading tag as there is no multithreading in the code, or in the question.) – James_D Jun 10 '21 at 12:31

3 Answers3

4

First, you should never block the FX Application Thread by calling Thread.sleep() on it. That will prevent the UI from being updated, or events being handled, until the sleep() is complete.

If the intention is simply that each time the user presses the ESCAPE key that the menu is toggled, then your code is way too complex. Simply toggle a flag indicating if the menu should be painted in the onReleased handler, and check the flag in AnimationTimer.handle():

public class Gui extends Application {
    
    boolean inMenu;

    @Override
    public void start(Stage primaryStage) throws Exception {
        primaryStage.setTitle("GUI");

        AutoScalingCanvas canvas = new AutoScalingCanvas(1280, 720);
        Scene scene = new Scene(canvas);
        scene.setFill(Color.BLACK);
        primaryStage.setScene(scene);

        GraphicsContext gc = canvas.getGraphicsContext2D();


        scene.setOnKeyReleased(event -> {
            if (event.getCode() == KeyCode.ESCAPE) {
                inMenu = ! inMenu;
            }
        });

        new AnimationTimer() {
            @Override
            public void handle(long now) {
                gc.setFill(Color.WHITE);
                gc.fillRect(0, 0, 1280, 720);

                if (inMenu) {
                    gc.setFill(Color.BLUE);
                    gc.fillRect(300, 300, 200, 200);
                } else {
                    gc.setFill(Color.RED);
                    gc.fillRect(100, 100, 100, 100);
                }

            }
        }.start();

        primaryStage.show();
    }
}

If you want to optimize repaints only to when they are needed, simply introduce another flag indicating a repaint is necessary:

public class Gui extends Application {
    
    private boolean inMenu;
    private boolean repaintRequested = true ;

    @Override
    public void start(Stage primaryStage) throws Exception {
        primaryStage.setTitle("GUI");

        AutoScalingCanvas canvas = new AutoScalingCanvas(1280, 720);
        Scene scene = new Scene(canvas);
        scene.setFill(Color.BLACK);
        primaryStage.setScene(scene);

        GraphicsContext gc = canvas.getGraphicsContext2D();


        scene.setOnKeyReleased(event -> {
            if (event.getCode() == KeyCode.ESCAPE) {
                inMenu = ! inMenu;
                repaintRequested = true ;
            }
        });

        new AnimationTimer() {
            @Override
            public void handle(long now) {
                if (repaintRequested) {
                    gc.setFill(Color.WHITE);
                    gc.fillRect(0, 0, 1280, 720);
                    if (inMenu) {
                        gc.setFill(Color.BLUE);
                        gc.fillRect(300, 300, 200, 200);
                    } else {
                        gc.setFill(Color.RED);
                        gc.fillRect(100, 100, 100, 100);
                    } 
                    repaintRequested = false ;
                }

            }
        }.start();

        primaryStage.show();
    }
}
James_D
  • 201,275
  • 16
  • 291
  • 322
  • 1
    I agree to your answer if you include the check if ESCAPE was pressed and no other key – Kenny Jun 10 '21 at 13:03
2

I didn't try any of this, so I can only nudge you in a general direction.

You could add another boolean variable esc_handled you set to true at the end of your handle method. Then you can add one more check to the method if the event has already been handled and if it has, you skip the handling step.

The following code achieves this:

  1. add variable
    boolean in_menu;
    boolean esc_down;
    boolean esc_handled;
    
  2. check for esc_handled (inside handle) and set event to handled
    if (esc_down && !esc_handled)
    {
        in_menu = !in_menu;
        esc_handled = true;
    }
    
  3. on release esc set esc_handled to false
    scene.setOnKeyReleased(new EventHandler<KeyEvent>(){
          @Override
          public void handle(KeyEvent event)
          {
            if (event.getCode() == KeyCode.ESCAPE)
            {
              esc_down = false;
              esc_handled = false;
            }
          }
        });
    
Wai Ha Lee
  • 8,598
  • 83
  • 57
  • 92
Kenny
  • 530
  • 3
  • 12
1

It looks like you're using the animation timer to do some kind of sampling of the state of the in or out and the press/release of the escape key. You don't need to do that at all.

You're trying to turn the key press/release events into a state, which makes sense, but you simply need to toggle that state in the event handler. Since the show/hide action is in response to an event, you can just call the draw routine directly from the event. So then the event will toggle the state and call the screen redraw:

public class Gui extends Application {
    boolean in_menu;
    GraphicsContext gc;

    @Override
    public void start(Stage primary_stage) throws Exception {
        primary_stage.setTitle("GUI");

        AutoScalingCanvas canvas = new AutoScalingCanvas(1280, 720);
        primary_stage.setScene(new Scene(canvas));
        gc = canvas.getGraphicsContext2D();
        showOrHideMenu();
        new Scene(canvas).setOnKeyPressed(event -> {
            if (event.getCode() == KeyCode.ESCAPE) {
                in_menu = !in_menu;
                showOrHideMenu();
            }
        });
        primary_stage.show();
    }

    private void showOrHideMenu() {
        gc.setFill(Color.WHITE);
        gc.fillRect(0, 0, 1280, 720);
        if (in_menu) {
            gc.setFill(Color.BLUE);
            gc.fillRect(300, 300, 200, 200);
        } else {
            gc.setFill(Color.RED);
            gc.fillRect(100, 100, 100, 100);
        }
    }
}

Alternatively (and this is probably more "JavaFX"), you can make the In/Out Menu state observable, and then put a change listener on the state to do the repaint:

public class Gui extends Application {
    BooleanProperty in_menu = new SimpleBooleanProperty(false);
    GraphicsContext gc;

    @Override
    public void start(Stage primary_stage) throws Exception {
        primary_stage.setTitle("GUI");

        AutoScalingCanvas canvas = new AutoScalingCanvas(1280, 720);
        Scene scene = new Scene(canvas);
        primary_stage.setScene(scene);
        gc = canvas.getGraphicsContext2D();
        showOrHideMenu(false);
        scene.setOnKeyPressed(event -> {
            if (event.getCode() == KeyCode.ESCAPE) {
                in_menu.set(!in_menu.get());
            }
        });
        in_menu.addListener(((observable, oldValue, newValue) -> showOrHideMenu(newValue)));
        primary_stage.show();
    }

    private void showOrHideMenu(boolean inMenu) {
        gc.setFill(Color.WHITE);
        gc.fillRect(0, 0, 1280, 720);
        if (inMenu) {
            gc.setFill(Color.BLUE);
            gc.fillRect(300, 300, 200, 200);
        } else {
            gc.setFill(Color.RED);
            gc.fillRect(100, 100, 100, 100);
        }
    }
}
DaveB
  • 1,836
  • 1
  • 15
  • 13