On Thu, 15 Feb 2024 09:31:34 GMT, Alexey Ivanov <[email protected]> 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
