30.11.2012 14:42, Petr Pchelko пишет:
Thank you for your feedback, here is the new version of the fix:
http://cr.openjdk.java.net/~art/pchelko/7154778/
And the patch for the SWT is available at:
http://cr.openjdk.java.net/~art/pchelko/7154778/swt_patch.txt
Sergey wrote:
4 Probably we shall replace performOnMainThread on
AWT_PERFORM_ON_MAIN_THREAD_WAITING everywhere?
Yes, but it would require too many change unrelated to the bug itself,
so may be we should do it later?
ok
5 Where CPlatformView.setAutoResizable is used?
In the initializer of the CViewPlatformEmbeddedFrame.
ok
6 CViewPlatformEmbeddedFrame could cache the native bounds, like CPWindows
does? in this way CPlatformView.nativeGetLocationOnScreen is unnecessary.
As I see this is impossible, because CViewEmbeddedFrame knows it's
bounds only relative to some hosting superview, not relative to the screen. And
as we have now access to the superview in the Java code we need this native
method.
7 Why we need override resizeWithOldSuperviewSize() in AWTView? How it work in
the usual way when we use AWTWindow windowDidResize/windowDidMove?
NSView does not provide the viewDidResize/viewDidMove notifications. It
has only notifications about the liveResize, but it is insufficient because we
need to notify Java not only about live resize but about any resize triggered
by the superview.
So we did not get notifications twice in the ususal mode? From
viewDidResize/viewDidMove and from resizeWithOldSuperviewSize?
Leonid wrote:
I don't quite understand some focus related changes you've made. What is the
reason for generating focus events from - (BOOL) becomeFirstResponder and -
(BOOL) resignFirstResponder? There are - (void) windowDidBecomeKey and - (void)
windowDidResignKey methods in AWTWindow we've been using to generate focus
events from, so I'm afraid that with your change will be getting duplicate
focus events.
At first I wanted to make the EmbeddedFrame manage it's focus itself in
order to make as low amount of changes in SWT as possible. But, I think you are
right and this might be dangerous, so I have changed the focus management
system.
Now the hosting application must handle focus management for us by
synthesizeWindowActivation/synthesizeWindowDeactivation methods.
Anthony wrote:
4. CWrapper.m
You could use the new ThreadingUtilities machinery here as well.
We do not need to do it to fix this bug, so may be it would be better
to refactor CWrapper later when we will change everywhere?
All the suggestion I did not answer explicitly are used for the changes in the
code.
Changes in CPlatformEmbeddedFrame.java is not complete?
194 @Override
195 public boolean isUnderMouse() {
196 //Impossible to figure out if CALayer is under mouse
197 throw new RuntimeException("Not implemented");
198 }
CPlatformWindow.getGraphicsDevice() could be replaced by
contenView.getGraphicsDevice()?
All other parts looks good.
Best, Petr.
On Nov 28, 2012, at 7:09 PM, Anthony Petrov wrote:
Hi Petr,
1. src/macosx/native/sun/osxapp/ThreadUtilities.h
I suggest to introduce a static (+) method in the ThreadUtilities class rather
than a new global macro.
Also, while I agree with Sergey that eventually we should use it everywhere in
AWT, I suggest to replace other occurrences of this code later, with a separate
refactoring fix. Use the new method only for the code that you change/introduce
with this fix.
BTW, in case wait==NO, we should force going through the JNFRunLoop even if
we're running on the main thread.
2. LWWindowPeer.java, CPlatformWindow.java, and peerType
It looks like the CPlatformWindow doesn't even need to know the peerType. I
think we can safely drop the peerType argument from CPlatformWindow
constructor, and only store it in the LWWindowPeer itself. Should we need the
value in the future, we can always ask the LWWindowPeer since CPW keeps a
reference to the peer object.
3. src/macosx/classes/sun/lwawt/macosx/CMouseInfoPeer.java
I think we could avoid using the instancof and instead introduce a method in
the PlatformWindow interface (like isUnderMouse()). The CPW and CVPEF would
simply provide different implementations for this method.
4. CWrapper.m
You could use the new ThreadingUtilities machinery here as well.
--
best regards,
Anthony
On 11/27/2012 1:55 PM, Petr Pchelko wrote:
Hello, AWT team.
please, review the following fix for 7154778:
http://cr.openjdk.java.net/~art/pchelko/7154778/
The bug description and evaluation is available here:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7154778
To work with the new EmbeddedFrame implementation, existing code (e.g. SWT)
needs to be modified to use sun.lwawt.macosx.CViewEmbeddedFrame class instead
of Apple's apple.awt.CEmbeddedFrame. Here is the corresponding patch for SWT:
http://cr.openjdk.java.net/~art/pchelko/7154778/swt_patch.txt
Best, Petr Pchelko
--
Best regards, Sergey.
