palexdev / MaterialFX

A library of material components for JavaFX
GNU Lesser General Public License v3.0
1.21k stars 122 forks source link

MFXTextField rendering problem #238

Closed deni5n closed 2 years ago

deni5n commented 2 years ago

Describe the bug When the text is set via a controller method, sometimes the MFXTextField component does not update.

MRE(Minimal Reproducible Example)

public class App extends Application {

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

    @Override
    public void start(Stage primaryStage) throws IOException {
        var main = controllerLoader("main.fxml");
        Parent root = main.load();

        var spOne = controllerLoader("sp-one.fxml");
        Parent startView = spOne.load();

        SpOne controller = spOne.getController();
        controller.setup("someText");

        var sp = (SplitPane) root.lookup("#splitPane");
        sp.getItems().set(0, startView);

        Scene scene = new Scene(root, 500, 500);
        primaryStage.setScene(scene);
        primaryStage.show();

        sp.setDividerPosition(0, 1);
    }

    private FXMLLoader controllerLoader(String path) {
        return new FXMLLoader(Main.class.getResource(path));
    }
}

main.fxml

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.scene.control.SplitPane?>
<?import javafx.scene.layout.AnchorPane?>
<?import javafx.scene.layout.VBox?>
<VBox prefHeight="400" prefWidth="600"
      xmlns="http://javafx.com/javafx/10.0.2-internal" xmlns:fx="http://javafx.com/fxml/1"
      fx:controller="org.example.Main">
    <SplitPane fx:id="splitPane" orientation="VERTICAL" VBox.vgrow="ALWAYS">
        <AnchorPane/>
        <AnchorPane/>
    </SplitPane>
</VBox>

sp-one.fxml

<?xml version="1.0" encoding="UTF-8"?>

<?import io.github.palexdev.materialfx.controls.*?>
<?import javafx.scene.control.*?>
<?import javafx.scene.layout.*?>

<ScrollPane fitToWidth="true"  xmlns="http://javafx.com/javafx/10.0.2-internal" xmlns:fx="http://javafx.com/fxml/1" fx:controller="org.example.SpOne">
    <GridPane>
        <columnConstraints>
            <ColumnConstraints hgrow="NEVER" maxWidth="30.0" minWidth="30.0" prefWidth="100.0" />
            <ColumnConstraints hgrow="ALWAYS" minWidth="350" />
            <ColumnConstraints hgrow="NEVER" maxWidth="30.0" minWidth="30.0" prefWidth="30.0" />
        </columnConstraints>
        <rowConstraints>
            <RowConstraints maxHeight="60.0" minHeight="60.0" prefHeight="60.0" valignment="CENTER" vgrow="NEVER" />
            <RowConstraints valignment="CENTER" vgrow="ALWAYS" />
            <RowConstraints maxHeight="80.0" minHeight="80.0" prefHeight="80.0" valignment="CENTER" vgrow="NEVER" />
            <RowConstraints />
        </rowConstraints>
        <children>

            <VBox spacing="20.0" GridPane.columnIndex="1" GridPane.columnSpan="1" GridPane.rowIndex="1" GridPane.vgrow="SOMETIMES">
                <MFXTextField fx:id="address0" floatMode="BORDER" floatingText="address0" maxWidth="5000" prefHeight="40"  />
                <MFXTextField fx:id="address1" floatMode="BORDER" floatingText="address1" maxWidth="5000" prefHeight="40"  />
                <MFXTextField fx:id="address2" floatMode="BORDER" floatingText="address2" maxWidth="5000" prefHeight="40"  />
                <MFXTextField fx:id="address3" floatMode="BORDER" floatingText="address3" maxWidth="5000" prefHeight="40"  />
                <MFXTextField fx:id="address4" floatMode="BORDER" floatingText="address4" maxWidth="5000" prefHeight="40"  />
            </VBox>
        </children>
    </GridPane>
</ScrollPane>

SpOne.java

