From a692c2bbe6a5b0df310a825afdc081fcfb540393 Mon Sep 17 00:00:00 2001 From: Bruno Salmon Date: Tue, 12 Sep 2023 09:11:45 +0100 Subject: [PATCH] Fixed possible lost of viewport position in HtmlScrollPanePeer --- .../gwt/html/HtmlScrollPanePeer.java | 7 ++ .../emul_coupling/CanvasScenePeer.java | 2 +- .../emul_coupling/ScenePeer.java | 2 +- .../src/main/java/javafx/scene/Node.java | 75 +++++++++++++------ .../src/main/java/javafx/scene/Parent.java | 70 +++++++++-------- .../src/main/java/javafx/scene/Scene.java | 68 ++++++++--------- .../gwt/html/HtmlScenePeer.java | 22 +++--- .../javafxgraphics/gwt/svg/SvgScenePeer.java | 2 +- 8 files changed, 145 insertions(+), 103 deletions(-) diff --git a/webfx-kit/webfx-kit-javafxcontrols-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxcontrols/gwt/html/HtmlScrollPanePeer.java b/webfx-kit/webfx-kit-javafxcontrols-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxcontrols/gwt/html/HtmlScrollPanePeer.java index dd4649d0ef..eebf11a132 100644 --- a/webfx-kit/webfx-kit-javafxcontrols-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxcontrols/gwt/html/HtmlScrollPanePeer.java +++ b/webfx-kit/webfx-kit-javafxcontrols-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxcontrols/gwt/html/HtmlScrollPanePeer.java @@ -1,5 +1,6 @@ package dev.webfx.kit.mapper.peers.javafxcontrols.gwt.html; +import dev.webfx.kit.util.properties.FXProperties; import elemental2.dom.Element; import elemental2.dom.HTMLElement; import javafx.geometry.BoundingBox; @@ -39,6 +40,12 @@ public void bind(N node, SceneRequester sceneRequester) { setChildrenContainer(psContainer); HtmlUtil.setChildren(getElement(), psContainer); node.setOnChildrenLayout(HtmlScrollPanePeer.this::scheduleUpdate); + // The following listener is to reestablish the scroll position on scene change. For ex when the user 1) switches + // to another page through UI routing and then 2) come back, this node is removed from the scene graph at 1) => + // scene = null until 2) => scene reestablished, but Perfect scrollbar lost its state when removed from the DOM. + // This listener will trigger a schedule update at 2) which will restore the perfect scrollbar state (scrollTop + // & scrollLeft will be reapplied). + FXProperties.runOnPropertiesChange(this::scheduleUpdate, node.sceneProperty()); } private double scrollTop, scrollLeft; diff --git a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/emul_coupling/CanvasScenePeer.java b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/emul_coupling/CanvasScenePeer.java index 865640db29..61871182c7 100644 --- a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/emul_coupling/CanvasScenePeer.java +++ b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/emul_coupling/CanvasScenePeer.java @@ -24,7 +24,7 @@ public CanvasScenePeer(Scene scene) { } @Override - public void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange) { + public void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange) { scene.updateChildrenPeers(parent.getChildren()); requestCanvasRepaint(); } diff --git a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/emul_coupling/ScenePeer.java b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/emul_coupling/ScenePeer.java index 019accef3f..dc0d2ab419 100644 --- a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/emul_coupling/ScenePeer.java +++ b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/emul_coupling/ScenePeer.java @@ -14,7 +14,7 @@ public interface ScenePeer extends TKScene { Scene getScene(); - void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange); + void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange); NodePeer pickPeer(double sceneX, double sceneY); diff --git a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Node.java b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Node.java index 407e1527ab..6c4af25d6a 100644 --- a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Node.java +++ b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Node.java @@ -49,12 +49,57 @@ */ public abstract class Node implements INode, EventTarget, Styleable { - private final Property parentProperty = new SimpleObjectProperty<>(); + + // This method is called when the node peer has been created, inserted to the DOM, and bound to this node (i.e. + // reacting to the properties changes to update the HTML mapping). The node may need to do something at this point. + void onNodePeerBound() { } + + private final Property parentProperty = new SimpleObjectProperty<>() { + protected void invalidated() { + // The child automatically inherits the scene from the parent (if the parent is null, the scene is set to + // null for this child) + Parent parent = getParent(); + Scene newScene = parent == null ? null : parent.getScene(); + setScene(newScene); // Note: the scene will be propagated to the possible children in the scene listener (see below) + } + }; @Override public Property parentProperty() { return parentProperty; } + private final ObjectProperty scene = new SimpleObjectProperty<>() { + @Override + protected void invalidated() { + Scene newScene = getScene(); + // Initialising the event dispatcher if not already done + if (newScene != null) + newScene.initializeInternalEventDispatcher(); + // Propagating the scene to the children + if (Node.this instanceof Parent) { + for (Node child : ((Parent) Node.this).getChildren()) { + child.setScene(newScene); + } + } + // Also to the clip (if set) + Node clip = getClip(); + if (clip != null) + clip.setScene(newScene); + } + }; + + public ObjectProperty sceneProperty() { + return scene; + } + + public Scene getScene() { + return scene.getValue(); + } + + public void setScene(Scene scene) { + this.scene.setValue(scene); + } + private final Property managedProperty = new SimpleObjectProperty(true) { @Override protected void invalidated() { @@ -64,6 +109,7 @@ protected void invalidated() { notifyManagedChanged(); } }; + @Override public Property managedProperty() { return managedProperty; @@ -636,7 +682,7 @@ public ObservableList getTransforms() { private Translate layoutTranslateTransform; private Scale scaleTransform; private Rotate rotateTransform; - + @Override public List getAllNodeTransforms() { // We need to consider all transforms: I) those declared directly at the Node level via translateX/Y, rotateX/Y @@ -648,8 +694,8 @@ public List getAllNodeTransforms() { double ltX = getTranslateX(); double ltY = getTranslateY(); //if (!(getParent() instanceof Group)) { // Breaks EnzoClocks, TallyCounter and SpaceFX demos - ltX += getLayoutX(); - ltY += getLayoutY(); + ltX += getLayoutX(); + ltY += getLayoutY(); //} if (ltX != 0 || ltY != 0) { if (layoutTranslateTransform == null) @@ -1288,25 +1334,6 @@ public void requestPeerFocus() { peerFocusRequested = true; } - private ObjectProperty scene = new SimpleObjectProperty<>(); - - public ObjectProperty sceneProperty() { - return scene; - } - - public Scene getScene() { - return scene.getValue(); - } - - public void setScene(Scene scene) { - this.scene.setValue(scene); - Node clip = getClip(); - if (clip != null) - clip.setScene(scene); - if (scene != null) - scene.initializeInternalEventDispatcher(); - } - private LayoutMeasurable layoutMeasurable; public LayoutMeasurable getLayoutMeasurable() { @@ -1338,7 +1365,7 @@ protected void createLayoutMeasurable(NodePeer nodePeer) { // Always creating a new LayoutMeasurable (even when nodePeer is valid) so that min/pref/max // width/height user values are returned in priority whenever they have been set. layoutMeasurable = new LayoutMeasurable() { - private LayoutMeasurable acceptedLayoutMeasurable = nodePeer instanceof LayoutMeasurable && shouldUseLayoutMeasurable() ? (LayoutMeasurable) nodePeer : null; + private final LayoutMeasurable acceptedLayoutMeasurable = nodePeer instanceof LayoutMeasurable && shouldUseLayoutMeasurable() ? (LayoutMeasurable) nodePeer : null; { if (nodePeer instanceof HasSizeChangedCallback) UiScheduler.scheduleDeferred(() -> diff --git a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Parent.java b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Parent.java index c49e655b56..9d717052f3 100644 --- a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Parent.java +++ b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Parent.java @@ -3,15 +3,17 @@ import com.sun.javafx.geom.BaseBounds; import com.sun.javafx.geom.RectBounds; import com.sun.javafx.geom.transform.BaseTransform; +import dev.webfx.kit.mapper.peers.javafxgraphics.markers.HasManagedProperty; +import dev.webfx.kit.util.properties.FXProperties; +import dev.webfx.kit.util.properties.ObservableLists; import javafx.beans.property.Property; import javafx.beans.property.SimpleObjectProperty; +import javafx.beans.value.ObservableValue; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; import javafx.scene.layout.LayoutFlags; import javafx.scene.layout.PreferenceResizableNode; -import dev.webfx.kit.mapper.peers.javafxgraphics.markers.HasManagedProperty; -import dev.webfx.kit.util.properties.ObservableLists; import java.util.ArrayList; import java.util.List; @@ -23,40 +25,48 @@ public class Parent extends Node { private final ObservableList children = FXCollections.observableArrayList(); - { - children.addListener((ListChangeListener) c -> { - Scene scene = getScene(); + children.addListener(this::onChildrenChanged); + } + + private void onChildrenChanged(ListChangeListener.Change c) { + // This listener has 2 main tasks: 1) propagate this parent change to the children & 2) ask the scene to update + // the peers structure (scene graph => DOM tree mapping). + + // Note: the following sequence is the only one that works to preserve the HTML scrollbar state when removed and + // put back to the DOM (perfectscrollbar.js loses its state when removed from the DOM). See HtmlScrollPanePeer. + + // First we propagate this parent to the children. This is half of the job for 1) as we also need to null the + // parent to the removed children, but it's important to not do it yet at this stage. + for (Node child : getChildren()) + child.setParent(this); + // Then we do 2) i.e. call scene.updateParentAndChildrenPeers() + Scene scene = getScene(); // Of course scene needs to be non-null and the node peer of this parent needs to be created + if (scene != null && getNodePeer() != null) { + scene.updateParentAndChildrenPeers(this, c); + } + // We do the second part of the job for 1) here, which is to null the parent of the removed children. + if (c != null) { + c.reset(); // Because scene.updateParentAndChildrenPeers() already used it. while (c.next()) { - if (c.wasAdded()) - for (Node child : c.getAddedSubList()) { - setAndPropagateScene(child, scene); - child.setParent(this); - } - else if (c.wasRemoved())// Setting parent and scene to null for removed children + if (c.wasRemoved())// Setting parent and scene to null for removed children for (Node child : c.getRemoved()) { - child.setParent(null); - setAndPropagateScene(child, null); + if (child.getParent() == Parent.this) + child.setParent(null); } } - managedChildChanged(); - }); - } - - static void setAndPropagateScene(Node node, Scene scene) { - node.setScene(scene); - if (node instanceof Parent) { - for (Node child : ((Parent) node).getChildren()) { - //if (child.getScene() != scene) // Commented this optimisation as it prevents the Mandelbrot canvas thumbnail to get the scene back when returning to home page - setAndPropagateScene(child, scene); -/* Commented this optimization as it was causing problems in Mandelbrot demo: when going back to first page, 1 or 2 thumbnails had the scene set and not others - else // If already done for this child, assuming it's done for others - break; -*/ - } } + // Final detail, we need to call managedChildChanged() + managedChildChanged(); } + @Override + void onNodePeerBound() { + FXProperties.onPropertySet((ObservableValue) getProperties().get("skinProperty"), + skin -> onChildrenChanged(null), true); + } + + Parent() { } @@ -636,8 +646,8 @@ private void recomputeBounds() { ? dirtyChildren : children, dirtyChildrenCount)) */ - // failed to update cached bounds, recreate them - createCachedBounds(children); + // failed to update cached bounds, recreate them + createCachedBounds(children); } // Note: this marks the currently processed child in terms of transformed bounds. In rare situations like diff --git a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Scene.java b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Scene.java index 67c8824a6c..00cb442dc2 100644 --- a/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Scene.java +++ b/webfx-kit/webfx-kit-javafxgraphics-emul/src/main/java/javafx/scene/Scene.java @@ -16,7 +16,6 @@ import dev.webfx.kit.mapper.peers.javafxgraphics.markers.HasHeightProperty; import dev.webfx.kit.mapper.peers.javafxgraphics.markers.HasRootProperty; import dev.webfx.kit.mapper.peers.javafxgraphics.markers.HasWidthProperty; -import dev.webfx.kit.util.properties.FXProperties; import dev.webfx.kit.util.properties.ObservableLists; import dev.webfx.platform.scheduler.Scheduled; import dev.webfx.platform.uischeduler.AnimationFramePass; @@ -185,7 +184,7 @@ protected void invalidated() { createAndBindRootNodePeerAndChildren(root); // Resetting the scene to null for the old root and its children if (oldRoot != null && oldRoot.getScene() == Scene.this) - Parent.setAndPropagateScene(oldRoot, null); + oldRoot.setScene(null); oldRoot = root; } }; @@ -652,11 +651,12 @@ private void keepParentAndChildrenPeersUpdated(Parent parent) { // Setting the parent to all children for (Node child : parent.getChildren()) child.setParent(parent); - updateParentAndChildrenPeers(parent, (ListChangeListener.Change) c); + // Note: + updateParentAndChildrenPeers(parent, c); }, parent.getChildren()); } - private void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange) { + void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange) { impl_getPeer().updateParentAndChildrenPeers(parent, childrenChange); } @@ -704,9 +704,7 @@ public NodePeer getOrCreateAndBindNodePeer(Node node) { node.setNodePeer(nodePeer = createUnimplementedNodePeer(node)); // Displaying a "Unimplemented..." button instead else { // Standard case (the node view was successfully created) nodePeer.bind(node, sceneRequester); - if (node instanceof Parent) - FXProperties.onPropertySet((ObservableValue) node.getProperties().get("skinProperty"), - skin -> keepParentAndChildrenPeersUpdated((Parent) node), true); + node.onNodePeerBound(); } node.callNodePeerHandlers(); } @@ -1185,36 +1183,36 @@ public void changedSize(float w, float h) { if (h != Scene.this.getHeight()) Scene.this.setHeight((double)h); } -/* - @Override - public void mouseEvent(EventType type, double x, double y, double screenX, double screenY, - MouseButton button, boolean popupTrigger, boolean synthesized, - boolean shiftDown, boolean controlDown, boolean altDown, boolean metaDown, - boolean primaryDown, boolean middleDown, boolean secondaryDown) - { - MouseEvent mouseEvent = new MouseEvent(type, x, y, screenX, screenY, button, - 0, // click count will be adjusted by clickGenerator later anyway - shiftDown, controlDown, altDown, metaDown, - primaryDown, middleDown, secondaryDown, synthesized, popupTrigger, false, null); - impl_processMouseEvent(mouseEvent); - } + /* + @Override + public void mouseEvent(EventType type, double x, double y, double screenX, double screenY, + MouseButton button, boolean popupTrigger, boolean synthesized, + boolean shiftDown, boolean controlDown, boolean altDown, boolean metaDown, + boolean primaryDown, boolean middleDown, boolean secondaryDown) + { + MouseEvent mouseEvent = new MouseEvent(type, x, y, screenX, screenY, button, + 0, // click count will be adjusted by clickGenerator later anyway + shiftDown, controlDown, altDown, metaDown, + primaryDown, middleDown, secondaryDown, synthesized, popupTrigger, false, null); + impl_processMouseEvent(mouseEvent); + } - @Override - public void keyEvent(KeyEvent keyEvent) - { - impl_processKeyEvent(keyEvent); - } + @Override + public void keyEvent(KeyEvent keyEvent) + { + impl_processKeyEvent(keyEvent); + } - @Override - public void inputMethodEvent(EventType type, - ObservableList composed, String committed, - int caretPosition) - { - InputMethodEvent inputMethodEvent = new InputMethodEvent( - type, composed, committed, caretPosition); - processInputMethodEvent(inputMethodEvent); - } - */ + @Override + public void inputMethodEvent(EventType type, + ObservableList composed, String committed, + int caretPosition) + { + InputMethodEvent inputMethodEvent = new InputMethodEvent( + type, composed, committed, caretPosition); + processInputMethodEvent(inputMethodEvent); + } + */ public void menuEvent(double x, double y, double xAbs, double yAbs, boolean isKeyboardTrigger) { Scene.this.processMenuEvent(x, y, xAbs,yAbs, isKeyboardTrigger); diff --git a/webfx-kit/webfx-kit-javafxgraphics-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/gwt/html/HtmlScenePeer.java b/webfx-kit/webfx-kit-javafxgraphics-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/gwt/html/HtmlScenePeer.java index 8985aac92d..f5f7b9ec86 100644 --- a/webfx-kit/webfx-kit-javafxgraphics-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/gwt/html/HtmlScenePeer.java +++ b/webfx-kit/webfx-kit-javafxgraphics-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/gwt/html/HtmlScenePeer.java @@ -72,15 +72,15 @@ private void installMouseListeners() { //e.stopPropagation(); e.preventDefault(); // To prevent the browser default context menu if (e instanceof MouseEvent // For now we manage only context menu from the mouse - // Also checking that we received the mouse up event on that scene before. This is to prevent the - // following case: when a context menu is already displayed (=> in another popup window/scene) and - // the user right-click on a menu item, the menu action will be triggered on the mouseup within the - // popup window/scene (so far, so good) but then oncontextmenu is called on this scene by the browser - // whereas the intention of the user was just to trigger the menu action (which also closes the - // context menu) but not to display the context menu again. So we prevent this by checking the last - // mouse event was a mouse up on that scene which is the correct sequence (in the above case, the - // last mouse event will be the oncontextmenu event). - /*&& lastMouseEvent != null && "mouseup".equals(lastMouseEvent.type)*/) { + // Also checking that we received the mouse up event on that scene before. This is to prevent the + // following case: when a context menu is already displayed (=> in another popup window/scene) and + // the user right-click on a menu item, the menu action will be triggered on the mouseup within the + // popup window/scene (so far, so good) but then oncontextmenu is called on this scene by the browser + // whereas the intention of the user was just to trigger the menu action (which also closes the + // context menu) but not to display the context menu again. So we prevent this by checking the last + // mouse event was a mouse up on that scene which is the correct sequence (in the above case, the + // last mouse event will be the oncontextmenu event). + /*&& lastMouseEvent != null && "mouseup".equals(lastMouseEvent.type)*/) { MouseEvent me = (MouseEvent) e; // By the way, we set the correction for window.screenY (see GwtWindowPeer comment), knowing that pageY // is giving the correct value for the context menu in the page coordinates. @@ -262,7 +262,7 @@ public void onRootBound() { } @Override - public void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange) { + public void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange) { if (!(parent instanceof HasNoChildrenPeers)) { HtmlSvgNodePeer parentPeer = HtmlSvgNodePeer.toNodePeer(parent, scene); //long t0 = System.currentTimeMillis(); @@ -283,7 +283,7 @@ public void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Chang } } - private List toChildElements(List nodes) { + private List toChildElements(List nodes) { return Collections.map(nodes, this::toChildElement); } diff --git a/webfx-kit/webfx-kit-javafxgraphics-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/gwt/svg/SvgScenePeer.java b/webfx-kit/webfx-kit-javafxgraphics-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/gwt/svg/SvgScenePeer.java index a5ff542ea9..cae9e9eb55 100644 --- a/webfx-kit/webfx-kit-javafxgraphics-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/gwt/svg/SvgScenePeer.java +++ b/webfx-kit/webfx-kit-javafxgraphics-peers-gwt/src/main/java/dev/webfx/kit/mapper/peers/javafxgraphics/gwt/svg/SvgScenePeer.java @@ -100,7 +100,7 @@ public void onRootBound() { } @Override - public void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange) { + public void updateParentAndChildrenPeers(Parent parent, ListChangeListener.Change childrenChange) { HtmlSvgNodePeer parentPeer = HtmlSvgNodePeer.toNodePeer(parent, scene); HtmlUtil.setChildren(parentPeer.getChildrenContainer(), Collections.map(parent.getChildren(), node -> HtmlSvgNodePeer.toContainerElement(node, scene))); }