<Swing Dev> [11] RFR JDK-8189687:Swing: Invalid position of candidate pop-up of InputMethod in Hi-DPI on Windows
Anton Tarasov
anton.tarasov at jetbrains.com
Tue Apr 24 18:07:18 UTC 2018
Hi Sergey,
On 4/24/2018 4:48 AM, Sergey Bylokhov wrote:
> For the record, the fix which is under discussion:
> http://cr.openjdk.java.net/~ant/JDK-8202084/webrev.0
>
> If I understand it correctly then GetHWnd() was replaced by
> ImmGetHWnd() to support the focus proxy. But note that
> ScaleUpX/ScaleUpY will use device where GetHWnd() is located, not a
> device where ImmGetHWnd() is located. Is it possible that these
> windows will be located on a different screens -> wrong scale factor
> will be used?
The thing is this. Say, we have a simple window and its owner. Then,
what we do in java is this:
Point pt = textInWindow.getLocationOnScreen();
showCandidateWindow(pt);
'pt' is the coordinate in the global multi-monitor user space (relative
to the origin of the main monitor). We construct it on the native side
(AwtComponent::_GetLocationOnScreen):
RECT rect;
::GetWindowRect(peer->GetHWnd(), &rect); // the owned window location in
the global multi-monitor device space
x = peer->ScaleDownX(rect.left)
y = peer->ScaleDownY(rect.top)
where 'peer' is the owned window. Next, we do the following:
POINT p = <owner origin> // location of the owner, in the global
multi-monitor device space
int sx = ScaleUpX(x) - p.x;
int sy = ScaleUpY(y) - p.y;
Here, ScaleUp(x/y) is the inverse operation of what we did above. As the
result we get the owned window location in the global multi-monitor
device space.
(ScaleUp(x,y) - p) is the same, but relative to the owner.
I've checked that it indeed works, placing the owner and the window on
different monitors with different scales.
However, there's some serious problem in this construct which you may
have noticed. It's here:
x = peer->ScaleDownX(rect.left);
'rect.left' may span a number of displays, each with different scale.
Thus, to get the real ScaleDown, rect.left should be split into parts,
corresponding to the displays it crosses.
Then a display scale should be applied to each part, and then the sum
would be the correct result in the user space. As for now, the result is
incorrect.
Why it works is just because textInWindow.getLocationOnScreen() is
immediately passed back to native, where it is up-scaled the same
(incorrect) way.
However, in other cases this problem may cause issues. And I have no
simple solution for it...
>
> BTW there is a typo:
> 3888 int sx = ScaleUpX(x) - p.x;
> 3889 int sy = ScaleUpX(y) - p.y;
Thanks for the catch, fixed it.
http://cr.openjdk.java.net/~ant/JDK-8202084/webrev.1
Regards,
Anton.
>
>
> On 22/04/2018 22:47, Prasanta Sadhukhan wrote:
>> Hi Anton,
>>
>> This fix looks ok to me.
>> Probably, you should consider naming the variable at the top as some
>> old compiler may give warning and make the build fail.
>>
>> Regards
>> Prasanta
>> On 4/21/2018 12:53 AM, Anton Tarasov wrote:
>>> Hi Sergey, Prasanta,
>>>
>>> I'm sorry to interpose, but I've just submitted a fix for this
>>> issue:
>>> http://mail.openjdk.java.net/pipermail/awt-dev/2018-April/013891.html
>>> As it seems to more relate to AWT not Swing, I submitted it to
>>> awt-dev (I didn't notice you discussion here, but thanks to Kevin
>>> who pointed me to)
>>>
>>> So, let me please comment on the suggested fix. As far as I
>>> understand the scaling design in AWT, the boundary b/w the user and
>>> device spaces lies on the native side of the AWT peer implementation
>>> (talking about the Windows platform). Roughly speaking, every
>>> coordinate passed to the native peer belongs to the user space and
>>> is converted to the device space (the device with which the peer is
>>> currently associated) immediately after. And vice versa, every
>>> coordinate passed from the native peer up to java is converted from
>>> the device space to the user space just before passing. So, as
>>> Sergey said below, Swing/AWT operates coordinates strictly in user
>>> space (except for the cases when it comes to raster images may be).
>>> In scope of this, the suggested approach doesn't seem right...
>>>
>>> So, could you please share your thoughts on it and consider the
>>> alternative fix referenced above?
>>>
>>> Thanks,
>>> Anton.
>>>
>>>
>>> On 3/31/2018 4:21 AM, Sergey Bylokhov wrote:
>>>> I guess we need to check it again.
>>>> We have two coordinate spaces in awt/swing:
>>>> - User-space which is used by all(most) of our API in awt/swing.
>>>> It means that all methods like
>>>> setSize/setBounds/getLocationOnScreen etc use this coordinate space.
>>>> - Device-space which is used by the native system. It is used when
>>>> we show something on the screen, when we get some notification from
>>>> the OS etc.
>>>>
>>>> So for the example if we try to set the bounds of the window then
>>>> we convert the size from the user-space to device-space. But when
>>>> we get some notification from the native system then we will
>>>> convert device-space to the user-space.
>>>>
>>>> In this bug we use wrong location of candidate window. It means
>>>> that we lost some conversion.
>>>>
>>>> So if we start from the first iteration of the fix:
>>>> http://cr.openjdk.java.net/~psadhukhan/8189687/webrev.00/src/java.desktop/windows/classes/sun/awt/windows/WInputMethod.java.sdiff.html
>>>>
>>>>
>>>> We calculate the x,y in two places using getTextLocation() and
>>>> getLocationOnScreen(). Both functions should return coordinates in
>>>> the user-space because both are public API in AWT. If some of these
>>>> methods use wrong device space it should be updated(in the place
>>>> where we get such coordinates from the native).
>>>>
>>>> We show the candidate window using openCandidateWindow() method,
>>>> this method uses x,y coordinates. In which coordinate system x,y
>>>> should be?
>>>> If it uses device space then we should convert both - results of
>>>> getTextLocation() and getLocationOnScreen().
>>>>
>>>>
>>>> On 28/03/2018 22:02, Prasanta Sadhukhan wrote:
>>>>> Hi Sergey,
>>>>>
>>>>> The code flows to JTextComponent.getTextLocation() which does not
>>>>> return a scaled rectangle as it uses
>>>>> awt_Component.cpp#_GetLocationOnScreen where it is scaled down. Do
>>>>> you want me to move the code to JTextComponent.getTextLocation()
>>>>> instead?
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>> On 3/28/2018 9:06 PM, Sergey Bylokhov wrote:
>>>>>> Hi, Prasanta.
>>>>>> The same question about this code:
>>>>>> 281 Rectangle r = getReq().getTextLocation(offset);
>>>>>>
>>>>>> The getReq() returns InputMethodRequests which is implemented by
>>>>>> a number of classes, and I think one of them
>>>>>> "InputMethodRequestsHandler" returns scaled values from
>>>>>> "getTextLocation()" already.
>>>>>> Some of these classes may return some stubs which should not be
>>>>>> scaled, like in CompositionAreaHandler:
>>>>>> // passive client, no composed text, so fake a rectangle
>>>>>> return new Rectangle(0, 0, 0, 10);
>>>>>>
>>>>>
>>>>>>
>>>>>> On 27/03/2018 22:39, Prasanta Sadhukhan wrote:
>>>>>>> Any more comments?
>>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>>> On 3/26/2018 1:21 PM, Prasanta Sadhukhan wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> On 3/23/2018 3:44 AM, Sergey Bylokhov wrote:
>>>>>>>>> Hi, Prasanta.
>>>>>>>>> Did you check why the "InputMethodContext.getTextLocation()"
>>>>>>>>> returns non-scaled rectangle? Maybe we should do this inside
>>>>>>>>> InputMethodContext()?
>>>>>>>>>
>>>>>>>> Yes, this code
>>>>>>>> http://hg.openjdk.java.net/jdk/client/annotate/f46bfa7a2956/src/java.desktop/windows/native/libawt/windows/awt_Component.cpp#l5673
>>>>>>>>
>>>>>>>> scales down x,y as part of JDK-8073320 fix.
>>>>>>>> I have moved the fix to InputMethodContext as suggested
>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8189687/webrev.02/
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Prasanta
>>>>>>>>> On 20/03/2018 22:17, Prasanta Sadhukhan wrote:
>>>>>>>>>> Hi Krishna,
>>>>>>>>>>
>>>>>>>>>> Yes, I did not provide any since the testcase needs to be
>>>>>>>>>> manual and would have to contain lots of instructions of how
>>>>>>>>>> to install Japanese language and changing the input mode to
>>>>>>>>>> hiragana
>>>>>>>>>> and also similar fix of input method earlier did not have a
>>>>>>>>>> testcase for similar reason.
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Prasanta
>>>>>>>>>> On 3/20/2018 8:42 PM, Krishna Addepalli wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Prasanta,
>>>>>>>>>>>
>>>>>>>>>>> Now the changes look fine. However, I noticed that there is
>>>>>>>>>>> no testcase associated with this fix. Is that intentional?
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Krishna
>>>>>>>>>>>
>>>>>>>>>>> *From:*Prasanta Sadhukhan
>>>>>>>>>>> *Sent:* Tuesday, March 20, 2018 5:04 PM
>>>>>>>>>>> *To:* Krishna Addepalli <krishna.addepalli at oracle.com>;
>>>>>>>>>>> swing-dev at openjdk.java.net
>>>>>>>>>>> *Subject:* Re: <Swing Dev> [11] RFR JDK-8189687:Swing:
>>>>>>>>>>> Invalid position of candidate pop-up of InputMethod in
>>>>>>>>>>> Hi-DPI on Windows
>>>>>>>>>>>
>>>>>>>>>>> Thanks Krishna for your comment. Modified webrev catering to
>>>>>>>>>>> retrieval of scalefactor of the active monitor being shown
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8189687/webrev.01/
>>>>>>>>>>> <http://cr.openjdk.java.net/%7Epsadhukhan/8189687/webrev.01/>
>>>>>>>>>>>
>>>>>>>>>>> Regards
>>>>>>>>>>> Prasanta
>>>>>>>>>>>
>>>>>>>>>>> On 3/20/2018 2:40 PM, Krishna Addepalli wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Prasanta,
>>>>>>>>>>>
>>>>>>>>>>> I have couple questions regarding your fix:
>>>>>>>>>>>
>>>>>>>>>>> 1.The AffineTransform object should be retrieved from
>>>>>>>>>>> the Screen
>>>>>>>>>>> on which the window is showing, whereas in your fix, you
>>>>>>>>>>> are
>>>>>>>>>>> directly getting it from the default screen. In multi
>>>>>>>>>>> screen
>>>>>>>>>>> environment, it may not be aligned correctly.
>>>>>>>>>>>
>>>>>>>>>>> 2.Is there any reason to retrieve the object in the top.
>>>>>>>>>>> I mean,
>>>>>>>>>>> the AffineTransform object can be declared inside the “if
>>>>>>>>>>> (haveActiveClient())” block, at the point of use.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Krishna
>>>>>>>>>>>
>>>>>>>>>>> *From:*Prasanta Sadhukhan
>>>>>>>>>>> *Sent:* Tuesday, March 20, 2018 1:01 PM
>>>>>>>>>>> *To:* swing-dev at openjdk.java.net
>>>>>>>>>>> <mailto:swing-dev at openjdk.java.net>
>>>>>>>>>>> *Subject:* <Swing Dev> [11] RFR JDK-8189687:Swing: Invalid
>>>>>>>>>>> position of candidate pop-up of InputMethod in Hi-DPI on
>>>>>>>>>>> Windows
>>>>>>>>>>>
>>>>>>>>>>> Hi All,
>>>>>>>>>>>
>>>>>>>>>>> Please review a fix for an issue where it is seen that
>>>>>>>>>>> for Japanese IME on windows 10 with scaleFactor 1.5,
>>>>>>>>>>> when we enter
>>>>>>>>>>> text using IME popup, the popup is positioned on top of
>>>>>>>>>>> text,
>>>>>>>>>>> thereby obscuring it
>>>>>>>>>>> as seen in this screenshot.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Proposed fix is to apply the scaleFactor on the
>>>>>>>>>>> candidate's popup
>>>>>>>>>>> positional coordinates x,y to generate proper
>>>>>>>>>>> coordinates to show
>>>>>>>>>>> this popup
>>>>>>>>>>> webrev:
>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/8189687/webrev.00/
>>>>>>>>>>> <http://cr.openjdk.java.net/%7Epsadhukhan/8189687/webrev.00/>
>>>>>>>>>>>
>>>>>>>>>>> The screenshot after the fix is
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Regards
>>>>>>>>>>> Prasanta
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>
>
More information about the swing-dev
mailing list