<AWT Dev> [8] Please review fix for 7175183: [macosx] Objective-C exception thrown when switching monitor configuration

[Alexander Zuev]

Sergey,

   reviewing that on 2d alias can take a pretty long time to complete. 
As for the questions see answers inside:

On 9/4/12 15:08, Sergey Bylokhov wrote:
> Hi, Alexander.
> I think that it should be reviewed on 2d alias.
>
> But a few comments:
>  - It is unclear how getBackupSurface can set sdAccel to null, but if 
> it set, it should invalidate old surface.
getBackupSurface() by creating new surface data may trigger revalidate() 
call which (in method VolatileSurfaceManager.validate())
contains the code that, if acceleration mode has been disabled, 
nullifies the sdAccel variable:
         } else if (sdAccel != null) {
             // if the "acceleration enabled" state changed to disabled,
             // switch to software surface
             sdCurrent = getBackupSurface();
             sdAccel = null;
             returnCode = VolatileImage.IMAGE_RESTORED;
         }
Note that this does not happened often - only when the application is 
being reanimated from the
hibernate mode and during the hibernate the  external display was 
disconnected potentially resulting in
switching to the video adapter which does not support accelerated drawing.

>  - Same question about CWrapper: if pointer contains nil do we release 
> this object before?
If it is nil then it was not initialized in the first place, this means 
that displayID does not represent anymore a valid
device so when we are attempting to get 
CWrapper.NSScreen.screenByDisplayId(displayId) we are getting nil and
later trying to release it via CWrapper.NSObject.release(screen)

With best regards,
Alex

>
>
> 04.09.2012 13:48, Alexander Zuev wrote:
>> Hello,
>>
>>   please review my fix for CR 7175183: [macosx] Objective-C exception 
>> thrown when switching monitor configuration
>>
>>   The NPE happens because of the getBackupSurface method call results 
>> in the resetting of the current surface to null. The idea of the fix 
>> is to check for it in both native and java methods before using it.
>>
>> Bug description is: 
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7175183
>> Webrev can be found here: 
>> http://cr.openjdk.java.net/~kizune/7175183/webrev.00
>>
>> With best regards,
>> Alex
>
>