On Fri, 16 Feb 2024 08:45:59 GMT, Christoph Langer <clan...@openjdk.org> wrote:

>> src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsDevice.cpp 
>> line 191:
>> 
>>> 189:         return;
>>> 190:     }
>>> 191:     VERIFY(::GetDIBits(hBMDC, hBM, 0, 1, NULL, gpBitmapInfo, 
>>> DIB_RGB_COLORS));
>> 
>> I think the return value of `::GetDIBits` should also be analysed explicitly.
>> 
>> I would prefer backing out if any of the functions above including 
>> `::GetDIBits` returns an error.
>> 
>> Yet I'm not sure how critical it is to run the code below. [As I 
>> mentioned](https://github.com/openjdk/jdk/pull/17197#issuecomment-1875562325)
>>  in PR #17197, there's a workaround for a case where `::GetDIBits` returns 
>> 0. @MBaesken implemented a fallback to run the code below if even if all the 
>> above calls fail. It may be safe to ignore … in headless environment.
>
> Hm, I added some code for verification of ::GetDIBits result. If it is 0, I 
> skip the rest of the coding. I don't want to touch more here since I don't 
> understand what it is doing.

It is not what I meant… If we want to preserve the way it work, we should 
execute the below code even if `this->GetDC()` fails…

On the other hand, testing didn't find any problems, in which case I prefer 
bailing out after `::GetDIBits` fails too.

@prrace, Do you know how critical the code below is? Is it worth to code the 
fallback if the path to `::GetDIBits`?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17614#discussion_r1513292270

Reply via email to