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

[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/
>>>
>
>