public class SpOne implements Initializable {
    public MFXTextField address0;
    public MFXTextField address1;
    public MFXTextField address2;
    public MFXTextField address3;
    public MFXTextField address4;

    public SpOne() { }

    public void setup(String str){
        address0.setText(str);
        address1.setText(str);
        address2.setText(str);
        address3.setText(str);
        address4.setText(str);
    }

    @Override
    public void initialize(URL location, ResourceBundle resources) {
    }
}

Screenshots ksnip_20220819-121410

The problem comes from an optimization in MFXTextFieldSkin. After removing the skipLayout variable everything works as expected.

I can add a PR if needed, let me know.

palexdev commented 2 years ago

I'll investigate The skipLayout flag is there for a reason, I'd like to keep it if possible

deni5n commented 2 years ago

UPDATE

Apparently, these are not all display problems. see screenshot. In some cases, the fields are not displayed correctly. It is fixed if you make any update of the element (click on the field or set the text property programmatically). It always appears right after loading.

I have an assumption that updates do not reach somewhere or some property is cached. I can't do MRE yet

screen

deni5n commented 2 years ago

UPDATE

My previous guess about problems with the skipLayout flag is not correct. Your code is working.

However, I decided to leave a message here, for those who may encounter a similar problem. The true reason for the "weird" behavior of the MFXTextField component and its derivatives is that in some cases the component simply doesn't have enough data to update correctly. In my case, the problem was using a SplitPane. For the purpose of changing the current scene, I use the replacement of one of the children

splitPane.getItems().set(0, newView);

this causes the delimiter to be set to the standard position 0.5, which means it needs to be updated:

splitPane.setDividerPosition(0, newValue);

This is where javafx handles the window change event in a "special" way and in some cases simply "delays" the start of redrawing, respectively, the MFXTextField component does not receive a new height / width value and is not displayed correctly.

To solve this problem, I know two ways: 1. patch MFXTextFieldSkin and PositionUtils 2. "double" setting the separator.

The implementation of the first solution is to disable the skipLayout flag and patch the overloaded layoutChildren function. The main idea here is to pass the height and width to the static method PositionUtils.computePosition and replace them with the height and width of the node for which the position is calculated. I will give an example of the updated computePosition method

public static PositionBean computePosition(Region parent, Node child, double areaX, double areaY, double areaWidth, double areaHeight,
                                               double areaBaselineOffset, Insets margin, HPos hAlignment, VPos vAlignment, double elHeight, double elWidth, boolean snapToPixel) {

        Insets snappedMargin = margin == null ? Insets.EMPTY : margin;

        if (snapToPixel) {
            snappedMargin = InsetsFactory.of(
                    parent.snapSpaceY(snappedMargin.getTop()),
                    parent.snapSpaceX(snappedMargin.getRight()),
                    parent.snapSpaceY(snappedMargin.getBottom()),
                    parent.snapSpaceX(snappedMargin.getLeft())
            );
        }

        double xPosition = computeXPosition(parent, child, areaX, areaWidth, snappedMargin, false, hAlignment, snapToPixel, elWidth);
        double yPosition = computeYPosition(parent, child, areaY, areaHeight, areaBaselineOffset, snappedMargin, false, vAlignment, snapToPixel, elHeight);
        return PositionBean.of(xPosition, yPosition);
    }

Height (elHeight) and width (elWidth) values are passed into it, which are already available at the very first stage of rendering. FloatingText only needs floatH for positioning, while icons (such as those used in MFXDatePicker) need leadingW and leadingH. Accordingly, overloaded versions of computeYPosition and computeXPosition look like this:

