<AWT Dev> [8] Review request for CR 8006406: lightweight embedding in other Java UI toolkits
Anton V. Tarasov
anton.tarasov at oracle.com
Tue Feb 19 05:14:37 PST 2013
On 19.02.2013 16:22, Anthony Petrov wrote:
>
> On 2/19/2013 16:05, Anton V. Tarasov wrote:
>> On 19.02.2013 14:28, Anthony Petrov wrote:
>>> (please find my reply inline)
>>>
>>> On 2/19/2013 14:14, Anton V. Tarasov wrote:
>>>> On 19.02.2013 13:24, Anthony Petrov wrote:
>>>>> On 2/18/2013 18:10, Anton V. Tarasov wrote:
>>>>>> On 18.02.2013 16:50, Anthony Petrov wrote:
>>>>>>> As far as I know there's no RFEs filed that request to provide public API for grab/ungrab
>>>>>>> functionality. Also, it isn't clear whether this API could be useful for GUI applications.
>>>>>>
>>>>>> Current grab API was created as a draft, with an intention to make it public later. However
>>>>>> you're right, there's no RFE which could prove this fact (but we did have in our todo list :)).
>>>>>
>>>>> Right. My point is that I doubt this API could be useful for application developers.
>>>>>
>>>>>
>>>>>>> As such I don't find it ultimately needed to extend the WindowPeer interface with
>>>>>>> grabFocus/ungrabFocus() methods, nor add these methods as public interface to the
>>>>>>> LightweightFrame class at this time.
>>>>>>>
>>>>>>> Instead, I propose to introduce a sun.awt.FocusGrabbable interface with two methods:
>>>>>>> grabFocus() and ungrabFocus(). This interface needs to be implemented by the
>>>>>>> JLightweightFrame class only, where we can provide reasonable implementation by redirecting
>>>>>>> the calls to FX. In SunToolkit.grab()/ungrab() we should check for instanceof
>>>>>>> FocusGrabbable, and call the methods appropriately. Otherwise, if the window doesn't
>>>>>>> implement this interface, we proceed as usual.
>>>>>>>
>>>>>>> This would eliminate changes to the WindowPeer, remove the unneeded
>>>>>>> LightweightFrame.grab/ungrabFocus() methods, and generally simplify the fix (e.g. we won't
>>>>>>> need any changes in XWindowPeer, etc.)
>>>>>>>
>>>>>>> Opinions?
>>>>>>
>>>>>> At least, your suggestion bring us back to the discussion about the direction of the call
>>>>>> chain. What should a user expect from the JLF.grabFocus() method? I think the same as from
>>>>>> the SunToolkit.grab(JLF), right?
>>>>>> However, your version of JLF.grabFocus() looks more like a callback - JLF.notifyGrabbed. It
>>>>>> won't do real grab, but will tell the frame that someone has grabbed focus on it. Evolving
>>>>>> this approach, I'd say that,
>>>>>> from AWT perspective, it would be more natural to have a listener - GrabListener, and
>>>>>> GrabEvent/UngrabEvent appropriately. In which case, we would need to add a GrabEvent class, a
>>>>>> pair for the existing UngrabEvent.
>>>>>> This seems consistent to me (with AWT standarts). But at the same time, IMHO, this doesn't
>>>>>> worth the candle. And so, I still think the original approach better serves the needs.
>>>>>
>>>>> I think I didn't provide you with any version of "my" JLF.grabFocus() yet, so here it is:
>>>>>
>>>>> public interface sun.awt.FocusGrabbable {
>>>>> void grabFocus();
>>>>> void ungrabFocus(boolean postEvent);
>>>>> }
>>>>>
>>>>> As you can see, these are very much the same grab/ungrabFocus() methods that you have in your
>>>>> current fix. I suggest to implement this interface in the JLightweightFrame class only, since
>>>>> this is where they are really needed, and their implementation is entirely different from
>>>>> others (including the LightweightFrame, which in the end does the same thing as e.g. Frame,
>>>>> currently).
>>>>>
>>>>>
>>>>>> By the way, current SunToolkit.ungrab() implementation does ungrab silently, without posting
>>>>>> UngrabEvent. This is not enough for FX SwingNode. And thus, we anyway need
>>>>>> SunToolkit.ungrab(boolean) or
>>>>>> SunToolkit.ungrabAndPost() or something of the like. LightweightFrame.ungrabFocus(boolean)
>>>>>> looks better to me.
>>>>>
>>>>> Now, in e.g. WToolkit the grab() method should look like:
>>>>>
>>>>> 987 public void grab(Window w) {
>>>>> 991 if (w instanceof FocusGrabbable) {
>>>>> 992 ((FocusGrabbable)w).grabFocus();
>>>>> 993 return;
>>>>> 994 }
>>>>> 995 if (w.getPeer() != null) {
>>>>> 996 ((WWindowPeer)w.getPeer()).grab();
>>>>> 997 }
>>>>> 998 }
>>>>
>>>>>
>>>>> and a similar implementation with a check for FocusGrabbable for the ungrab().
>>>>>
>>>>> I hope the above clarifies my proposal well enough.
>>>>
>>>> Anthony,
>>>>
>>>> Sorry, but I don't understand your proposal.
>>>>
>>>> In the fix, LightweightFrame does the following:
>>>>
>>>> 113 public void grabFocus() {
>>>> 114 ((WindowPeer)getPeer()).grabFocus();
>>>> 115 }
>>>>
>>>>
>>>> In the above code you call JLF.grabFocus() and then return. At the same time, you're rejecting
>>>> grab/ungrab in WindowPeer, from what I can conclude you're not going to call the peer (line 114
>>>> above).
>>>> So, how JLF.grabFocus() will _grab_? (Currently, only a peer does grab/ungrab logic).
>>>>
>>>> When SunToolkit.grab(Window) is called for JLF it should do the following:
>>>>
>>>> 1. grab like a generic frame
>>>> 2. notify (the client) via LightweightContent instance about the grab
>>>>
>>>> I'd understand the following logic:
>>>>
>>>> public void grab(Window w) {
>>>> if (w instanceof FocusGrabbable) {
>>>> ((FocusGrabbable)w).grabFocus(); // 2. notify
>>>> }
>>>> if (w.getPeer() != null) {
>>>> ((WWindowPeer)w.getPeer()).grab(); // 1. do grab
>>>> }
>>>> }
>>>>
>>>> in which case it notifies and then grabs. But, as I wrote before, here the name grabFocus()
>>>> doesn't seem correct, but something like notifyGrabbed() would make sense.
>>>
>>> I don't understand why you need to call WWindowPeer.grab(). In our use case, we embed Swing
>>> content in an FX window. When Swing code requests to grab focus, we should simply redirect this
>>> request to FX (i.e. call TKStage.grabFocus() for the FX window). That's all what is needed. We
>>> don't need to also arm the AWT's native grab machinery in this case. We expect FX to send us an
>>> UNGRAB event when the focus is ungrabbed.
>>>
>>> Why do you want to call WWindowPeer.grab()? What will break if you remove the following line:
>>>
>>> src/share/classes/sun/swing/JLightweightFrame.java:
>>>> 102 super.grabFocus();
>>>
>>> ? I believe nothing should break, and this call is just unneeded here.
>>
>> I suppose you suggest to remove super.ungrabFocus(boolean) as well. Then, in the following scenario:
>>
>> 1. I show a swing popup in JLF, it calls SunToolkit.grab(JLF) which just forwards the grab to the
>> fx stage.
>>
>> 2. I click in the title of the stage. It initiates ungrab, I intercept the FocusUngrabEvent and
>> call SunToolkit.ungrab(true) (let's assume we have SunToolkit.ungrab(boolean)).
>> It's no op, because AWT grab has not been armed. JLF won't receive UngrabEvent.
>
> FX's UNGRAB_FOCUS event should be translated directly into an AWT's UngrabEvent. We don't need to
> call ungrab() upon receiving this event. We should just send a corresponding AWT event. This is
> exactly what JFXPanel does, but the other way around. I've sent you a link (privately) to a webrev
> that shows what we do for JFXPanel in this case.
>
>
>> Or you think we should post UngrabEvent instead of calling SunToolkit.ungrab? From my point of
>> view this is an interference into the mechanism. Yes, it's (currently) simple, but still this
>> approach doesn't seem correct to me.
>> The proposed fix is reusing the logic, not changing it, not half-copying. This also means less
>> chances to face pitfalls.
>
> I don't see how this looks like "half-copying" to you. A request is translated into a request, an
> event - into an event. Everything is symmetrical.
Well, I think I see your point now. Let me think if this is indeed symmetrical...
Thanks,
Anton.
>
>
> --
> best regards,
> Anthony
>
>>
>> Thanks,
>> Anton.
>>
>>
>>
>>
>>>
>>>
>>>>>> I don't think having it in WindowPeer is a big price. Moreover, the implementation is already
>>>>>> in the peers. I just added the methods to the peer interfaces.
>>>>>
>>>>> I don't think this is necessary at this moment. Besides, not adding them to the interfaces
>>>>> allows us to avoid implementing them in XWindowPeer, WWindowPeer, and LWWindowPeer, too. Also,
>>>>> we absolutely don't need public grab/ungrabFocus() methods in LightweightFrame, too. Hence, as
>>>>> you can see, the fix could be simplified.
>>>>
>>>> All *WindowPeer classes _already_ implement grab/ungrab functionality. What I'm doing is just
>>>> refining the interface.
>>>
>>> I'd be OK if you removed the old methods (e.g. WWindowPeer.grab()) and used the new ones
>>> (grabFocus()) exclusively. However, my above proposal suggests that we just don't need to change
>>> this part at all.
>>>
>>> --
>>> best regards,
>>> Anthony
>>>
>>>>
>>>> Please, clarify your point...
>>>>
>>>> Thanks,
>>>> Anton.
>>>>
>>>>>
>>>>>
>>>>>> And also, com.sun.javafx.tk.TKStage has grabFocus() and ungrabFocus() methods (the latter
>>>>>> does send the event with no option). Why not in AWT? At worst, it can stay in
>>>>>> sun.awt.LightweightFrame forever.
>>>>>
>>>>> TKStage is a peer interface, it is not a part of public FX API. Also, AWT has its own,
>>>>> "legacy" code structure, and I don't see a reason to change it anyhow at this time.
>>>>>
>>>>> --
>>>>> best regards,
>>>>> Anthony
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Anton.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> best regards,
>>>>>>> Anthony
>>>>>>>
>>>>>>> On 02/15/13 13:56, Anton V. Tarasov wrote:
>>>>>>>> Hi Jim,
>>>>>>>>
>>>>>>>> I agree with the rest of your comments, fixed it.
>>>>>>>>
>>>>>>>> Here's the latest:
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ant/8006406/webrev.8
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~ant/8006406/javadoc.8
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>> Anton.
>>>>>>>>
>>>>>>>> On 2/15/13 3:56 AM, Jim Graham wrote:
>>>>>>>>> Great Anton,
>>>>>>>>>
>>>>>>>>> Some spelling fixes.
>>>>>>>>>
>>>>>>>>> occures -> occurs
>>>>>>>>>
>>>>>>>>> {@code {i, j}} -> {@code (i, j)}
>>>>>>>>> (I usually use parens to encapculate coordinates, but maybe there are
>>>>>>>>> conventions in other places that use braces?)
>>>>>>>>>
>>>>>>>>> It might be good to put parens around the (0 <= i/j < width) formulas?
>>>>>>>>> But that would depend on how it looks in a browser and I didn't
>>>>>>>>> compile and javadoc the code to see that - if it looks fine without
>>>>>>>>> the parens then that's good.
>>>>>>>>>
>>>>>>>>> But, the description(s) look(s) good and accurate. Thanks!
>>>>>>>>>
>>>>>>>>> ...jim
>>>>>>>>>
>>>>>>>>> On 2/14/13 5:51 AM, Anton V. Tarasov wrote:
>>>>>>>>>> Hi Jim,
>>>>>>>>>>
>>>>>>>>>> On 2/14/13 6:26 AM, Jim Graham wrote:
>>>>>>>>>>> I've been busy with FX things, but I just got a chance to look at some
>>>>>>>>>>> of the new interfaces.
>>>>>>>>>>
>>>>>>>>>> Ok, thanks =)
>>>>>>>>>>
>>>>>>>>>>> Here are some (hopefully) minor comments:
>>>>>>>>>>>
>>>>>>>>>>> LightweightContent:
>>>>>>>>>>>
>>>>>>>>>>> You never really define "image origin". The imageBufferReset() takes
>>>>>>>>>>> an x,y, but it doesn't state what those are referring to. Is that the
>>>>>>>>>>> x,y on the screen/scene where the image should be rendered to? Are
>>>>>>>>>>> they the values to use to figure out what the starting offset in the
>>>>>>>>>>> data array for the data for the image should be? One thing that would
>>>>>>>>>>> help would be to include a formula in the method comments that
>>>>>>>>>>> indicates how the data for pixels is retrieved from the buffer so
>>>>>>>>>>> there is no confusion, something like:
>>>>>>>>>>>
>>>>>>>>>>> -----
>>>>>>>>>>> The {w} and {h} should match the width and height of the component
>>>>>>>>>>> returned from {getComponent()} with the pixel at the origin of the
>>>>>>>>>>> component, {(0, 0)} in the coordinate space of the component,
>>>>>>>>>>> appearing at {data[y * linestride + x]}. All indices {data[(y + j) *
>>>>>>>>>>> linestride + (x + i)]} where {0 <= i < w} and {0 <= j < h} will
>>>>>>>>>>> represent valid pixel data for the component.
>>>>>>>>>>> -----
>>>>>>>>>>>
>>>>>>>>>>> Did I interpret that correctly?
>>>>>>>>>>
>>>>>>>>>> Yes, the formula is correct. I've put it into the doc:
>>>>>>>>>>
>>>>>>>>>> /**
>>>>>>>>>> * {@code JLightweightFrame} calls this method to notify the client
>>>>>>>>>> application that a new data buffer
>>>>>>>>>> * has been set as a content pixel buffer. Typically this occures
>>>>>>>>>> when a buffer of a larger size is
>>>>>>>>>> * created in response to a content resize event. The method
>>>>>>>>>> reports a reference to the pixel data buffer,
>>>>>>>>>> * the content image bounds within the buffer and the line stride
>>>>>>>>>> of the buffer. These values have the
>>>>>>>>>> * following correlation.
>>>>>>>>>> * <p>
>>>>>>>>>> * The {@code width} and {@code height} matches the size of the
>>>>>>>>>> content (the component returned from the
>>>>>>>>>> * {@link #getComponent} method). The {@code x} and {@code y} is
>>>>>>>>>> the origin of the content, {@code {0, 0}}
>>>>>>>>>> * in the coordinate space of the content, appearing at {@code
>>>>>>>>>> data[y * linestride + x]} in the buffer.
>>>>>>>>>> * All indices {@code data[(y + j) * linestride + (x + i)]} where
>>>>>>>>>> {@code 0 <= i < width} and {@code 0 <= j < height}
>>>>>>>>>> * will represent valid pixel data, {@code {i, j}} in the
>>>>>>>>>> coordinate space of the content.
>>>>>>>>>> *
>>>>>>>>>> * @param data the content pixel data buffer of INT_ARGB_PRE type
>>>>>>>>>> * @param x the x coordinate of the image
>>>>>>>>>> * @param y the y coordinate of the image
>>>>>>>>>> * @param width the width of the image
>>>>>>>>>> * @param height the height of the image
>>>>>>>>>> * @param linestride the line stride of the pixel buffer
>>>>>>>>>> */
>>>>>>>>>> public void imageBufferReset(int[] data, int x, int y, int width,
>>>>>>>>>> int height, int linestride);
>>>>>>>>>>
>>>>>>>>>> Is that enough clear now?
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Then when you refer to xywh in imageReshaped I'm guessing it is just
>>>>>>>>>>> supplying 4 new parameters to replace the identical parameters that
>>>>>>>>>>> were in the Reset() method?
>>>>>>>>>>
>>>>>>>>>> That's right. There're three distinct events: buffer recreation (always
>>>>>>>>>> or usually connected with new image bounds), image reshape (may not be
>>>>>>>>>> connected to the first event)
>>>>>>>>>> and image update (may not be connected to the first and second events).
>>>>>>>>>> So, I thought all the three events should be reflected separately.
>>>>>>>>>>
>>>>>>>>>>> Then in imageUpdated(), are the xywh relative to the coordinate system
>>>>>>>>>>> of the Component? Or are they in the same space as the original xywh
>>>>>>>>>>> were supplied to imageBufferReset? When you say they are "relative to
>>>>>>>>>>> the origin" I think you mean the former. The thing that makes it
>>>>>>>>>>> difficult to describe that is that you have the parameters to Reset
>>>>>>>>>>> and Reshape both named x,y and the parameters to Updated are also
>>>>>>>>>>> named x,y and one set of x,y parameters is relative to the other set
>>>>>>>>>>> and you end up having to say "The x and y are relative to the x and
>>>>>>>>>>> y". One of the sets of parameters should be renamed to make it easier
>>>>>>>>>>> to discuss how they relate. Some sort of "All indices in the range
>>>>>>>>>>> ..." statement would help to show how all of the numbers relate to
>>>>>>>>>>> each other.
>>>>>>>>>>
>>>>>>>>>> OK, I put it into the formula:
>>>>>>>>>>
>>>>>>>>>> /**
>>>>>>>>>> * {@code JLightweightFrame} calls this method to notify the client
>>>>>>>>>> application that a part of
>>>>>>>>>> * the content image, or the whole image has been updated. The
>>>>>>>>>> method reports bounds of the
>>>>>>>>>> * rectangular dirty region. The {@code dirtyX} and {@code dirtyY}
>>>>>>>>>> is the origin of the dirty
>>>>>>>>>> * rectangle, which is relative to the origin of the content,
>>>>>>>>>> appearing at
>>>>>>>>>> * {@code data[(y + dirtyY] * linestride + (x + dirtyX)] in the
>>>>>>>>>> pixel buffer
>>>>>>>>>> * (see {@link #imageBufferReset}). All indices {@code data[(y +
>>>>>>>>>> dirtyY + j] * linestride +
>>>>>>>>>> * (x + dirtyX + i)]} where {@code 0 <= i < dirtyWidth} and {@code
>>>>>>>>>> 0 <= j < dirtyHeight}
>>>>>>>>>> * will represent valid pixel data, {@code {i, j}} in the
>>>>>>>>>> coordinate space of the dirty rectangle.
>>>>>>>>>> *
>>>>>>>>>> * @param dirtyX the x coordinate of the dirty rectangle, relative
>>>>>>>>>> to the image origin
>>>>>>>>>> * @param dirtyY the y coordinate of the dirty rectangle, relative
>>>>>>>>>> to the image origin
>>>>>>>>>> * @param dirtyWidth the width of the dirty rectangle
>>>>>>>>>> * @param dirtyHeight the height of the dirty rectangle
>>>>>>>>>> *
>>>>>>>>>> * @see #imageBufferReset
>>>>>>>>>> * @see #imageReshaped
>>>>>>>>>> */
>>>>>>>>>> public void imageUpdated(int dirtyX, int dirtyY, int dirtyWidth,
>>>>>>>>>> int dirtyHeight);
>>>>>>>>>>
>>>>>>>>>>> In SwingNode:
>>>>>>>>>>>
>>>>>>>>>>> Why is getContent() not just "return content;"?
>>>>>>>>>>
>>>>>>>>>> Actually, I had some more complicated construction there (wrapped
>>>>>>>>>> content ref), in which I needed the local copy, then I reduced it, but
>>>>>>>>>> missed the fact that it's even simpler now.
>>>>>>>>>> Thanks for noticing, I'll fix it.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Have you discussed the threading issues with anyone in FX? There is a
>>>>>>>>>>> big discussion right now on the appropriate threads for various
>>>>>>>>>>> activities...
>>>>>>>>>>
>>>>>>>>>> Not yet. I think we can start this discussion in the context of the
>>>>>>>>>> review of the fx part (which actually does the threading stuff).
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Anton.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ...jim
>>>>>>>>>>>
>>>>>>>>>>> On 2/13/13 4:57 AM, Anton V. Tarasov wrote:
>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
More information about the awt-dev
mailing list