Bug 969

Created attachment 590 [details]
Basic NewtCanvasSWT tester with Sash

If we add a println(event) in NewtCanvasSWT (constr) -> listener -> handleEvent we see that:

 If we use a GridLayout (NewtSashTest.java / L42) for the NewtCanvasSWT parent, SWT.Resize events ARE NOT sent when resizing, but only once in the beginning. In OSX (Mavericks) SWT.Paint events are sent while resizing

 If we use a FillLayout (NewtSashTest.java / L41) SWT.Resize events ARE sent in the beginning and while resizing. SWT.Paint events, only in OSX when resizing

Without fix for Bug 672, NewtCanvasSWT in OSX does not get position updates, but does so in Win7x64. The fix was to take advantage of the SWT.Paint events to update position of the newtChild:

  if( SWTAccessor.isOSX ) {
    newtChild.setPosition(parent.getLocation().x,parent.getLocation().y);
  }

Problem comes with multiple parents in different places like in bug 965

Created attachment 591 [details]
patch to fix positioning in OSX

proposed patch is to use the existing SWT method Control.setBounds(int x, int y, int width, int height); which trickles down from parent elements to help update the newtChild's position. Thus, no need for hacking the SWT.Paint event. The method getLocationOnScreen() in SWTNativeWindow is also simplified (for OSX) to use SWT's Control.toDisplay(int x, int y) which returns the position of the parent element on the screen. setBounds() can then position the element from that point, since the numbers it provides are relative to the parent element. 

The new positioning requires the newtChild to be manually positioned when it is created, otherwise the parent has to be resized for it to update its position. Thus newtChild.setPosition(getLocation().x, getLocation().y); has been placed after the creation of the SWTNativeWindow in validateNative().

Last but not least, since setBounds() will now provide coordinates relative to the parent, if the NewtCanvasSWT is not moved within the parent, it will not be updated due to line 2090 in WindowImpl.java:

if ( !isFullscreen() && ( getX() != x || getY() != y ) ) {

this line has thus been changed to allow for moving the newtChild as long as the parent is not null:

if ( !isFullscreen() && ( getX() != x || getY() != y || null != getParent()) ) {

Apart from the last piece in WindowImpl.java, all the others are wrapped with checks to only be executed in OSX since windows seems to be handling the positioning of child windows on its own...

78fcb8228d4a391054501aef16eb0462322ba39d

Refer to commit: https://github.com/orange-vertex/jogl/commit/78fcb8228d4a391054501aef16eb0462322ba39d

Tested on OSX Mavericks and Windows 7 JVMx64. Refer to test TestBug672NewtCanvasSWTSashForm as it is a better solution for the same problem.

(In reply to comment #2)
> Created attachment 591 [details]
> patch to fix positioning in OSX
> 
> proposed patch is to use the existing SWT method Control.setBounds(int x,
> int y, int width, int height); which trickles down from parent elements to
> help update the newtChild's position. Thus, no need for hacking the
> SWT.Paint event. The method getLocationOnScreen() in SWTNativeWindow is also
> simplified (for OSX) to use SWT's Control.toDisplay(int x, int y) which
> returns the position of the parent element on the screen. setBounds() can
> then position the element from that point, since the numbers it provides are
> relative to the parent element. 

We discussed a probable case for a deadlock, i.e. if 
we need to run the command on the SWT-EDT
and the caller is on another thread.
Is this deadlock threat still exisiting or will you call toDisplay(..) 
always while on the SWT-EDT ?

> 
> The new positioning requires the newtChild to be manually positioned when it
> is created, otherwise the parent has to be resized for it to update its
> position. Thus newtChild.setPosition(getLocation().x, getLocation().y); has
> been placed after the creation of the SWTNativeWindow in validateNative().
> 
> Last but not least, since setBounds() will now provide coordinates relative
> to the parent, if the NewtCanvasSWT is not moved within the parent, it will
> not be updated due to line 2090 in WindowImpl.java:
> 
> if ( !isFullscreen() && ( getX() != x || getY() != y ) ) {
> 
> this line has thus been changed to allow for moving the newtChild as long as
> the parent is not null:
> 
> if ( !isFullscreen() && ( getX() != x || getY() != y || null != getParent())
> ) {

OK, as discussed

> 
> Apart from the last piece in WindowImpl.java, all the others are wrapped
> with checks to only be executed in OSX since windows seems to be handling
> the positioning of child windows on its own...

Please mention the test case name and platform for which you have
tested this patch and can confirm it's working.

Thank you.

(In reply to comment #3)
> 78fcb8228d4a391054501aef16eb0462322ba39d
> 
> Refer to commit:
> https://github.com/orange-vertex/jogl/commit/
> 78fcb8228d4a391054501aef16eb0462322ba39d
> 
> Tested on OSX Mavericks and Windows 7 JVMx64. Refer to test
> TestBug672NewtCanvasSWTSashForm as it is a better solution for the same
> problem.


Thank you!

Fix probably landed in version 2.1.5