Thanks for the clarification. I see your point and I sort of agree that
passing the call from *LWFP to LWF, and then to LWC would look not
better than the instanceof check in SunToolkit.
I'm fine with the current code then, as long as it works as expected.
--
best regards,
Anthony
On 2/12/2013 16:22, Anton V. Tarasov wrote:
Hi Anthony,
(sorry, I missed your reply)
On 2/11/13 5:46 PM, Anthony Petrov wrote:
Hi Anton,
If I read your explanation correctly, the current calls chain is:
popups -> SunToolkit.grab -> (JLightweightFrame)Window.grabFocus ->
WindowPeer.grabFocus
I.e. we want to end up in WindowPeer.grabFocus() anyway. Note that for
non-lightweight frames we want to call WindowPeer.grabFocus() as well
(even though currently it's named e.g. WWindowPeer.grab() instead of
grabFocus(), etc.)
I suggest to reduce this call chain to just:
popups -> SunToolkit.grab -> WindowPeer.grabFocus
which would work for both lightweight and heavyweight frames.
In other words, I suggest to rename the current
*WindowPeer.grab()/ungrab() methods in all toolkits to "@Override
grab/ungrabFocus()", and implement the SunToolkit.grab/ungrab() as
final methods just delegating to the peer's methods. For lightweight
frames, the *LightweightFramePeer classes must delegate the requests
to FX.
The main difference here is that you suggest to delegate to FX from
peers, while I tend to put delegation to the target (JLightweightFrame).
From my point of view, delegating from peers has the following
disadvantages:
- grab/ungrab delegation to FX is platform independent, so peers don't
seem the right place to implement it
- anyway, a *LightweightFramePeer only connects to its target, it knows
nothing about the LightweightContent delegate, so a peer would need to
delegate via a JLightweightFrame,
for which it would need to either call a suitable public method or
use "shared secrets" approach to workaround access control. Passing a
LightweightContent delegate to a peer
is also not good from my point of view.
Note that we can't remove the SunToolkit.grab/ungrab() interface
anyway since JFXPanel code relies on it already.
I think we can rewrite it in JFXPanel (it's implementation details)
right after we have a new AWT grab API.
Therefore, I think that we don't want to introduce the Window. (or
currently just LightweightFrame.)grab/ungrabFocus() API at this time.
I believe that a proper peer interface should be enough for now.
I supposed we could, at least, rewrite our own code to use
Window.grabFocus and declare SunToolkit.grab deprecated. At last, this
is an _internal_ public API which has a disclaimer.
Thanks,
Anton.
--
best regards,
Anthony
On 02/11/13 12:39, Anton V. Tarasov wrote:
Hi Anthony,
JLightweightFrame overrides focusGrab/focusUngrab and forwards the calls
to the client app. Ideally, when the grab API is public, it should be
called directly via Window.focusGrab/focusUngrab. Currently it is called
via SunToolkit.grab/ungrab (by swing popups). So, normal call chain
should look like:
popups -> (JLightweightFrame)Window.grabFocus -> WindowPeer.grabFocus
Temporarily it's as follows:
popups -> SunToolkit.grab -> (JLightweightFrame)Window.grabFocus ->
WindowPeer.grabFocus
If I called it directly it would be:
popups -> SunToolkit.grab -> WindowPeer.grabFocus ->
(JLightweightFrame)Window.grabFocus -> WindowPeer.grabFocus -> ...
So, this is quite the reverse direction.
Thanks,
Anton.
On 2/8/13 7:23 PM, Anthony Petrov wrote:
Hi Anton,
src/windows/classes/sun/awt/windows/WToolkit.java
987 public void grab(Window w) {
991 if (w instanceof LightweightFrame) {
992 ((LightweightFrame)w).grabFocus();
993 return;
994 } 995 if (w.getPeer() != null) {
996 ((WWindowPeer)w.getPeer()).grab();
997 }
998 }
You've already added grab/ungrabFocus() to the WindowPeer interface.
Why not use them here directly instead of these instanceof checks?
Applies to other toolkits as well.
The fix looks good otherwise.
--
best regards,
Anthony
On 2/8/2013 21:27, Anton V. Tarasov wrote:
Hi All,
Please, review the changes for the CR:
http://bugs.sun.com/view_bug.do?bug_id=8006406
webrev: http://cr.openjdk.java.net/~ant/8006406/webrev.6
It introduces sun.swing.JLightweightFrame class, aimed at lightweight
embedding of Swing components into java-based toolkits.
The primary target is JavaFX toolkit, however the class is not
limited to this usage and the API it provides is quite generic.
Below I'm giving a link to the jfx side implementation. This
implementation should not be reviewed in this thread (it is in a
pre-review phase),
it should just clarify how the introduced API is supposed to be used.
Namely, SwingNode.SwingNodeContent which implements
sun.swing.LightweightContent and forwards requests from
sun.swing.JLightweightFrame to NGExternalNode which does the
rendering.
webrev: http://cr.openjdk.java.net/~ant/RT-27887/webrev.1
Some comments on the awt/swing part:
- Only Win and Mac implementation is currently available, X11 will
come lately.
- Win implementation uses a heavyweight window behind the lightweight
frame, while it is not actually needed for lightweight embedding.
This is due to the architecture of the Win AWT peers which are
strongly tight to the native code, and it's not a trivial task to
separate them.
On Mac the lightweight frame peer is truly lightweight, meaning that
it doesn't create an NSWindow object behind it. The Mac port LW
abstraction
allows to override and substitute CPlatform* classes with their
lightweight stubs.
- LightweightFrame, among others, introduces two new methods -
grabFocus() and ungrabFocus(boolean). Ideally, these methods should
go to
the super Window class where the grab API becomes public (which is a
long-term project...). Current host of the grab API is SunToolkit,
which
now forwards the calls to LightweightFrame. This is necessary to
intercommunicate with the client when grab/ungrab happens on both
sides.
- Unresolved issues exist, like modal dialogs, d&d etc. They are to
be addressed further.
Thanks,
Anton.
