<AWT Dev> [8] Review request for 8027628: JWindow jumps to (0, 0) after mouse clicked
Oleg Pekhovskiy
oleg.pekhovskiy at oracle.com
Fri Nov 15 07:35:26 PST 2013
Hi Anthony,
thank you for the review,
the second version of fix is here:
http://cr.openjdk.java.net/~bagiras/8027628.2/
Brief:
1. XBaseWindow changes were reverted.
2. Shared code moved from XDecoratedPeer to XWindowPeer
3. XWindowPeer.handleConfigureNotifyEvent() method improved.
please see my comments below...
Thanks,
Oleg
On 11/08/2013 06:18 PM, Anthony Petrov wrote:
> One more point is about the performance. It could be degraded
> significantly if we call toGlobal() all the time unconditionally.
>
> So I suggest to test this using SwingMark to ensure the effect is not
> very severe.
Does it make sense to perform such testing if I made functionality
changes in XWindowPeer.java only?
>
> --
> best regards,
> Anthony
>
> On 11/08/2013 06:11 PM, Anthony Petrov wrote:
>> Hi Oleg,
>>
>> The XBaseWindow class is not the best place for this code. Firstly, the
>> XDecoratedPeer.handleConfigureNotifyEvent() never calls its super.*
>> counterpart. Secondly, all top-level windows use XWindowPeer as an
>> instance for its peer, so this code actually belongs there (e.g. you can
>> update x, y after calling the super method.)
I analyzed the code in XDecoratedPeer.handleConfigureNotifyEvent()
method and found the part that could be shared with
XWindowPeer. I left XDecoratedPeer.handleConfigureNotifyEvent()
functioanlity intact (just moved the code).
I reverted my changes in XBaseWindow.handleConfigureNotifyEvent() method
and implemented everything in XWindowPeer.
>>
>> However, I'd still suggest to analyze why this isn't a problem for
>> decorated peers in the first place, and consider using the same
>> mechanism for undecorated peers as well (perhaps we could share some
>> code in this area and move some common parts to XWindowPeer, for
>> example).
It's not a problem for decorated windows because location is corrected
there too using the way similar to the proposed way
in the first version of my fix. So the code was shared.
>>
>> Also, the handleConfigureNotifyEvent code is *VERY* fragile. After we
>> settle on some more-or-less final version of the fix, you should run
>> most of Window/Frame/Dialog/Layout regression tests (both automatic and
>> manual, from open and closed repos) to make sure we don't regress. This
>> needs to be tested on OEL 6 (or which is the minimum supported version
>> for JDK 8?) and Solaris boxes as well, because the code in this event
>> handler covers many rare cases only reproducible with specific window
>> managers found on old systems.
I ran the following tests just on Ubuntu:
test/java/awt/Dialog
test/java/awt/Frame
test/java/awt/Window
closed/test/java/awt/Dialog
closed/ test/java/awt/Frame
closed/ test/java/awt/Layout
closed/ test/java/awt/Window
There were no regressions with my fix even one more previously failing
test passed.
I also ran SQE functional tests mentioned in JIRA issue description.
Windows stopped jumping.
Thanks,
Oleg
>>
>> --
>> best regards,
>> Anthony
>>
>> On 11/08/2013 12:00 PM, Oleg Pekhovskiy wrote:
>>> Hi all,
>>>
>>> please review the fix
>>> http://cr.openjdk.java.net/~bagiras/8027628.1/
>>> for
>>> https://bugs.openjdk.java.net/browse/JDK-8027628
>>>
>>> The issue affects all top-level windows. XConfigureEvent.x and
>>> XConfigureEvent.y fields
>>> from ConfigureNotify event could have wrong values, e.g. when the
>>> window
>>> is resized by not moved.
>>> That's why manual calculation was added.
>>>
>>> This is explained in ICCCM, in "4.1.5. Configuring the Window" section:
>>>
>>> "Advice to Implementors
>>>
>>> Clients cannot distinguish between the case where a top-level window is
>>> resized and moved from the case where the window is resized but not
>>> moved, since a real ConfigureNotify event will be received in both
>>> cases. Clients that are concerned with keeping track of the absolute
>>> position of a top-level window should keep a piece of state indicating
>>> whether they are certain of its position. Upon receipt of a real
>>> ConfigureNotify event on the top-level window, the client should note
>>> that the position is unknown. Upon receipt of a synthetic
>>> ConfigureNotify event, the client should note the position as known,
>>> using the position in this event. If the client receives a KeyPress ,
>>> KeyRelease , ButtonPress , ButtonRelease , MotionNotify , EnterNotify ,
>>> or LeaveNotify event on the window (or on any descendant), the client
>>> can deduce the top-level window's position from the difference between
>>> the (event-x, event-y) and (root-x, root-y) coordinates in these
>>> events.
>>> Only when the position is unknown does the client need to use the
>>> TranslateCoordinates request to find the position of a top-level
>>> window."
>>>
>>> Thanks,
>>> Oleg
More information about the awt-dev
mailing list