[8] Request for review: 8003173 [macosx] Fullscreen on Mac leaves an empty rectangle

[Anthony Petrov]

On 1/9/2013 19:57, Sergey Bylokhov wrote:
> 09.01.2013 18:40, Anthony Petrov wrote:
>> src/macosx/classes/sun/lwawt/LWWindowPeer.java:
>> Note that theoretically the insets can be changed w/o changing the 
>> content size. For example, if a user switches to a theme with enlarged 
>> window decorations. Not sure if this applies to Mac presently, but in 
>> theory this is possible. Will sending the COMPONENT_RESIZE event be 
>> equivalent to calling the replaceSurfaceData() in this case? Also, 
>> since the event is only posted but not processed yet, what is the 
>> point to call repaintPeer() before the surface data is replaced?
> In general surfaceData should include top level size of the 
> window(including insets) So it's not necessary to replace surface here. 
> Because there is no api for target notification about new insets I use 
> COMPONENT_RESIZE event.

Thanks for clarifying that.

>> src/macosx/classes/sun/lwawt/macosx/CPlatformView.java:
>> The actual fix seems to reside in this file. Why doesn't 
>> peer.getInsets() return zeros in the full screen mode? If it does 
>> actually, why do we need this change then? A generic, 
>> insets-accounting size calculation seems to be preferable in case we 
>> need a non-zero insets for some specific use-cases in the future.
> When we set NSView to full screen the native insets(as we calculate it) 
> does not change. Because of that we use synthetic resize notifications 
> in enterFullScreenMode() we just should not include insets in this case.

At line 98 of CPlatformView.java in "peer.getInsets()", the peer is an 
LWWindowPeer instance associated with the view. In 
CPlatformWindow.enterFullScreenMode() you already call 
"peer.updateInsets(new Insets(0, 0, 0, 0))" which should zero out the 
insets stored in the same peer. So the peer.getInsets() call in 
CPlatformView will return these zeros, and hence they shouldn't affect 
the calculations you perform at lines 111-115 in CPlatformView.

So I repeat my question, why do we need to change anything in 
CPlatformView then? Can we just revert the changes?


>> src/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java:
>>>  876 peer.updateInsets(getInsets());
>>
>> This will call a native method upon sending every move/resize event. 
>> Why do we actually have to do this? I assume the peer already calls 
>> the PlatformWindow.getInsets() whenever needed.
> No. It does not call, that's the problem.
>> Also, AFAIK, insets rarely change on Mac, and you already handle their 
>> manual changes when entering/exiting the full screen mode. Can we just 
>> remove this line?
> No. There are two different fullscreens.
> 1 The full screen based on Nsview, which is used via Graphics device. 
> For this we emulate insets and events.
> 2 The full screen in lion style. We can handle it in 
> windowWillEnterFullScreen/windowDidEnterFullScreen. In the WillEnterXX 
> window have old insets and in the DidEnterXX window have new insets. 
> DidEnterXX comes after Resize events, which will repaint the window ==> 
> we get content's jumping.

So, why not simply call "peer.updateInsets(new Insets(0, 0, 0, 0))" from 
windowWillEnter... then? Otherwise this looks inconsistent because in 
the case of the GraphicsDevice-based full screen mode you zero out the 
insets manually, but they remain being non-zero in the native full 
screen mode.

--
best regards,
Anthony

>> src/macosx/native/sun/awt/AWTWindow.m
>>>  821     [ThreadUtilities performOnMainThreadWaiting:YES block:^(){
>>>  822         AWT_ASSERT_APPKIT_THREAD;
>>
>> This ASSERT statement may safely be removed since the ThreadUtilities 
>> already guarantee us that we're running on the main thread.
> I just use standard template from AWTWindow.m, so it will be simple to 
> change this template at once.
>>
>> -- 
>> best regards,
>> Anthony
>>
>> On 1/9/2013 17:32, Sergey Bylokhov wrote:
>>> Hello,
>>> Please review the fix.
>>> The reason why we have an empty space in the full screen mode is that 
>>> we did not update our insets.
>>>  - I update insets in the deliverMoveResizeEvent not in the 
>>> windowDidEnterFullScreen, in this case animation became more 
>>> smooth(since we update insets just before paint action). So all old 
>>> fullscreen handle methods in CPlatformWindow were removed.
>>>  - LWWindowPeer.updateInsets will post ComponentEvent if insets were 
>>> changed.
>>>  - CGraphicsDevice.setDisplayMode now stores/restores full screen 
>>> window, because otherwise NSView looks shifted on screen.
>>>  - CPlatformView.enterFullScreenMode(): code related to insets was 
>>> removed, since NSVIew uses the whole screen unlike jdk 6.(see related 
>>> changes in CPlatformWindow.enter/exitFullScreenMode)
>>>
>>>
>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8003173
>>> Webrev can be found at: 
>>> http://cr.openjdk.java.net/~serb/8003173/webrev.00
>>>
> 
>