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.

Reply via email to