<AWT Dev> [8] Review request for CR 8006406: lightweight embedding in other Java UI toolkits
Anthony Petrov
anthony.petrov at oracle.com
Tue Feb 19 04:22:29 PST 2013
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.
--
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