<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
On 07/11/2024 20:06, Andy Goryachev wrote:<br>
<blockquote type="cite"
cite="mid:BL3PR10MB61851CB9CB5BD024A9534B55E55C2@BL3PR10MB6185.namprd10.prod.outlook.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<meta name="Generator"
content="Microsoft Word 15 (filtered medium)">
<style>@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}@font-face
{font-family:"Yu Gothic";
panose-1:2 11 4 0 0 0 0 0 0 0;}@font-face
{font-family:Aptos;
panose-1:2 11 0 4 2 2 2 2 2 4;}@font-face
{font-family:"Iosevka Fixed SS16";
panose-1:2 0 5 9 3 0 0 0 0 4;}@font-face
{font-family:"Times New Roman \(Body CS\)";
panose-1:2 11 6 4 2 2 2 2 2 4;}@font-face
{font-family:"\@Yu Gothic";
panose-1:2 11 4 0 0 0 0 0 0 0;}@font-face
{font-family:"Helvetica Neue";
panose-1:2 0 5 3 0 0 0 2 0 4;}p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:10.0pt;
font-family:"Aptos",sans-serif;}a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}p.li1, li.li1, div.li1
{mso-style-name:li1;
margin:0in;
font-size:10.0pt;
font-family:"Helvetica Neue";}span.apple-converted-space
{mso-style-name:apple-converted-space;}span.EmailStyle23
{mso-style-type:personal-reply;
font-family:"Iosevka Fixed SS16";
color:windowtext;}.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;
mso-ligatures:none;}div.WordSection1
{page:WordSection1;}ol
{margin-bottom:0in;}ul
{margin-bottom:0in;}</style>
<div class="WordSection1">
<p style="margin-left:.5in">> 4. The target allows you to
register the same event handler instance multiple times on
multiple different controls without having to create a new
lambda each time that knows which control (target) it is for.
It has a similar function to the Observable parameter in
InvalidationListeners and ChangeListeners. In effect, having
the Target will also make it possible in the future to share a
single input map with all controls of the same type (currently
there is a ton of memory being wasted here duplicating input
maps, especially for controls with large input maps like
TextFields).<o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">This
is a very strange rationale, in my opinion. And why talk
about Controls when the Event.copyFor() is at the very
fundamental level? To me, this severely complicates
everything, including breaking isConsumed() logic. Sending
events that are not designed to bubble up seems to run
contrary to the whole event dispatching idea. Are you
trying to explain the current implementation? Swing never
needed to store the target in the event, and it works just
fine.</span></p>
</div>
</blockquote>
<p>FX is not Swing, they're designed quite differently in many
areas, and trying to apply Swing logic to FX is not a good idea --
it would be better to look at what FX is doing an trying to
understand why it does things the way it does. I'm not sure what
logic you are referring to that is being broken, but I never had
problems with FX's event system, and am using it for several
non-FX events to coordinate my UI. I'm also unsure what you mean
that the events are not designed to bubble up; they mostly
certainly are. Events are only copied when their target changes,
and this normally doesn't happen (ie. the target is the focus
owner or the control under the mouse).</p>
<p>For example, if I create an Event, and I set its target to X,
then firing this event by calling X.fireEvent will not make any
copies as it filters/bubbles. If I however use that same event to
call `otherNode.fireEvent` then a copy will be made to correct the
target (the target becomes `otherNode`). It will also make a copy
if you leave the target `null`. <br>
</p>
<p>What you see for example in SpinnerSkin is that KeyEvents arrive
and that they're targetted at the Spinner (which is correct as it
has the focus). However, as there is a "fake focus" control (the
TextField) it won't get those events without some extra effort (if
it had real focus, it would work out of the box, but we don't want
the TextField to have focus as Controls are supposed to be black
boxes). So, these events need to be forwarded to the control that
has focus within the Skin. In order to do so, the target must be
changed, which is why you need to make a copy. The other reason
you need to make a copy is that the original event is not done yet
(it still needs to bubble up if it is not consumed). The original
event should only be consumed if the forwarded event is consumed,
otherwise it should be left untouched. The mistake made in
SpinnerSkin is to have this retargetted event propagate all the
way back down from Window/Scene, passing the Spinner Control again
(where its handlers will see it), to this new target, instead of
only forwarding it within the Skin's internal hierarchy
(understandably this is something that is non-obvious on how to do
this, as the standard `fireEvent` code is no help to you there).<br>
</p>
<p>See this solution here: <a class="moz-txt-link-freetext" href="https://github.com/openjdk/jfx/pull/1629">https://github.com/openjdk/jfx/pull/1629</a></p>
<p>It is simpler, removes dependencies between Skin and Behavior
(yay) and probably faster as well (no 2nd event bubbling all the
way down the hierarchy).<br>
</p>
<blockquote type="cite"
cite="mid:BL3PR10MB61851CB9CB5BD024A9534B55E55C2@BL3PR10MB6185.namprd10.prod.outlook.com">
<div class="WordSection1">
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">This
brings me to another problem I am struggling with: why
expose the event dispatchers?? This should have never been
done, in my opinion. Swing's FwDispatcher is an
implementation detail. But instead, we seem to be dealing
with unnecessarily increased complexity, including
misbehaving EventDispatcher implementations.</span></p>
</div>
</blockquote>
<p>I haven't quite figured out why that is public, and why you can
set it, but I suppose it allows you to influence event handling
even more radically then just installing filters/handlers, perhaps
useful for custom controls or some skins (many odd choices seem to
have been influenced by the needs Skins have). Finding locations
where this method this property is used may give some insight why
this was done.<br>
</p>
<p>One use case seems to be that it allows you to fire an event
directly at a single Node (or a partial hierarchy) without going
through a full dispatch/bubbling phase from the Window level. In
my case, I used it now to solve the problem with SpinnerSkin, so
it does seem to have some merit in complex scenario's.</p>
<p>--John<br>
</p>
<span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span>
<blockquote type="cite"
cite="mid:BL3PR10MB61851CB9CB5BD024A9534B55E55C2@BL3PR10MB6185.namprd10.prod.outlook.com">
<div class="WordSection1">
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">May
be it's just me, I don't see the beauty of this thing.</span></p>
</div>
</blockquote>
<br>
<blockquote type="cite"
cite="mid:BL3PR10MB61851CB9CB5BD024A9534B55E55C2@BL3PR10MB6185.namprd10.prod.outlook.com">
<div class="WordSection1">
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">-andy<o:p></o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""><o:p> </o:p></span></p>
<div id="mail-editor-reference-message-container">
<div>
<div>
<div
style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span
style="font-size:12.0pt;color:black">From:
</span></b><span
style="font-size:12.0pt;color:black">openjfx-dev
<a class="moz-txt-link-rfc2396E" href="mailto:openjfx-dev-retn@openjdk.org"><openjfx-dev-retn@openjdk.org></a> on behalf of
John Hendrikx <a class="moz-txt-link-rfc2396E" href="mailto:john.hendrikx@gmail.com"><john.hendrikx@gmail.com></a><br>
<b>Date: </b>Thursday, November 7, 2024 at 05:35<br>
<b>To: </b><a class="moz-txt-link-abbreviated" href="mailto:openjfx-dev@openjdk.org">openjfx-dev@openjdk.org</a>
<a class="moz-txt-link-rfc2396E" href="mailto:openjfx-dev@openjdk.org"><openjfx-dev@openjdk.org></a><br>
<b>Subject: </b>Re: Prioritized event handlers<o:p></o:p></span></p>
</div>
<p>Hi Andy,<o:p></o:p></p>
<p>Simply replacing a Skin can change the order of event
handlers. In other words, if you registered your
handler **after** the control was shown, then replace
its Skin (which can be done by CSS state changes), then
your handler is now called before the Skin/Behavior
handlers. Also removing your own handlers, then
installing them back (in the same order) will not lead
to the same result either. The culprit is simply that
two different parties (FX and the User) are sharing the
same system -- this is mostly fine for properties, but
Events are stateful, and so the processing order
matters.<o:p></o:p></p>
<p>I've given this quite a lot of thought recently, and I
also looked at the Spinner problem, and I think I do
have an all encompassing solution for both the ordering
problem and the Spinner problem.<o:p></o:p></p>
<p>Observations:<o:p></o:p></p>
<p>Controls have some trouble keeping their Skin internals
hidden from the outside world. For the user, the
Spinner is a black box; it is the thing that gets the
focus and accepts input, even though the actual
implementation may be delegating this to an internal
component (in the case of the default SpinnerSkin, a
FakeFocusTextField -- a detail that you can see
"leaking" out when registering a listener on
Scene#focusOwner).<o:p></o:p></p>
<p>Secondly, because Skins are re-using normal controls,
which only work when focused, they sometimes have to
"fake" the focus (this is a problem I won't be solving
in this post). IMHO however, this should be handled
differently altogether, perhaps with some kind of focus
delegation mechanism as part of Behaviors -- the user
observable focus should IMHO always be the Control, as
it is a black box. The current fake focus
implementation tries to keep this hidden (by not firing
change listeners) which is incredibly dubious, and also
fails (you can still see internals being focused with
Scene#focusOwner). <o:p></o:p></p>
<p>Analysis:<o:p></o:p></p>
<p>Inside SpinnerSkin, there is a lot of jumping through
hoops in the form of filtering events, copying them, and
being aware of what the Behavior needs (SpinnerSkin for
example knows that the Behavior has need of the
UP/DOWN/LEFT/RIGHT keys, it shouldn't...) -- all this
jumping through hoops is to make the internal TextField
component receive events, but at the same time, not
confusing the Behavior with duplicated events (as the
Behavior handlers are in between the event root (the
window) and the target (the internal textfield) events
flow past the Behavior's handlers).<o:p></o:p></p>
<p>Solution:<o:p></o:p></p>
<p>The Behavior event handlers are at the Control level --
as far as the user is concerned, this is the target and
also final destination (there is nothing deeper). In
effect you could say that the Behavior could register
either a Filter or a Handler, and the result would be
the same -- filtering down from Scene -> Node ->
Spinner and then bubbling up from Spinner -> Node
-> Scene, it shouldn't matter if the Behavior
registers a filter or a handler as they're at the same
position (of course there are some small differences,
but I think we actually WANT filters in behaviors, more
on this later).<o:p></o:p></p>
<p>One of the problems is that the event handlers in the
behavior trigger **after** the internal TextField -- but
we have the perfect mechanism to swap this around; if
the Behavior registered a filter (which as I said
earlier is sort of at the same position conceptually)
then it would trigger **before** the internal
TextField. Spinner is then free to process and consume
the arrow keys it needs, and the Skin would not need to
be aware of this (ie. if a different Behavior decides to
consume the +/- keys, then it would work with the
current Skin). The Skin then just forwards any events
that arrive at the Control level still (everything minus
potential arrow keys consumed by the Behavior) to the
internal text field.<o:p></o:p></p>
<p>The second part of this solution relates to how to get
Events to go down to the internal TextField. Events are
fired at the current focus owner, which is the Control.
The TextField only has fake focus. So we need to take
events that arrive at the Control, copy them and send
them to the internal TextField. However, there is a
small problem -- the standard event mechanism will make
such copies filter down from Window level all the way to
the TextField level (passing the Spinner control). But
this isn't want we want at all. Spinner is a black box;
when we want to fire an event into the internals, we
don't want this event to be exposed either (aside from
the problems that causes, with having to be careful not
to copy the event again as it will pass your forwarding
handler again). In effect, we want a different event
dispatch hierarchy that is limited to the Control only.
So, why not build a dispatch chain that exclusively
targets the internals? This chain in this case only
consists of one EventDispatcher, the one from the
internal TextField. The event copy is fired at it and
the control works as expected. It automatically also
solves the "ENTER" problem described in
<a href="https://bugs.openjdk.org/browse/JDK-8337246"
moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.org/browse/JDK-8337246</a>
without further hacks.<o:p></o:p></p>
<p>Takeaways:<o:p></o:p></p>
<p>I now think Behaviors should always be doing their
event handling in filters, for the simple reason that
they should get to act first before the Skin gets to act
on an event; events arriving at the Control level which
is the final destination can be handled by either a
handler or a filter and there would be no discernible
difference. Not only does this solve the problem with
the internal text field and the Skin needing to be aware
of what keys Behavior will be handling, but it also then
makes behaviors always act before user event handlers,
creating consistency here (ie. replacing a Skin will not
affect event handler order). This frees up handlers for
user purposes. The ordering problem moves to the
filters, so we may still want to do a solution here (see
below).<o:p></o:p></p>
<p>Event handler order when Skins get re-added:<o:p></o:p></p>
<p>Handlers registered by users can change order with
regards to handlers registered by Behaviors when the
Skin change or depending on when the user registered
their handlers. This is problematic for events as they
are stateful and can be consumed, and so an order change
can lead to a functionality change. This is not user
friendly, as the user expects that the event handler and
filter infrastructure is for their use alone. I can see
a few things we could explore further:<o:p></o:p></p>
<p>- See if it is possible to add some internal API that
allows Behaviors to register filters or handlers
first/last -- note that "last" here means **always**
last, even when user handlers are added later (first
would be an easier option).<br>
- See if it is possible to change how
`buildEventDispatchChain` works for Controls to include
another dispatcher level exclusively for Behaviors.
Where a standard chain would look like
Window->Scene->Nodes->Control, it would become
Window->Scene->Nodes->Control->Behavior(private) -- as this
is a completely separate level, handlers from users and
behaviors cannot become intertwined and so there will
always be a predictable ordering<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:12.0pt"><o:p> </o:p></span></p>
<div>
<p class="MsoNormal"><span style="font-size:12.0pt">On
07/11/2024 01:03, Andy Goryachev wrote:<o:p></o:p></span></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">Dear
Michael:</span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""> </span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">What
happened to this proposal? I would like to
restart the discussion, if possible.</span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""> </span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">More
specifically, I would like to discuss the
following topics:</span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""> </span><o:p></o:p></p>
<ol style="margin-top:0in" type="1" start="1">
<li class="li1" style="mso-list:l0 level1 lfo1"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">the
reason the discussion was started was due to
"priority inversion" problem in Controls/Skins,
ex.:
<a
href="https://bugs.openjdk.org/browse/JDK-8231245"
moz-do-not-send="true">JDK-8231245</a> 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?</span><o:p></o:p></li>
<li class="li1" style="mso-list:l0 level1 lfo1"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">dispatching
of events that have been consumed (as mentioned
in the earlier discussion)</span><o:p></o:p></li>
<li class="li1" style="mso-list:l0 level1 lfo1"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">problem
of creating unnecessary clones of events via
Event.copyFor(), leading to ex.:
<a
href="https://bugs.openjdk.org/browse/JDK-8337246"
moz-do-not-send="true">JDK-8337246</a>
SpinnerSkin does not consume ENTER KeyEvent when
editor ActionEvent is consumed</span><o:p></o:p></li>
<li class="li1" style="mso-list:l0 level1 lfo1"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">why
do we need Event.copyFor() in the first place?<span
class="apple-converted-space">
</span>why does Event contain the target??</span><o:p></o:p></li>
</ol>
</div>
</blockquote>
<p>1. You would have the same problem with other events
that Behaviors listen to (ie. mouse events), or events
they handle without going through the input map (TYPED,
RELEASED), or events they handle with a parent event
type (all KeyEvents, not a specific one).<o:p></o:p></p>
<p>2. This is IMHO a bug, it only happens for the same
event type at one level, it should simply be fixed<o:p></o:p></p>
<p>3. See above, but in short, I think the problem is not
the event clones, but the fact that these clones are
traversing the entire Window->Scene->Node
hierarchy. Events with a restricted hierarchy solve
this.<o:p></o:p></p>
<p>4. The target allows you to register the same event
handler instance multiple times on multiple different
controls without having to create a new lambda each time
that knows which control (target) it is for. It has a
similar function to the Observable parameter in
InvalidationListeners and ChangeListeners. In effect,
having the Target will also make it possible in the
future to share a single input map with all controls of
the same type (currently there is a ton of memory being
wasted here duplicating input maps, especially for
controls with large input maps like TextFields).<o:p></o:p></p>
<p>--John<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""> </span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">Too
many topics, yes, we might want to split the
discussion into separate threads.</span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""> </span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16"">-andy</span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""> </span><o:p></o:p></p>
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed SS16""> </span><o:p></o:p></p>
<div id="mail-editor-reference-message-container">
<div>
<div>
<div
style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"
style="margin-bottom:12.0pt"><b><span
style="font-size:12.0pt;color:black">From:
</span></b><span
style="font-size:12.0pt;color:black">openjfx-dev
<a
href="mailto:openjfx-dev-retn@openjdk.org" moz-do-not-send="true">
<openjfx-dev-retn@openjdk.org></a>
on behalf of Michael Strauß <a
href="mailto:michaelstrau2@gmail.com"
moz-do-not-send="true">
<michaelstrau2@gmail.com></a><br>
<b>Date: </b>Friday, October 27, 2023 at
19:41<br>
<b>To: </b>openjfx-dev <a
href="mailto:openjfx-dev@openjdk.org"
moz-do-not-send="true"><openjfx-dev@openjdk.org></a><br>
<b>Subject: </b>Re: Prioritized event
handlers</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span
style="font-size:11.0pt">Here is the
proposal:<br>
<a
href="https://gist.github.com/mstr2/4bde9c97dcf608a0501030ade1ae7dc1"
moz-do-not-send="true"
class="moz-txt-link-freetext">https://gist.github.com/mstr2/4bde9c97dcf608a0501030ade1ae7dc1</a><br>
<br>
Comments are welcome.<br>
<br>
<br>
On Fri, Oct 27, 2023 at 8:21</span><span
style="font-size:11.0pt;font-family:"Arial",sans-serif"> </span><span
style="font-size:11.0pt">PM Andy Goryachev<br>
<a href="mailto:andy.goryachev@oracle.com"
moz-do-not-send="true"><andy.goryachev@oracle.com></a>
wrote:<br>
><br>
> Would it be possible to create a
proposal in the JEP format outlining the
proposed public API?<br>
><br>
><br>
><br>
> Thank you<br>
><br>
> -andy</span><o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
</body>
</html>