Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scrollbars in Dark Theme broken on Windows Arm64 version 24H2 #1763

Closed
chirontt opened this issue Jan 25, 2025 · 4 comments · Fixed by #1765
Closed

Scrollbars in Dark Theme broken on Windows Arm64 version 24H2 #1763

chirontt opened this issue Jan 25, 2025 · 4 comments · Fixed by #1765
Milestone

Comments

@chirontt
Copy link
Contributor

Description
With the upgrade to version 24H2 of Windows on Arm64, the SWT scrollbars in Dark Theme remain light rather than turning dark/black as should be expected. This problem affects the Eclipse IDE release 2024-12, as well as the latest 4.35M2 platform SDK.

I guess this is a continuation of the work in PR #1172

cc @hmartinez82 @SyntevoAlex

To Reproduce
Here's the screenshot when running the test snippet Bug562043_DarkTableNoHover.java and using the relevant swt.jar as provided by the 4.35M2 platform SDK build:

Image

Expected behavior
The scrollbars should turn dark/black in Dark Theme.

Environment:

  1. Select the platform(s) on which the behavior is seen:
    • All OS
    • Windows on Arm64
    • Linux
    • macOS
  1. Additional OS info
    Here's the current 24H2 version of Windows on Arm64:

Image

  1. JRE/JDK version
>java -version
openjdk version "21.0.5" 2024-10-15 LTS
OpenJDK Runtime Environment Temurin-21.0.5+11 (build 21.0.5+11-LTS)
OpenJDK 64-Bit Server VM Temurin-21.0.5+11 (build 21.0.5+11-LTS, mixed mode)

Version since
The issue affects both Eclipse IDE version 2024-12 (4.34) and the latest 4.35M2 version of the platform SDK.

Workaround (or) Additional context
None, unless one is willing to downgrade back to the pre-24H2 version of Windows on Arm64.

Note that this issue happens on the Windows on Arm64 only; it doesn't happen on the Windows x64 even with 24H2 version.

@HeikoKlare
Copy link
Contributor

Seems like the update of the validation for dark theme due to a change in 24H2 was only applied to x64: #1547
@sratz do you think your fix can easily be extended to the ARM?

@sratz
Copy link
Member

sratz commented Jan 26, 2025

When I made #1547 I checked also the arm64 version, and the AllowDarkModeForWindowWithTelemetryId signature was unchanged for arm64.
However, it appears that SetPreferredAppMode has changed in 24H2 on arm64 instead (which I did not look at at the time).

Instead of

                             **************************************************************
                             *                          FUNCTION                          *
                             **************************************************************
                             PreferredAppMode __cdecl Ordinal_135(LONG param_1)
             PreferredAppMo    w0:4           <RETURN>
             LONG              w0:4           param_1
             undefined8        Stack[-0x10]:8 local_10                                XREF[1]:     171b23e14(W)  
             undefined8        Stack[-0x20]:8 local_20                                XREF[2]:     171b23e18(W), 
                                                                                                   171b23e38(*)  
                             0x23e10  135  
                             SetPreferredAppMode                             XREF[4]:     Entry Point(*), 171bbf89b(*), 
                             Ordinal_135                                                  171c183f0(*), 171c25380(*)  
       171b23e10 7f 23 03 d5     pacibsp
       171b23e14 f3 0f 1f f8     str        x19,[sp, #local_10]!
       171b23e18 fd 7b bf a9     stp        x29,x30,[sp, #local_20]!
       171b23e1c fd 03 00 91     mov        x29,sp
       171b23e20 e8 07 00 d0     adrp       x8,0x171c21000
       171b23e24 13 b9 4b b9     ldr        w19,[x8, #0xbb8]
       171b23e28 e1 03 00 2a     mov        w1,w0
       171b23e2c 00 e1 2e 91     add        x0,x8,#0xbb8
       171b23e30 10 79 ff 97     bl         _InterlockedExchange                             LONG _InterlockedExchange(LONG *
       171b23e34 e0 03 13 2a     mov        w0,w19
       171b23e38 fd 7b c1 a8     ldp        x29,x30,[sp], #0x10
       171b23e3c f3 07 41 f8     ldr        x19,[sp], #0x10
       171b23e40 ff 23 03 d5     autibsp
       171b23e44 c0 03 5f d6     ret

we now have only

                             **************************************************************
                             *                          FUNCTION                          *
                             **************************************************************
                             PreferredAppMode __cdecl Ordinal_135(PreferredAppMode pa
             PreferredAppMo    w0:4           <RETURN>
             PreferredAppMo    w0:4           param_1
                             0x43d50  135  
                             SetPreferredAppMode                             XREF[3]:     Entry Point(*), 171bcba4d(*), 
                             Ordinal_135                                                  171c03640(*)  
       171b43d50 48 06 00 f0     adrp       x8,0x171c0e000
       171b43d54 08 c1 3d 91     add        x8,x8,#0xf70
       171b43d58 e9 03 00 2a     mov        w9,w0
       171b43d5c 00 01 40 b9     ldr        w0,[x8]
       171b43d60 09 81 e9 b8     swpal      w9,w9,[x8]
       171b43d64 c0 03 5f d6     ret

The address of g_preferredAppMode now only appears once in ADRL (ADRP+ADD), so we can't double-check this value anymore like was done previously (in #1172).
However, the function is much simpler now, so we can validate the whole function (exclusing the dynamic address of g_preferredAppMode).

This whole approach of validating the function content in prevision that the exported ordinal (135) might change in future versions is really fragile.

I wonder if there is any precedence of such internal API ordinals ever having changed?

I'll provide a PR for the 24H2 validation.

@hmartinez82
Copy link
Contributor

hmartinez82 commented Jan 26, 2025

I wonder if there is any precedence of such internal API ordinals ever having changed?

My thoughts since the get go.

@sratz
Copy link
Member

sratz commented Jan 27, 2025

I wonder if there is any precedence of such internal API ordinals ever having changed?

My thoughts since the get go.

I've merged the change for now to get things running again, but maybe we really should consider relaxing/dropping these validations in the future.

@sratz sratz added this to the 4.35 M3 milestone Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants