RFR: 8185862: AWT Assertion Failure in ::GetDIBits(hBMDC, hBM, 0, 1, 0, gpBitmapInfo, 0) 'awt_Win32GraphicsDevice.cpp', at line 185 [v2]

[Christoph Langer]

On Thu, 15 Feb 2024 09:31:34 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Christoph Langer has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add comments
>
> src/java.desktop/windows/native/libawt/windows/Devices.cpp line 96:
> 
>> 94: 
>> 95: // Callback for CountMonitors below
>> 96: BOOL WINAPI clb_fCountMonitors(HMONITOR hMon, HDC hDC, LPRECT rRect, LPARAM lP)
> 
> Can `clb_fCountMonitors` be declared `static`? It's not used from other translation units.

Done.

> src/java.desktop/windows/native/libawt/windows/Devices.cpp line 143:
> 
>> 141:         } else {
>> 142:             J2dTraceLn1(J2D_TRACE_INFO, "Devices::CollectMonitors: GetMonitorInfo failed for monitor with handle %p", hMon);
>> 143:         }
> 
> The code to verify the monitor is repeated. To avoid code duplication, I suggest moving it into a helper function `IsValidMonitor` or `CanCreateDCForMonitor` and use the helper in both `clb_fCountMonitors` and `clb_fCollectMonitors`.

Done. Good suggestion.

> src/java.desktop/windows/native/libawt/windows/Devices.cpp line 149:
> 
>> 147: }
>> 148: 
>> 149: int WINAPI CollectMonitors(HMONITOR* hmpMonitors, int nNum)
> 
> As far as I can see both `CollectMonitors` and `CountMonitors` above can be declared `static`.

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17614#discussion_r1492137483
PR Review Comment: https://git.openjdk.org/jdk/pull/17614#discussion_r1492136615
PR Review Comment: https://git.openjdk.org/jdk/pull/17614#discussion_r1492137874