On Tue, 16 Apr 2024 06:58:19 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:

>> This clean-up PR removes unused Windows version macro from 
>> `ShellFolder2.cpp`.
>> 
>> `IS_WINVISTA` was not used at all.
>> 
>> `IS_WINXP` guarded support for icons with alpha channel. It is now safe to 
>> assume Java runs on a Windows version later than Windows XP. Java launchers 
>> specify 6.0 as the minimum OS version which corresponds to Windows Vista.
>
> src/java.desktop/windows/native/libawt/windows/ShellFolder2.cpp line 131:
> 
>> 129: #ifndef IS_WINVISTA
>> 130: #define IS_WINVISTA (!(::GetVersion() & 0x80000000) && 
>> LOBYTE(LOWORD(::GetVersion())) >= 6)
>> 131: #endif
> 
> there are exactly the same macro in awt.h which are mostly unused except 
> "IS_WIN8"

There's a separate bug for removing the macro from `awt.h`:

[JDK-8289772](https://bugs.openjdk.org/browse/JDK-8289772): Remove obsolete 
Windows version macros: IS_WIN2000, IS_WINXP, IS_WINVISTA

Why am I taking `ShellFolder2.cpp` separately? The comment in the 
`ShellFolder2.cpp` file suggest these are copies from `awt.h`. This makes 
`ShellFolder2.cpp` isolated from any other usage. Removing the macro cleans up 
a single file, easy to do, easy to review.

Removing the macro from `awt.h` is not as simple, some macro are used in the 
code. For example, `IS_WIN8` is used in `awt_Toolkit.cpp` to determine whether 
touch keyboard handling is enabled.

https://github.com/openjdk/jdk/blob/58911ccc2c48b4b5dd2ebc9d2a44dcf3737eca50/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.cpp#L669

Then `IS_WINVISTA` is used in the code which determines whether DWM is 
available; `IS_WINXP` is used in many places to determine menu bar colors, to 
handle subclassing default Windows controls. Removing the macro will make the 
code cleaner but it requires modifying the code. Eventually, the task may be 
broken into smaller pieces.

The usage of `IS_WIN8` could be replaced with 
[`IsWindows8OrGreater`](https://learn.microsoft.com/en-us/windows/win32/api/VersionHelpers/nf-versionhelpers-iswindows8orgreater)
 which is part of [Version Helper 
functions](https://learn.microsoft.com/en-us/windows/win32/sysinfo/version-helper-apis)
 and is the recommended method to determine the version of the Windows OS.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18736#discussion_r1567192769

Reply via email to