public static double computeXPosition(Region parent, Node child, double areaX, double areaWidth, Insets margin, boolean snapMargin, HPos hAlignment, boolean snapToPixel, double elWidth) {
        Insets snappedMargin = margin == null ? Insets.EMPTY : margin;
        if (snapMargin) {
            snappedMargin = InsetsFactory.of(
                    parent.snapSpaceY(snappedMargin.getTop()),
                    parent.snapSpaceX(snappedMargin.getRight()),
                    parent.snapSpaceY(snappedMargin.getBottom()),
                    parent.snapSpaceX(snappedMargin.getLeft())
            );
        }

        final double leftMargin = snappedMargin.getLeft();
        final double rightMargin = snappedMargin.getRight();
        var w = elWidth == -1 ? child.getLayoutBounds().getWidth() : elWidth;
        final double xOffset = leftMargin + computeXOffset(areaWidth - leftMargin - rightMargin, w, hAlignment);
        final double xPosition = areaX + xOffset;
        return snapToPixel ? parent.snapPositionX(xPosition) : xPosition;
    }

    public static double computeYPosition(Region parent, Node child, double areaY, double areaHeight, double areaBaselineOffset, Insets margin, boolean snapMargin, VPos vAlignment, boolean snapToPixel, double elHeight) {
        Insets snappedMargin = margin == null ? Insets.EMPTY : margin;
        if (snapMargin) {
            snappedMargin = InsetsFactory.of(
                parent.snapSpaceY(snappedMargin.getTop()),
                parent.snapSpaceX(snappedMargin.getRight()),
                parent.snapSpaceY(snappedMargin.getBottom()),
                parent.snapSpaceX(snappedMargin.getLeft())
                                            );
        }

        final double topMargin = snappedMargin.getTop();
        final double bottomMargin = snappedMargin.getBottom();
        final double yOffset;

        if (vAlignment == VPos.BASELINE) {
            double bo = child.getBaselineOffset();
            if (bo == Node.BASELINE_OFFSET_SAME_AS_HEIGHT) {
                // We already know the layout bounds at this stage, so we can use them
                yOffset = areaBaselineOffset - child.getLayoutBounds().getHeight();
            } else {
                yOffset = areaBaselineOffset - bo;
            }
        } else {
            var h = elHeight == -1 ? child.getLayoutBounds().getHeight() : elHeight;
            yOffset = topMargin + computeYOffset(areaHeight - topMargin - bottomMargin, h, vAlignment);
        }
        final double yPosition = areaY + yOffset;
        return snapToPixel ? parent.snapPositionY(yPosition) : yPosition;
    }

The implementation of the second solution is to double set the separator. In the method where the scene is replaced, a separator change listener is set, which leads to a screen redraw and, accordingly, the correct display of the MFXTextField component and its derivatives

changeListener = new ChangeListener<>() {
    @Override
    public void changed(ObservableValue<? extends Number> observable, Number oldValue, Number newValue) {
        observable.removeListener(this);
        splitPaneRoot.setDividerPosition(0, (double) newValue);
        }
    };
splitPane.getItems().set(0, newRoot);
splitPane.getDividers().get(0).positionProperty().addListener(changeListener);
splitPane.setDividerPosition(0, newPosition);

Additionally, you need to use the animation trick when expanding the window to full screen, because. this again leads to a change in the separator, correspondingly resizing the window and incorrect rendering of the MFXTextField

palexdev commented 2 years ago

@deni5n wow that's a great study you've made, well done, compliments. As a token of appreciation I'll pin this issue.

I know that the skin for MFXTextField and derivatives is not the best. Maybe better solutions could be implemented but that's not an excuse for computePosition methods to not work as intended

Those methods are just copies of the ones found in the Region class. Because they are super useful I copied them in a separate utility class to make them public and available for everyone. With the default parameters they should work always with no exceptions, or at least that's what I'm expecting

The skipLayout flag is there because sometimes there's no need to execute layoutChildren again, this is true for example for subsequent layout requests for which the layout state is practically the same (width, height, properties)

I'll take a look at the skin in the future and see if anything can be done to make it lighter and more stable at the same time. As of now I'm still working on VirtualizedFX (which is delaying updates on MaterialFX, sorry) so I don't know when I will be able to work on it