On Thu, 15 Feb 2024 09:31:34 GMT, Alexey Ivanov <aiva...@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