04.09.2012 16:37, Alexander Zuev пишет:
Sergey,
reviewing that on 2d alias can take a pretty long time to complete. As
for the questions see answers inside:
but anyway this is 2d code.
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;
}
So old surface is not invalidated? Strange logic we call
getBackupSurface() twice in this case?
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
in this case we should not call screenByDisplayId and release?
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
--
Best regards, Sergey.