[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-openjfx-dev
Subject: Re: [External] : KeyCodeCombination
From: John Hendrikx <john.hendrikx () gmail ! com>
Date: 2023-11-29 23:26:05
Message-ID: 84872155-c703-3e18-f553-b7bd7674cd1d () gmail ! com
[Download RAW message or body]
Hi Andy and List,
I did some checking on the internal KeyBinding, and I think it has a
broken hashCode/equals implementation, which if used as part of a map (I
think they aren't currently) would mean it won't find always find the
correct (or any) match with a simple Map#get.
This is because the hashCode for modifiers returns unique values for
TRUE, FALSE and ANY, but equals accepts that ANY can match TRUE or
FALSE. So, if you look up a binding, the hash code generated may not be
the same for a binding for the same KeyCode depending on the modifier
state, even though equals would consider it matching -- the map would
then look in the wrong bucket, and may miss the match.
Therefore, I simpy only put KeyCode in a Map as key, then do further
specific checking for modifiers.
I haven't checked your implementation, but it might be something you
need to be aware of.
Anyway, as I lookup by KeyCode, and then look through a list for which I
call KeyCodeCombination#match, I have no problems reusing
KeyCodeCombination.
--John
On 30/11/2023 00:09, Andy Goryachev wrote:
>
> The rationale was this: I wanted KeyBinding to be a key in a hash
> map. KeyCodeCombination cannot be used as keys since they might
> correspond to multiple key combinations, as far as I can tell. Also,
> it would necessitate the use of a linear search in some cases.
>
> I did not like the internal KeyBinding as it wasn’t convenient enough
> for my taste. I wanted something that can be easily created. There
> were some early ideas about coding platform specificity into the KB,
> but either you or Michael mentioned that this isn’t really necessary.
> KeyBinding also deals with shortcut, meta, and option modifiers to get
> what I want.
>
> Can I copy my response to the list?
>
Sorry, I copied it now.
--John
> Cheers,
>
> -andy
>
> *From: *John Hendrikx <john.hendrikx@gmail.com>
> *Date: *Wednesday, November 29, 2023 at 14:57
> *To: *Andy Goryachev <andy.goryachev@oracle.com>
> *Subject: *[External] : KeyCodeCombination
>
> Hi Andy,
>
> I think you mentioned there was a reason for introducing a new
> KeyBinding type class (or at least copying part of the internal one).
>
> I've used KeyCodeCombination now, and this seems to do everything I
> really want (including having a `match` method that can match it against
> a KeyEvent).
>
> What were problems you encountered using this class, or what was the
> reasoning for introducing a new class for key bindings?
>
> --John
>
[Attachment #3 (text/html)]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi Andy and List,<br>
</p>
<p>I did some checking on the internal KeyBinding, and I think it
has a broken hashCode/equals implementation, which if used as part
of a map (I think they aren't currently) would mean it won't find
always find the correct (or any) match with a simple Map#get.</p>
<p>This is because the hashCode for modifiers returns unique values
for TRUE, FALSE and ANY, but equals accepts that ANY can match
TRUE or FALSE. So, if you look up a binding, the hash code
generated may not be the same for a binding for the same KeyCode
depending on the modifier state, even though equals would consider
it matching -- the map would then look in the wrong bucket, and
may miss the match.<br>
</p>
<p>Therefore, I simpy only put KeyCode in a Map as key, then do
further specific checking for modifiers.</p>
<p>I haven't checked your implementation, but it might be something
you need to be aware of.</p>
<p>Anyway, as I lookup by KeyCode, and then look through a list for
which I call KeyCodeCombination#match, I have no problems reusing
KeyCodeCombination.</p>
<p>--John<br>
</p>
<div class="moz-cite-prefix">On 30/11/2023 00:09, Andy Goryachev
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:DM5PR1001MB21727AB5652F8564A2BE6BE8E583A@DM5PR1001MB2172.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:Calibri;
panose-1:2 15 5 2 2 2 4 3 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;}p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:10.0pt;
font-family:"Calibri",sans-serif;}span.EmailStyle19
{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;}</style>
<div class="WordSection1">
<p class="MsoNormal"><span
style="font-size:11.0pt;font-family:"Iosevka Fixed
SS16"">The rationale was this: I wanted KeyBinding to
be a key in a hash map. KeyCodeCombination cannot be used
as keys since they might correspond to multiple key
combinations, as far as I can tell. Also, it would
necessitate the use of a linear search in some \
cases.<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"">I did not like the internal KeyBinding as it
wasn’t convenient enough for my taste. I wanted something
that can be easily created. There were some early ideas
about coding platform specificity into the KB, but either
you or Michael mentioned that this isn’t really necessary.
KeyBinding also deals with shortcut, meta, and option
modifiers to get what I want.
<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"">Can I copy my response to the list?</span></p>
</div>
</blockquote>
<p>Sorry, I copied it now.</p>
<p>--John<br>
</p>
<blockquote type="cite"
cite="mid:DM5PR1001MB21727AB5652F8564A2BE6BE8E583A@DM5PR1001MB2172.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"">Cheers,<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 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">John
Hendrikx <a class="moz-txt-link-rfc2396E" \
href="mailto:john.hendrikx@gmail.com"><john.hendrikx@gmail.com></a><br> \
<b>Date: </b>Wednesday, November 29, 2023 at 14:57<br> <b>To: </b>Andy Goryachev
<a class="moz-txt-link-rfc2396E" \
href="mailto:andy.goryachev@oracle.com"><andy.goryachev@oracle.com></a><br>
<b>Subject: </b>[External] : \
KeyCodeCombination<o:p></o:p></span></p> </div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span
style="font-size:11.0pt">Hi Andy,<br>
<br>
I think you mentioned there was a reason for
introducing a new <br>
KeyBinding type class (or at least copying part of the
internal one).<br>
<br>
I've used KeyCodeCombination now, and this seems to do
everything I <br>
really want (including having a `match` method that
can match it against <br>
a KeyEvent).<br>
<br>
What were problems you encountered using this class,
or what was the <br>
reasoning for introducing a new class for key
bindings?<br>
<br>
--John<o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</blockquote>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic