Re: AWT Dev [8] Review request for 7142091: [macosx] RFE: Refactoring of peer initialization/disposing

2012-06-25 Thread Artem Ananiev

Hi, Sergey,

this version looks fine.

Thanks,

Artem

On 6/21/2012 2:42 PM, Sergey Bylokhov wrote:

Hello,
new version of the fix:
http://cr.openjdk.java.net/~serb/7142091/webrev.01/
- invokeLater was removed from setVisible and dispose.
- instanceof Graphics2D was added.

Run some awt related jck and regression tests, no new issues found.

On 18.06.2012 19:13, Artem Ananiev wrote:

Hi, Sergey,

some minor comments (may be unrelated to the fix):

1. LWComponentPeer.setVisible() can be made final. Do expect any
subclass to override it instead of setVisibleImpl()?

It is overriden in CPrinterDialogPeer.


2. invokeLater() in LWWindowPeer.setVisibleImpl(): I realize this is
really painful issue, but I'd vote for removing this workaround. It
would result in faster startup (although, the window will be solid
gray for some time), and make LWAWT code similar to what we have on
other platforms.

removed


Then next step will to minimize the delay between showing the window
and painting its content.

3. LWWindowPeer.replaceSurfaceData(): what are benefits of
setBackground() + clearRect() over setColor() + fillRect()? Although
we always expect the Graphics object to be Graphics2D instance, this
unconditional cast doesn't look great.

done


Thanks,

Artem

On 5/31/2012 5:43 PM, Sergey Bylokhov wrote:

Hi Everyone,
Please review the fix.

Notes from the bug and comments:
1. setVisible() should be called at the end of the peers initialization.
We can move super.initialize() to the end of the peers initializations.
Initialize() was split to initialize() and initializeImpl(). In the
initialize() we call initializeImpl and then we call to setVisible().
initializeImpl overridden in subclasses.

2. Invokelater in the initialization/disposing is a tricky.
Left it as is. Probably later it will be changed. Comments was updated.

3. replaceSurfacedata() should be moved outside of
LWWindowPeer.setVisible()
Done. Also duplicate code was extracted to setVisible() method which
call setVisibleImpl().

4. Backbuffer in replaceSurfacedata() should be initialized by clearRect
instead of fillrect(composite is important).
Done. related to composite.

5. During lwwindowpeer initialization we call two similar methods
nativeSetNSWindowAlpha() and setAlphaValue().
nativeSetNSWindowAlpha() removed from CPlatformWindow.java.


Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7142091
Webrev can be found at:
http://cr.openjdk.java.net/~serb/7142091/webrev.00/






Re: AWT Dev [8] Review request for 7142091: [macosx] RFE: Refactoring of peer initialization/disposing

2012-06-22 Thread Anthony Petrov

Hi Sergey,

The fix looks fine to me, although I don't see changes to the 
CPrinterDialogPeer in this webrev.


--
best regards,
Anthony

On 6/21/2012 2:42 PM, Sergey Bylokhov wrote:

Hello,
new version of the fix:
http://cr.openjdk.java.net/~serb/7142091/webrev.01/
- invokeLater was removed from setVisible and dispose.
- instanceof Graphics2D was added.

Run some awt related jck and regression tests, no new issues found.

On 18.06.2012 19:13, Artem Ananiev wrote:

Hi, Sergey,

some minor comments (may be unrelated to the fix):

1. LWComponentPeer.setVisible() can be made final. Do expect any 
subclass to override it instead of setVisibleImpl()?

It is overriden in CPrinterDialogPeer.


2. invokeLater() in LWWindowPeer.setVisibleImpl(): I realize this is 
really painful issue, but I'd vote for removing this workaround. It 
would result in faster startup (although, the window will be solid 
gray for some time), and make LWAWT code similar to what we have on 
other platforms.

removed


Then next step will to minimize the delay between showing the window 
and painting its content.


3. LWWindowPeer.replaceSurfaceData(): what are benefits of 
setBackground() + clearRect() over setColor() + fillRect()? Although 
we always expect the Graphics object to be Graphics2D instance, this 
unconditional cast doesn't look great.

done


Thanks,

Artem

On 5/31/2012 5:43 PM, Sergey Bylokhov wrote:

Hi Everyone,
Please review the fix.

Notes from the bug and comments:
1. setVisible() should be called at the end of the peers initialization.
We can move super.initialize() to the end of the peers initializations.
Initialize() was split to initialize() and initializeImpl(). In the
initialize() we call initializeImpl and then we call to setVisible().
initializeImpl overridden in subclasses.

2. Invokelater in the initialization/disposing is a tricky.
Left it as is. Probably later it will be changed. Comments was updated.

3. replaceSurfacedata() should be moved outside of
LWWindowPeer.setVisible()
Done. Also duplicate code was extracted to setVisible() method which
call setVisibleImpl().

4. Backbuffer in replaceSurfacedata() should be initialized by clearRect
instead of fillrect(composite is important).
Done. related to composite.

5. During lwwindowpeer initialization we call two similar methods
nativeSetNSWindowAlpha() and setAlphaValue().
nativeSetNSWindowAlpha() removed from CPlatformWindow.java.


Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7142091
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/7142091/webrev.00/







Re: AWT Dev [8] Review request for 7142091: [macosx] RFE: Refactoring of peer initialization/disposing

2012-06-21 Thread Sergey Bylokhov

Hello,
new version of the fix:
http://cr.openjdk.java.net/~serb/7142091/webrev.01/
- invokeLater was removed from setVisible and dispose.
- instanceof Graphics2D was added.

Run some awt related jck and regression tests, no new issues found.

On 18.06.2012 19:13, Artem Ananiev wrote:

Hi, Sergey,

some minor comments (may be unrelated to the fix):

1. LWComponentPeer.setVisible() can be made final. Do expect any 
subclass to override it instead of setVisibleImpl()?

It is overriden in CPrinterDialogPeer.


2. invokeLater() in LWWindowPeer.setVisibleImpl(): I realize this is 
really painful issue, but I'd vote for removing this workaround. It 
would result in faster startup (although, the window will be solid 
gray for some time), and make LWAWT code similar to what we have on 
other platforms.

removed


Then next step will to minimize the delay between showing the window 
and painting its content.


3. LWWindowPeer.replaceSurfaceData(): what are benefits of 
setBackground() + clearRect() over setColor() + fillRect()? Although 
we always expect the Graphics object to be Graphics2D instance, this 
unconditional cast doesn't look great.

done


Thanks,

Artem

On 5/31/2012 5:43 PM, Sergey Bylokhov wrote:

Hi Everyone,
Please review the fix.

Notes from the bug and comments:
1. setVisible() should be called at the end of the peers initialization.
We can move super.initialize() to the end of the peers initializations.
Initialize() was split to initialize() and initializeImpl(). In the
initialize() we call initializeImpl and then we call to setVisible().
initializeImpl overridden in subclasses.

2. Invokelater in the initialization/disposing is a tricky.
Left it as is. Probably later it will be changed. Comments was updated.

3. replaceSurfacedata() should be moved outside of
LWWindowPeer.setVisible()
Done. Also duplicate code was extracted to setVisible() method which
call setVisibleImpl().

4. Backbuffer in replaceSurfacedata() should be initialized by clearRect
instead of fillrect(composite is important).
Done. related to composite.

5. During lwwindowpeer initialization we call two similar methods
nativeSetNSWindowAlpha() and setAlphaValue().
nativeSetNSWindowAlpha() removed from CPlatformWindow.java.


Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7142091
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/7142091/webrev.00/





--
Best regards, Sergey.



Re: AWT Dev [8] Review request for 7142091: [macosx] RFE: Refactoring of peer initialization/disposing

2012-06-18 Thread Artem Ananiev

Hi, Sergey,

some minor comments (may be unrelated to the fix):

1. LWComponentPeer.setVisible() can be made final. Do expect any 
subclass to override it instead of setVisibleImpl()?


2. invokeLater() in LWWindowPeer.setVisibleImpl(): I realize this is 
really painful issue, but I'd vote for removing this workaround. It 
would result in faster startup (although, the window will be solid gray 
for some time), and make LWAWT code similar to what we have on other 
platforms.


Then next step will to minimize the delay between showing the window and 
painting its content.


3. LWWindowPeer.replaceSurfaceData(): what are benefits of 
setBackground() + clearRect() over setColor() + fillRect()? Although we 
always expect the Graphics object to be Graphics2D instance, this 
unconditional cast doesn't look great.


Thanks,

Artem

On 5/31/2012 5:43 PM, Sergey Bylokhov wrote:

Hi Everyone,
Please review the fix.

Notes from the bug and comments:
1. setVisible() should be called at the end of the peers initialization.
We can move super.initialize() to the end of the peers initializations.
Initialize() was split to initialize() and initializeImpl(). In the
initialize() we call initializeImpl and then we call to setVisible().
initializeImpl overridden in subclasses.

2. Invokelater in the initialization/disposing is a tricky.
Left it as is. Probably later it will be changed. Comments was updated.

3. replaceSurfacedata() should be moved outside of
LWWindowPeer.setVisible()
Done. Also duplicate code was extracted to setVisible() method which
call setVisibleImpl().

4. Backbuffer in replaceSurfacedata() should be initialized by clearRect
instead of fillrect(composite is important).
Done. related to composite.

5. During lwwindowpeer initialization we call two similar methods
nativeSetNSWindowAlpha() and setAlphaValue().
nativeSetNSWindowAlpha() removed from CPlatformWindow.java.


Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7142091
Webrev can be found at: http://cr.openjdk.java.net/~serb/7142091/webrev.00/



Re: AWT Dev [8] Review request for 7142091: [macosx] RFE: Refactoring of peer initialization/disposing

2012-06-09 Thread Anthony Petrov

Hi Sergey,

Thanks for clarifications. Please find my comments below.

On 6/5/2012 8:13 PM, Sergey Bylokhov wrote:

3. src/macosx/classes/sun/lwawt/LWWindowPeer.java

 179 updateInsets(platformWindow.getInsets());


The system may report wrong insets before a window is shown on the 
screen. Perhaps, instead of initializeImpl() we should introduce 
preInitialize() (== current initializeImpl()), and postInitialize() 
(to where this, and the subsequent replaceSurfaceData() calls might go.)
Like it was done in XAWT? It is just a nightmare. I assume that 


What kind of nightmare do you mean? To me it looks logical to perform 
some tasks before, and some other tasks after disaplying a component. 
Hence the need for per- and post- initializers.


updateInsets() should be called by some of the native callbacks, like 
notifyExpose and reshape?


Nope. It's only called from the initialize() method once since on the 
Mac insets never change. Therefore, it is critical to call it only after 
showing the window on the screen.


On 6/5/2012 10:17 PM, Sergey Bylokhov wrote:

In this case In current implementation it works wrong too, because it can be 
called when the window initially invisible.


So far, it works fine. Please run insets-related tests (simply all 
automatic tests for Window, Frame, and Dialog both open and closed) to 
makes sure they pass.



4.

 220 protected void setVisibleImpl(final boolean visible) {
 221 super.setVisibleImpl(visible);


Why do we remove a call to replaceSurfaceData() in the beginning of 
the method?
Before the fix, setVisible() can be called before surface creation, but 
after the fix it will be called after.


Could you clarify this further please? What exactly is the reason to 
move the replaceSurfaceData() call from setVisible() to initializeImpl()?




5.
 983 ((Graphics2D) 
g).setBackground(getBackground());


I suggest to add an instanceof check before this call.

I guess that Buffered image cannot return something except Graphics2D


I guess so too. Nevertheless, I still suggest to add such a check.



6.
 227 // invokeLater() can be deleted, but in this case we get 
a lag between

 228 // windows showing and content painting.


Is the lag so very noticeable? On other platforms we don't actually do 
this, and I don't recall any issues about such lags.
Yes The difference is noticeable. So I update the comments and leave it 
as is for now.


Interesting. This needs to be investigated, perhaps under a separate CR.

--
best regards,
Anthony



--
best regards,
Anthony

On 5/31/2012 5:43 PM, Sergey Bylokhov wrote:

Hi Everyone,
Please review the fix.

Notes from the bug and comments:
1. setVisible() should be called at the end of the peers 
initialization. We can move super.initialize() to the end of the 
peers initializations.
Initialize() was split to initialize() and initializeImpl(). In the 
initialize() we call initializeImpl and then we call to setVisible(). 
initializeImpl overridden in subclasses.


2. Invokelater in the initialization/disposing is a tricky.
Left it as is. Probably later it will be changed. Comments was updated.

3. replaceSurfacedata() should be moved outside of 
LWWindowPeer.setVisible()
Done. Also duplicate code was extracted to setVisible() method which 
call setVisibleImpl().


4. Backbuffer in replaceSurfacedata() should be initialized by 
clearRect instead of fillrect(composite is important).

Done. related to composite.

5. During lwwindowpeer initialization we call two similar methods 
nativeSetNSWindowAlpha() and setAlphaValue().

nativeSetNSWindowAlpha() removed from CPlatformWindow.java.


Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7142091
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/7142091/webrev.00/







Re: AWT Dev [8] Review request for 7142091: [macosx] RFE: Refactoring of peer initialization/disposing

2012-06-09 Thread Sergey Bylokhov

Hi Anthony.
See my comments inline:

09.06.2012 17:34, Anthony Petrov wrote:

Hi Sergey,

Thanks for clarifications. Please find my comments below.

On 6/5/2012 8:13 PM, Sergey Bylokhov wrote:

3. src/macosx/classes/sun/lwawt/LWWindowPeer.java

 179 updateInsets(platformWindow.getInsets());


The system may report wrong insets before a window is shown on the 
screen. Perhaps, instead of initializeImpl() we should introduce 
preInitialize() (== current initializeImpl()), and postInitialize() 
(to where this, and the subsequent replaceSurfaceData() calls might 
go.)
Like it was done in XAWT? It is just a nightmare. I assume that 


What kind of nightmare do you mean? To me it looks logical to perform 
some tasks before, and some other tasks after disaplying a component. 
Hence the need for per- and post- initializers.
Because in general nobody know correct place for initialization. 
Moreover we can do something after component showing in setVisble(true) 
only.


updateInsets() should be called by some of the native callbacks, like 
notifyExpose and reshape?


Nope. It's only called from the initialize() method once since on the 
Mac insets never change. Therefore, it is critical to call it only 
after showing the window on the screen.
But what happens when the peer is invisible by default?  Probably the 
better place for updateInsets is setVisible()?


On 6/5/2012 10:17 PM, Sergey Bylokhov wrote:
In this case In current implementation it works wrong too, because it 
can be called when the window initially invisible.


So far, it works fine. Please run insets-related tests (simply all 
automatic tests for Window, Frame, and Dialog both open and closed) to 
makes sure they pass.

See previous comment.



4.

 220 protected void setVisibleImpl(final boolean visible) {
 221 super.setVisibleImpl(visible);


Why do we remove a call to replaceSurfaceData() in the beginning of 
the method?
Before the fix, setVisible() can be called before surface creation, 
but after the fix it will be called after.


Could you clarify this further please? What exactly is the reason to 
move the replaceSurfaceData() call from setVisible() to initializeImpl()?
Just because it is unnecessary in setVisble() because surface already 
created at the end of LWWindow.initializeImpl(). Why we should try to 
replace surface each time in setVisible()?




5.
 983 ((Graphics2D) 
g).setBackground(getBackground());


I suggest to add an instanceof check before this call.

I guess that Buffered image cannot return something except Graphics2D


I guess so too. Nevertheless, I still suggest to add such a check.

ok I will add.




6.
 227 // invokeLater() can be deleted, but in this case we 
get a lag between

 228 // windows showing and content painting.


Is the lag so very noticeable? On other platforms we don't actually 
do this, and I don't recall any issues about such lags.
Yes The difference is noticeable. So I update the comments and leave 
it as is for now.


Interesting. This needs to be investigated, perhaps under a separate CR.

I will create new CR.


--
best regards,
Anthony



--
best regards,
Anthony

On 5/31/2012 5:43 PM, Sergey Bylokhov wrote:

Hi Everyone,
Please review the fix.

Notes from the bug and comments:
1. setVisible() should be called at the end of the peers 
initialization. We can move super.initialize() to the end of the 
peers initializations.
Initialize() was split to initialize() and initializeImpl(). In the 
initialize() we call initializeImpl and then we call to 
setVisible(). initializeImpl overridden in subclasses.


2. Invokelater in the initialization/disposing is a tricky.
Left it as is. Probably later it will be changed. Comments was 
updated.


3. replaceSurfacedata() should be moved outside of 
LWWindowPeer.setVisible()
Done. Also duplicate code was extracted to setVisible() method 
which call setVisibleImpl().


4. Backbuffer in replaceSurfacedata() should be initialized by 
clearRect instead of fillrect(composite is important).

Done. related to composite.

5. During lwwindowpeer initialization we call two similar methods 
nativeSetNSWindowAlpha() and setAlphaValue().

nativeSetNSWindowAlpha() removed from CPlatformWindow.java.


Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7142091
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/7142091/webrev.00/








--
Best regards, Sergey.



Re: AWT Dev [8] Review request for 7142091: [macosx] RFE: Refactoring of peer initialization/disposing

2012-06-05 Thread Anthony Petrov

Hi Sergey,

1. src/macosx/classes/sun/lwawt/LWComponentPeer.java

 196 delegate = createDelegate();
 197 if (delegate != null) {
 198 delegate.setVisible(false);



The call at line 198 looks unnatural here. It looks as if the delegate 
is created visible initially which isn't true, is it?


2.

 204 delegate.setOpaque(true);


Related to the above comment, does this call mean that a delegate is 
created non-opaque initially? I feel uncomfortable with these calls. Do 
we workaround something with these calls? Can we give them more 
appropriate and meaningful names then?


3. src/macosx/classes/sun/lwawt/LWWindowPeer.java

 179 updateInsets(platformWindow.getInsets());


The system may report wrong insets before a window is shown on the 
screen. Perhaps, instead of initializeImpl() we should introduce 
preInitialize() (== current initializeImpl()), and postInitialize() (to 
where this, and the subsequent replaceSurfaceData() calls might go.)


4.

 220 protected void setVisibleImpl(final boolean visible) {
 221 super.setVisibleImpl(visible);


Why do we remove a call to replaceSurfaceData() in the beginning of the 
method?


5.

 983 ((Graphics2D) g).setBackground(getBackground());


I suggest to add an instanceof check before this call.

6.

 227 // invokeLater() can be deleted, but in this case we get a lag 
between
 228 // windows showing and content painting.


Is the lag so very noticeable? On other platforms we don't actually do 
this, and I don't recall any issues about such lags.


--
best regards,
Anthony

On 5/31/2012 5:43 PM, Sergey Bylokhov wrote:

Hi Everyone,
Please review the fix.

Notes from the bug and comments:
1. setVisible() should be called at the end of the peers initialization. 
We can move super.initialize() to the end of the peers initializations.
Initialize() was split to initialize() and initializeImpl(). In the 
initialize() we call initializeImpl and then we call to setVisible(). 
initializeImpl overridden in subclasses.


2. Invokelater in the initialization/disposing is a tricky.
Left it as is. Probably later it will be changed. Comments was updated.

3. replaceSurfacedata() should be moved outside of 
LWWindowPeer.setVisible()
Done. Also duplicate code was extracted to setVisible() method which 
call setVisibleImpl().


4. Backbuffer in replaceSurfacedata() should be initialized by clearRect 
instead of fillrect(composite is important).

Done. related to composite.

5. During lwwindowpeer initialization we call two similar methods 
nativeSetNSWindowAlpha() and setAlphaValue().

nativeSetNSWindowAlpha() removed from CPlatformWindow.java.


Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7142091
Webrev can be found at: http://cr.openjdk.java.net/~serb/7142091/webrev.00/



Re: AWT Dev [8] Review request for 7142091: [macosx] RFE: Refactoring of peer initialization/disposing

2012-06-05 Thread Sergey Bylokhov

Hi Anthony.
Thanks for review. See comments inline.
05.06.2012 19:47, Anthony Petrov wrote:

Hi Sergey,

1. src/macosx/classes/sun/lwawt/LWComponentPeer.java

 196 delegate = createDelegate();
 197 if (delegate != null) {
 198 delegate.setVisible(false);



The call at line 198 looks unnatural here. It looks as if the delegate 
is created visible initially which isn't true, is it?

Delegate is visible by default, but our awt peer is not.


2.

 204 delegate.setOpaque(true);


Related to the above comment, does this call mean that a delegate is 
created non-opaque initially? I feel uncomfortable with these calls. 
Do we workaround something with these calls? Can we give them more 
appropriate and meaningful names then?

Most of aqua components are non opaque by default, but our awt peer not.


3. src/macosx/classes/sun/lwawt/LWWindowPeer.java

 179 updateInsets(platformWindow.getInsets());


The system may report wrong insets before a window is shown on the 
screen. Perhaps, instead of initializeImpl() we should introduce 
preInitialize() (== current initializeImpl()), and postInitialize() 
(to where this, and the subsequent replaceSurfaceData() calls might go.)
Like it was done in XAWT? It is just a nightmare. I assume that 
updateInsets() should be called by some of the native callbacks, like 
notifyExpose and reshape?


4.

 220 protected void setVisibleImpl(final boolean visible) {
 221 super.setVisibleImpl(visible);


Why do we remove a call to replaceSurfaceData() in the beginning of 
the method?
Before the fix, setVisible() can be called before surface creation, but 
after the fix it will be called after.


5.
 983 ((Graphics2D) 
g).setBackground(getBackground());


I suggest to add an instanceof check before this call.

I guess that Buffered image cannot return something except Graphics2D


6.
 227 // invokeLater() can be deleted, but in this case we get 
a lag between

 228 // windows showing and content painting.


Is the lag so very noticeable? On other platforms we don't actually do 
this, and I don't recall any issues about such lags.
Yes The difference is noticeable. So I update the comments and leave it 
as is for now.


--
best regards,
Anthony

On 5/31/2012 5:43 PM, Sergey Bylokhov wrote:

Hi Everyone,
Please review the fix.

Notes from the bug and comments:
1. setVisible() should be called at the end of the peers 
initialization. We can move super.initialize() to the end of the 
peers initializations.
Initialize() was split to initialize() and initializeImpl(). In the 
initialize() we call initializeImpl and then we call to setVisible(). 
initializeImpl overridden in subclasses.


2. Invokelater in the initialization/disposing is a tricky.
Left it as is. Probably later it will be changed. Comments was updated.

3. replaceSurfacedata() should be moved outside of 
LWWindowPeer.setVisible()
Done. Also duplicate code was extracted to setVisible() method which 
call setVisibleImpl().


4. Backbuffer in replaceSurfacedata() should be initialized by 
clearRect instead of fillrect(composite is important).

Done. related to composite.

5. During lwwindowpeer initialization we call two similar methods 
nativeSetNSWindowAlpha() and setAlphaValue().

nativeSetNSWindowAlpha() removed from CPlatformWindow.java.


Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7142091
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/7142091/webrev.00/





--
Best regards, Sergey.



Re: AWT Dev [8] Review request for 7142091: [macosx] RFE: Refactoring of peer initialization/disposing

2012-06-05 Thread Sergey Bylokhov


The system may report wrong insets before a window is shown on the 
screen. Perhaps, instead of initializeImpl() we should introduce 
preInitialize() (== current initializeImpl()), and postInitialize() 
(to where this, and the subsequent replaceSurfaceData() calls might go.)


In this case In current implementation it works wrong too, because it 
can be called when the window initially invisible.


05.06.2012 19:47, Anthony Petrov wrote:

Hi Sergey,

1. src/macosx/classes/sun/lwawt/LWComponentPeer.java

 196 delegate = createDelegate();
 197 if (delegate != null) {
 198 delegate.setVisible(false);



The call at line 198 looks unnatural here. It looks as if the delegate 
is created visible initially which isn't true, is it?


2.

 204 delegate.setOpaque(true);


Related to the above comment, does this call mean that a delegate is 
created non-opaque initially? I feel uncomfortable with these calls. 
Do we workaround something with these calls? Can we give them more 
appropriate and meaningful names then?


3. src/macosx/classes/sun/lwawt/LWWindowPeer.java

 179 updateInsets(platformWindow.getInsets());


The system may report wrong insets before a window is shown on the 
screen. Perhaps, instead of initializeImpl() we should introduce 
preInitialize() (== current initializeImpl()), and postInitialize() 
(to where this, and the subsequent replaceSurfaceData() calls might go.)


4.

 220 protected void setVisibleImpl(final boolean visible) {
 221 super.setVisibleImpl(visible);


Why do we remove a call to replaceSurfaceData() in the beginning of 
the method?


5.
 983 ((Graphics2D) 
g).setBackground(getBackground());


I suggest to add an instanceof check before this call.

6.
 227 // invokeLater() can be deleted, but in this case we get 
a lag between

 228 // windows showing and content painting.


Is the lag so very noticeable? On other platforms we don't actually do 
this, and I don't recall any issues about such lags.


--
best regards,
Anthony

On 5/31/2012 5:43 PM, Sergey Bylokhov wrote:

Hi Everyone,
Please review the fix.

Notes from the bug and comments:
1. setVisible() should be called at the end of the peers 
initialization. We can move super.initialize() to the end of the 
peers initializations.
Initialize() was split to initialize() and initializeImpl(). In the 
initialize() we call initializeImpl and then we call to setVisible(). 
initializeImpl overridden in subclasses.


2. Invokelater in the initialization/disposing is a tricky.
Left it as is. Probably later it will be changed. Comments was updated.

3. replaceSurfacedata() should be moved outside of 
LWWindowPeer.setVisible()
Done. Also duplicate code was extracted to setVisible() method which 
call setVisibleImpl().


4. Backbuffer in replaceSurfacedata() should be initialized by 
clearRect instead of fillrect(composite is important).

Done. related to composite.

5. During lwwindowpeer initialization we call two similar methods 
nativeSetNSWindowAlpha() and setAlphaValue().

nativeSetNSWindowAlpha() removed from CPlatformWindow.java.


Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7142091
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/7142091/webrev.00/





--
Best regards, Sergey.