Ensures proper propagation of layout flags when using `forceParentLayout =
true`.
This was the root cause of issue #1874
### Note
Apparently it is still quite easy to mess up the layout flags. Basically, the
layout flag tracked by Parent should always either be `CLEAN` for any scene
graph branch, or `!CLEAN` + a layout pulse is scheduled on the corresponding
`Scene`.
However, with careful use of the public API `requestLayout` one can get these
flags in a bad state still:
Let's say I have a branch `A (root node under Scene) -> B -> C`, **and** a
layout is in progress, **and** we're currently in the `layoutChildren` method
of `C`. The flag `performingLayout` will be `true` for all nodes in the branch
`A` -> `B` -> `C`. The `layout` method will set the layout flag to `CLEAN` as
its first action, so when we're at `C::layoutChildren`, all flags have been
reset to `CLEAN` already. See the `Parent::layout` method for how all this
works.
Now, to mess up the flags, all you need to do is call `requestLayout` on `B` or
`C` from the `layoutChildren` of `C` (or indirectly by changing something and
something is listening to this and schedules a layout on something somewhere in
this branch); note that `requestLayout` is not documented to be illegal to call
during layout, and some classes in FX will do so (ScrollPaneSkin, NumberAxis,
etc..) risking the flags getting in a bad state... -- usually you get away with
this, as there are many ways that layout is triggered, and eventually the flags
will get overwritten and reset to a consistent state.
The bad state occurs because this code path is followed (all code from
`Parent`):
public void requestLayout() {
clearSizeCache();
markDirtyLayout(false, forceParentLayout);
}
Calls to `markDirtyLayout(false, false)`:
private void markDirtyLayout(boolean local, boolean forceParentLayout) {
setLayoutFlag(LayoutFlags.NEEDS_LAYOUT);
if (local || layoutRoot) {
if (sceneRoot) {
Toolkit.getToolkit().requestNextPulse();
if (getSubScene() != null) {
getSubScene().setDirtyLayout(this);
}
} else {
markDirtyLayoutBranch();
}
} else {
requestParentLayout(forceParentLayout);
}
}
Before going into the `else` (as none of the nodes is a layout root, and
`local` was set to `false`) it will do
**setLayoutFlag(LayoutFlags.NEEDS_LAYOUT)** -- this will set a flag on some
node; to eventually end up in a consistent state, it must mark all ancestors as
well and schedule a pulse (with `Toolkit.getToolkit().requestNextPulse()`)...
but:
void requestParentLayout(boolean forceParentLayout) {
if (!layoutRoot) {
final Parent p = getParent();
if (p != null && (!p.performingLayout || forceParentLayout)) {
/*
* The forceParentLayout flag must be propagated to mark all
ancestors
* as needing layout. Failure to do so while performingLayout
is true
* would stop the propagation mid-tree. This leaves some nodes
as needing
* layout, while its ancestors are clean, which is an
inconsistent state.
*/
p.requestLayout(forceParentLayout);
}
}
}
Here there is a guard `!p.isPerformingLayout`, blocking propagation up the
tree. As said, this flag is `true` for all nodes during a layout of the same
branch. The end result thus is that some nodes have their layout flag changed
to `NEEDS_LAYOUT`, but it was not propagated, nor was a pulse scheduled...
-------------
Commit messages:
- Remove trailing whitespace
- Propagate forceParentLayout flag to prevent inconsistent layout state
Changes: https://git.openjdk.org/jfx/pull/1879/files
Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1879&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8360940
Stats: 10 lines in 1 file changed: 8 ins; 0 del; 2 mod
Patch: https://git.openjdk.org/jfx/pull/1879.diff
Fetch: git fetch https://git.openjdk.org/jfx.git pull/1879/head:pull/1879
PR: https://git.openjdk.org/jfx/pull/1879
