On Mon, 3 Mar 2025 20:44:08 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> I haven't been tracking these fixes for allowing initialization in
>> background threads, but it seems to me that basically anything should be
>> allowed as long as you're not part of a visible scene graph -- and I think
>> there's also no expectation that all functionality of a control "works" as
>> long as it is not yet part of such a graph (ie. the listeners are only
>> needed once it is part of a scene graph).
>>
>> If you make listeners conditional on being part of a scene graph, then I
>> think you can handle these with a single code path. Such a condition would
>> be:
>>
>> ObservableValue<Boolean> partOfSceneGraph = node.sceneProperty()
>> .flatMap(Scene::windowProperty)
>> .flatMap(Window::showingProperty)
>> .orElse(false);
>>
>> // The node here is "this", the chart, because as soon as it is part
>> of the scene
>> // graph, you want the listeners to function.
>>
>> You can then safely install any listeners directly, regardless if they're on
>> a background thread or not:
>>
>> someProperty().when(partOfSceneGraph).subscribe( ... );
>>
>> Or:
>>
>> someProperty().when(partOfSceneGraph).addListener( ... );
>>
>> Or with any mappings added, you can even pick where you put the `when`:
>>
>> someProperty().flatMap(xyz).when(partOfSceneGraph).addListener( ... );
>> someProperty().when(partOfSceneGraph).flatMap(xyz).addListener( ... );
>>
>>
>> On a background thread, `partOfSceneGraph` will be `false`, and no listener
>> gets installed (yet). As soon as it becomes `true` all listeners with that
>> same condition become active. It becomes `true` automatically when the node
>> has a scene belonging to a window that is visible. Vice versa, if it ever
>> becomes `false` again, (which it may when the Node is removed or replaced)
>> all listeners using this condition are removed again.
>
> Thank you!
>
> This is not what I asked though: my question was about using `Subscription`
> in one-off case, and more specifically, about whether it is even possible to
> unsubscribe from within the lambda.
>
> What you are proposing is a slightly more involved change: for example,
> registering and unregistering listeners each time might introduce other
> issues (especially since we have no way to prioritize listeners/handlers).
> It also complicates the management of skin (as skins can be added/removed),
> something I would rather avoid.
As for this current code:
ObservableValue<Window> winProp =
sceneProperty().flatMap(Scene::windowProperty);
accessibilityActive = winProp; // keep the reference so it
won't get gc
Subscription sub = winProp.subscribe((win) -> {
if (win != null) {
if (accessibilityActive == winProp) {
accessibilityActive = null;
}
if (isAccessibilityActive()) {
handleAccessibilityActive(true);
}
//winProp.removeListener(this);
sub.unsubscribe(); <-- COMPILE ERROR
}
});
What you want there is not possible in this way as you can't use `this` in a
lambda, nor refer to it via a local. You can achieve it only if you store the
subscription in a field, not in a local. Assigning the Lambda to a
ChangeListener local, and using a ChangeListener may be simpler (unless you go
for the `when` solution that may eliminate the need for 2 different code paths).
Note that there is no need to store `winProp`; `flatMap` does not use weak
listeners (when in active use), and so as long as `sceneProperty()` exists, the
derived property via `flatMap` will also exist, and also its listener. The way
the fluent mapping system works is that listeners are only present when
something is actually listening:
ObservableValue<Window> winProp =
sceneProperty().flatMap(Scene::windowProperty);
// ^^ nobody is listening, so no listeners installed and no hard
references; it could GC but
// it is currently in a local, so it won't
ObservableValue<Window> winProp =
sceneProperty().flatMap(Scene::windowProperty);
winProp.subscribe(v -> { .. });
// ^^ somebody is listening; all listeners get installed automatically, and
now there are
// hard references. No risk of GC.
You can then write it as a single statement which won't be GC'd as long as the
subscription is active:
subscriptionFIeld = sceneProperty()
.flatMap(Scene::windowProperty);
.subscribe(v -> {
// use subscription field here if you want to unregister the lambda
// Note: when you do, there will again be no listeners and thus no
hard references and
// thus the intermediate property created by flatMap can now be
GC'd :)
});
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1697#discussion_r1978173310
