<AWT Dev> [8] Review request for CR 8006406: lightweight embedding in other Java UI toolkits

[Anton V. Tarasov]

Hi Sergey,

new webrev: http://cr.openjdk.java.net/~ant/8006406/webrev.7

On 2/12/13 4:57 PM, Sergey Bylokhov wrote:
> Hi, Anton.
> Notes about implementation:
> 1 Seems some code was changed for debug simplifications or changes 
> from previous implementations. It would be good to revert them back. 
> (Ex /LWComponentPeer.bounds).
Fixed all such occurrences (replaced with public "get" methods where 
available). Also, added "protected initializeBase(..)" method for field 
only initialization.

> 2 Probably it would be good to move grab/ungrab implementation from 
> LWToolkit/WToolkit to SunToolkit? It looks unclear why we need so many 
> grab/ungrab/grabFocus/ungrabFocus methods with the same implementation 
> in the different places.
We don't really need so much grabs and I will clean it when (and if) we 
publish the grab API. Please, see my replies to Anthony on this subject.

> 3 I suggest make all methods in LightweightFrame  final if possible.
Ok, I made toplevel related methods final. I'm not sure we should make 
final all the rest... (and by the way, the extender JLF class is final).

> 4 JLightweightFrame.rootPane could be final
Did.

> 5 JLightweightFrame.getGraphics() probably graphics should be 
> initialized by correct window fonts/background/foreground? Also when 
> you create backbuffer probably it should be filled by background 
> color? Note that if transparent images are supported you should be 
> aware about composite.

Ok, I did:

1) set transparent background for JLightweightFrame
2) set font/background/foreground for the Graphics.

Now I think I shouldn't specially care about the composite (am I right?).

> 6 JLightweightFrame.initInterior you shouldn't dispose graphics.

Yes, it seems this adheres to the javadoc:

      * Graphics objects which are provided as arguments to the
      * <code>paint</code> and <code>update</code> methods
      * of components are automatically released by the system when
      * those methods return. For efficiency, programmers should
      * call <code>dispose</code> when finished using
      * a <code>Graphics</code> object only if it was created
      * directly from a component or another <code>Graphics</code> object.

Fixed it.

> 7 JLightweightFrame.reshape width * height could be changed to width | 
> height ?
No =) It rather could be changed to w & h, but in order not to confuse a 
reader, I've changed it to w == 0 || h == 0.

>
> 8 JLightweightFrame.reshape you did not flush old backbuffer.

Did.

Also, I had to override LWWindowPeer.updateCursorImmediately() in LWLFP 
to workaround the deadlock I faced on Mac.
The deadlock has the following nature:

- EDT: holding the paintLock (a shared lock b/w JLF and SwingNode), and 
the cursor manager dives to native code and tries to invoke a method on 
Main (FX App) thread.
- FX Renderer: is about to render a SwingNode content, waiting on the 
paintLock.
- FX App: (as far as I can guess) waiting for the Renderer to finish.

I can start looking for the solution in parallel with the review, and if 
not yet found, I'd push the first patch with cursor updates disabled.

Thanks!
Anton.

>
> 08.02.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.
>>
>
>