I think this is a good discussion to continue. I have a a couple quick
comments:
The first question I would like to resolve is to determine whether the
problem exists globally, or only at the controls level. If even once
scenario exists that does not involve controls, we must find a
solution at the event dispatch level. If not - the solution can be at
the controls level, and I have proposed a good solution, but it's
premature to talk about it right now.
Unless it can be clearly shown that this is a controls-only problem, and
never will be something that other users of events need to worry about,
I favor a solution in the event handling mechanism itself rather than
something controls-specific. So I agree with Michael on this point.
4, 5. there seems to be general misunderstanding why I see copyFor()
as a big problem. (Performance is **not** the issue here).
Very likely. I certainly don't see it as a big problem, which suggests I
might be missing something. I do find it unlikely that we are going to
change something as fundamental as having a target in the event (which
is the main reason for using "copyFor").
-- Kevin
On 11/12/2024 8:27 AM, Andy Goryachev wrote:
Thank you Michael for answering my questions!
I get from your answers that:
1. the priorities are still needed, in one form or the other. Adding a
different type of the EH (ifUnconsumed) seems to me like a different
priority.
2. the problem seems to exist only at the controls level - nothing was
mentioned to cause issues related to priority outside of controls.
This seems right, because only in controls we have two (or more)
actors engaged in event handling - the application and the skin.
3. dispatching consumed events looks like a bug to all respondents
4, 5. there seems to be general misunderstanding why I see copyFor()
as a big problem. (Performance is ***not*** the issue here).
Please correct me if I summarized it incorrectly.
Another interesting observation is that proposals seem to have been
replaced by widely different alternatives - ifUnconsumed and event
filters. This might indicate that there is no consensus as of yet,
and the discussion must therefore be continued.
The first question I would like to resolve is to determine whether the
problem exists globally, or only at the controls level. If even once
scenario exists that does not involve controls, we must find a
solution at the event dispatch level. If not - the solution can be at
the controls level, and I have proposed a good solution, but it's
premature to talk about it right now.
So I would like to ask for clarifications on these three questions:
1. For ifUnconsumed idea: how will it work when both the application
and the skin register ifUnconsumed EH? Or is it only available to one
side, but not the other?
2. For event filter in behaviors idea: how does it work when both
behavior and the application register an event filter? and then the
skin is changed? wouldn't we have the same issue?
3. Are there any examples outside of controls where priority inversion
happens, or where we need explicit EH priorities for other reasons?
Thank you
-andy
*From: *openjfx-dev <openjfx-dev-r...@openjdk.org> on behalf of
Michael Strauß <michaelstr...@gmail.com>
*Date: *Friday, November 8, 2024 at 17:52
*To: *
*Cc: *openjfx-dev <openjfx-dev@openjdk.org>
*Subject: *Re: Prioritized event handlers
Hi Andy,
1. What happened to this proposal?
I've come to the conclusion that we need something like that, but
probably in a different form. My current thinking is that we don't
need prioritized handlers, but merely a way for interested listeners
to say "I'll take this event, but only if no one else wants it".
A possible API could be something like the following:
target.addEventHandler(KeyEvent.PRESSED, event -> {
event.ifUnconsumed(evt -> {
// This will be called after the event has bubbled up
// without being consumed.
});
});
This will allow skins to act on events only if user code didn't
consume them.
2. Does it make sense to change the API at the EventDispatcher level
when the problem can be easily solved by the InputMap at the Control
level?
Yes, because javafx.controls is not a core part of JavaFX, and it
should never be. People should be free to create their own controls
implementation, or alternative skinning systems. We need to give them
the tools to do so, and not continue the anti-pattern of shifting core
functionality into javafx.controls and special-casing this module even
more than it is already special-cased.
3. dispatching of events that have been consumed (as mentioned in the
earlier discussion)
Probably not necessary. Once an event is consumed, it's gone; we don't
need to dispatch it further.
4. Problem of creating unnecessary clones of events via Event.copyFor()
Unless there is a clear performance problem, I consider any
fundamental change here as a solution in search of a problem.
Events are usually not so plentiful that we're talking about serious
CPU cycles here. The highest-frequency events are probably mouse
events, and they happen at most hundreds of times per second.
5. If we removed the target, then a listener couldn't discern whether
the event was targeted at the receiving node, or at a descendant of
the node.
On Thu, Nov 7, 2024 at 1:03 AM Andy Goryachev
<andy.goryac...@oracle.com> wrote:
>
> Dear Michael:
> What happened to this proposal? I would like to restart the
discussion, if possible.
>
> More specifically, I would like to discuss the following topics:
>
> the reason the discussion was started was due to "priority
inversion" problem in Controls/Skins, ex.: JDK-8231245 Controls'
behavior must not depend on sequence of handler registration. Do we
have this problem elsewhere? In other words, does it make sense to
change the API at the EventDispatcher level when the problem can be
easily solved by the InputMap at the Control level?
> dispatching of events that have been consumed (as mentioned in the
earlier discussion)
> problem of creating unnecessary clones of events via
Event.copyFor(), leading to ex.: JDK-8337246 SpinnerSkin does not
consume ENTER KeyEvent when editor ActionEvent is consumed
> why do we need Event.copyFor() in the first place? why does Event
contain the target??
>
