One of the dangers of mucking around with the CSS code is whether or not
the changes break things like popups, dialogs, menus. And whether or not
the change breaks inline styles versus attributes set in code, versus
stylesheets added to the scene/subscene/control, versus default
stylesheets. And whether or not the changes breaks skins for controls.
Reapplying CSS to a Node but not its children could cause a problem if
there are styles in the parent or the parent's parents that affect the
children. It seems like bypassing children in reapplyCSS is bound to
cause a regression.
Also, because a new scene root could affect styles, CSS is reapplied
after calling sceneChanged - your change puts it before, so I question
whether or not this will cause a regression. I think if you skip
reapplying CSS when a Node's scene has changed, then the children won't
get the correct styles, and this will affect layout.
I haven't spent much time evaluating this change in detail, but I'm
doubtful that it won't cause regressions.
On 11/7/18 2:57 AM, Dean Wookey wrote:
Hi,
I was going to ask if it was possible to reopen JDK-8151756 (
https://bugs.openjdk.java.net/browse/JDK-8151756) since it was fixed but
reverted in JDK-8183100 (https://bugs.openjdk.java.net/browse/JDK-8183100)
after causing several regressions. I only noticed now that a followup bug
was created: JDK-8193445 which reopens the issue.
The code below demonstrates the problem where adding nodes to the scene
graph all at once performs exponentially slower than adding them one by
one. I get the following results with jfx11.0.1:
One by one: 138ms
All at once: 16704ms
I've made a potential fix, different to the one tried previously which
applies the css as if the nodes were being added one by one:
https://github.com/DeanWookey/openjdk-jfx/commit/65a1ed82bce262294f1969e9a12e1126ec8a1ec6
It passes the main tests, as well as the systemTest JDK8183100Test.java
from https://bugs.openjdk.java.net/browse/JDK-8193494.
This is probably not a suitable issue to work on for a first time
contributor, but perhaps I could work on a performance test if someone can
point me in the direction of existing performance tests?
Dean
package applycsstest;
import javafx.application.Application;
import javafx.application.Platform;
import javafx.scene.Scene;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;
/**
*
* @author Dean
*/
public class ApplyCssTest extends Application {
@Override
public void start(Stage primaryStage) {
System.out.println("One by one: " + addToSceneOneByOne() + "ms");
System.out.println("All at once: " + addToSceneAllAtOnce() + "ms");
Platform.exit();
}
public static void main(String[] args) {
launch(args);
}
public long addToSceneOneByOne() {
StackPane root = new StackPane();
Scene scene = new Scene(root, 300, 250);
long start = System.currentTimeMillis();
StackPane firstChild = new StackPane();
root.getChildren().add(firstChild); //add to the scene graph first
addNodesOneByOne(1000, firstChild); //then add children one by one
return System.currentTimeMillis() - start;
}
public long addToSceneAllAtOnce() {
StackPane root = new StackPane();
Scene scene = new Scene(root, 300, 250);
long start = System.currentTimeMillis();
StackPane firstChild = new StackPane();
addNodesOneByOne(1000, firstChild); //build the node up, then
root.getChildren().add(firstChild); //add to scene graph all at once
return System.currentTimeMillis() - start;
}
public void addNodesOneByOne(int numToAdd, StackPane parent) {
StackPane last = parent;
for (int i = 0; i < numToAdd; i++) {
StackPane curr = new StackPane();
last.getChildren().add(curr);
last = curr;
}
}
}
