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

[Sergey Bylokhov]

Hi, Anthony.
Thanks for review.
09.01.2013 18:40, Anthony Petrov wrote:
> Hi Sergey,
>
> 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.
>
>
> 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.
>
>
> 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.

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


-- 
Best regards, Sergey.