<AWT Dev> [8] Please review fix for 7175704: [macosx] "8" PIT: NPE in GetDisplayMode fullscreen test
Leonid Romanov
leonid.romanov at oracle.com
Wed Oct 10 03:04:41 PDT 2012
Looks good.
On 09.10.2012, at 14:38, Alexander Zuev <alexander.zuev at oracle.com> wrote:
> Because on Mac we are currently supporting display mode change operation in non-fullscreen mode.
>
> On 10/9/12 14:35, Sergey Bylokhov wrote:
>> Also why we don't throw IllegalStateException("Must be in fullscreen mode in order to set display mode") when getFullScreenWindow==null?
>>
>> 09.10.2012 14:31, Sergey Bylokhov wrote:
>>> 09.10.2012 14:25, Alexander Zuev wrote:
>>>> Sergey,
>>>>
>>>> the text message is not specified in the documentation and the new one is already used on MacOS X
>>>> in the jdk7 so i would let it be as it is for consistency.
>>> I don't see a reason not to unify it across tolkits. Why we should use different text on different platforms?
>>>> The check in obtaining matching display mode is already performed in the native code:
>>>> In CGraphicsDevice.m :
>>>>
>>>> CGDisplayModeRef closestMatch = getBestModeForParameters(allModes, (int)w, (int)h, (int)bpp, (int)refrate);
>>>> if (closestMatch != NULL) {
>>>> ...
>>>> } else {
>>>> [JNFException raise:env as:kIllegalArgumentException reason:"Invalid display mode"];
>>>> }
>>>>
>>>> With best regards,
>>>> Alexander Zuev
>>>>
>>>> On 10/9/12 14:12, Sergey Bylokhov wrote:
>>>>> Hi, Alexander
>>>>> On win32 anx X11 we have another exception text message "Invalid display mode", also there is additional check in getMatchingDisplayMode() is it unnecessary on the macosx?
>>>>>
>>>>> 09.10.2012 12:35, Alexander Zuev wrote:
>>>>>> Hello,
>>>>>>
>>>>>> please review my fix for CR 7175704: [macosx] "8" PIT: NPE in GetDisplayMode fullscreen test
>>>>>>
>>>>>> The NPE happens because we do not check the passed argument to null before using it. The correct exception according to the specification should be IllegalArgumentException.
>>>>>>
>>>>>> Bug description is: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7175704
>>>>>> Webrev can be found here: http://cr.openjdk.java.net/~kizune/7175704/webrev.00
>>>>>>
>>>>>> With best regards,
>>>>>> Alex
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>>
>
More information about the awt-dev
mailing list