Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8325402

TreeTableRow updateItem() does not check item with isItemChanged(..) unlike all other cell implementations

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Fixed
    • Icon: P4 P4
    • jfx23
    • jfx21
    • javafx
    • None
    • b09
    • generic
    • generic

      TreeTableRow does not check the item with isItemChanged(..), unlike all other implementations of the cell.

      This also means that the TreeTableRow always updates the item, although it should not, resulting in a performance loss as TreeTableRow will always call updateItem().

      It looks like that this was forgotten in JDK-8092593.
      Checking the whole history, the following was happening:
      1. There was a check if the item is the same in all cell implementations (with .equals(..))
      2. Check was removed as it caused bugs
      3. Check was readded, but instead we first check the index (same index) and then if we also have the same item (this time with .isItemChanged(..), to allow developers to subclass it)
      4. Only missing in TreeTableRow.

      # 2 Examples below, first for TreeTableView and then for TableView. Note that in the TableView example he is printing 'isItemChanged'. Especially if you resize a column, you will see only 'isItemChanged'.
      Compare that to the TreeTableView. You will see that 'isItemChanged' is never printed and every resize is triggering an (may) expensive updateItem(..) call.
       
      # Example for TreeTableView, where it does not behave correctly:

      import javafx.application.Application;
      import javafx.beans.property.SimpleStringProperty;
      import javafx.scene.Parent;
      import javafx.scene.Scene;
      import javafx.scene.control.TreeItem;
      import javafx.scene.control.TreeTableColumn;
      import javafx.scene.control.TreeTableRow;
      import javafx.scene.control.TreeTableView;
      import javafx.scene.layout.StackPane;
      import javafx.stage.Stage;

      public class TestTreeTable extends Application {

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

          @Override
          public void start(Stage primaryStage) {
              TreeTableView<P> tableView = new TreeTableView<>();
              tableView.setRowFactory(e -> new TreeTableRow<>() {
                  @Override
                  protected boolean isItemChanged(P oldItem, P newItem) {
                      System.out.println("isItemChanged");
                      return super.isItemChanged(oldItem, newItem);
                  }

                  @Override
                  protected void updateItem(P item, boolean empty) {
                      super.updateItem(item, empty);

                      System.out.println("updateItem");
                  }
              });
              tableView.setRoot(new TreeItem<>());
              tableView.getRoot().getChildren().addAll(new TreeItem<>(new P("aa")));

              TreeTableColumn<P, String> col = new TreeTableColumn<>("ss");
              col.setCellValueFactory(v -> new SimpleStringProperty(v.getValue().getValue() != null ? v.getValue().getValue().name : ""));
              tableView.getColumns().add(col);

              Parent root = new StackPane(tableView);

              Scene scene = new Scene(root, 600, 400);
              primaryStage.setScene(scene);
              primaryStage.show();
          }

          public record P(String name) {

          }

      }

      # Example for TableView, where it behaves correctly:

      ### START ###
      import javafx.application.Application;
      import javafx.beans.property.SimpleStringProperty;
      import javafx.collections.FXCollections;
      import javafx.scene.Parent;
      import javafx.scene.Scene;
      import javafx.scene.control.TableColumn;
      import javafx.scene.control.TableRow;
      import javafx.scene.control.TableView;
      import javafx.scene.layout.StackPane;
      import javafx.stage.Stage;

      public class TestTable extends Application {

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

          @Override
          public void start(Stage primaryStage) {
              TableView<P> tableView = new TableView<>();
              tableView.setRowFactory(e -> new TableRow<>() {
                  @Override
                  protected boolean isItemChanged(P oldItem, P newItem) {
                      System.out.println("isItemChanged");
                      return super.isItemChanged(oldItem, newItem);
                  }

                  @Override
                  protected void updateItem(P item, boolean empty) {
                      super.updateItem(item, empty);

                      System.out.println("updateItem");
                  }
              });
              tableView.setItems(FXCollections.observableArrayList(new P("aa")));

              TableColumn<P, String> col = new TableColumn<>("ss");
              col.setCellValueFactory(v -> new SimpleStringProperty(v.getValue().name));
              tableView.getColumns().add(col);

              Parent root = new StackPane(tableView);

              Scene scene = new Scene(root, 600, 400);
              primaryStage.setScene(scene);
              primaryStage.show();
          }

          public record P(String name) {

          }

      ### END ###

            mhanl Marius Hanl
            mhanl Marius Hanl
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: