eclipse-platform / eclipse.platform

https://eclipse.dev/eclipse/
Eclipse Public License 2.0
83 stars 113 forks source link

[Windows] Missing scrollbars in compare editor on non default screen scaling #1489

Closed merks closed 3 months ago

merks commented 3 months ago

Let's make sure issue is not already fixed in latest builds first.

Steps to reproduce

From a fresh installation and clean workspace:

Any files I compare in my updated SDK IDE, it doesn't show scroll bars:

image

But in a debug launched IDE it works.

I noticed this in other recently installed/updated IDEs.

Tested under this environment:

I'm on Windows 10.

iloveeclipse commented 3 months ago

@merks : could you please disable themes in preferences and check if that would "fix" the diff editor? @BeckerWdf : could it be related to latest theme changes?

merks commented 3 months ago

Neither no theme, nor the choice of theme has an impact. I tried clicking around in case it was just not visibly painted but actually there, but it really seems not to be there at all, just the blank area where it should be.

iloveeclipse commented 3 months ago

I see it now too. Linux is not affected. Bisecting... I20240612-0510 OK I20240614-1800 OK I20240617-1800 OK I20240618-1800 BAD I20240715-0020 BAD

iloveeclipse commented 3 months ago

I20240618-1800 BAD => Commit https://github.com/eclipse-platform/eclipse.platform.swt/commit/25b32ef8091c278928027b3326a83033874f134b from https://github.com/eclipse-platform/eclipse.platform.swt/pull/1209 is the bad one.

@ShahzaibIbrahim, @fedejeanne, the PR above broke compare editor on Windows. This is a blocker for 4.33, as it makes the compare editor hardly usable.

Please investigate as soon as possible!

ShahzaibIbrahim commented 3 months ago

Can I have more info on this? I tried reproducing on latest build with no VM arguments, Windows 11, Screen Resolution 1920x1080 and zoom level 100%.

I am unable to reproduce it for now. Perhaps, I am missing something?

image

iloveeclipse commented 3 months ago

Where one can find zoom value on Windows? I have Notebook with Win 11 and 1920x1080 resolution too.

ShahzaibIbrahim commented 3 months ago

Systems -> Display -> Scale

image

iloveeclipse commented 3 months ago

OK, I have 125%

iloveeclipse commented 3 months ago

After switching to 100% I see the tiny line for the scrollbar now, and it shows on hover, but I can't read the text anymore because it is all so small now :-)

merks commented 3 months ago

My primary display is also 125% The secondary is 225%.

ShahzaibIbrahim commented 3 months ago

It's now reproduced. It has indeed raised the regression. I will need time to investigate but in the meantime, I can either 1) Revert the change completely 2) or fix the regression with a small change but it won't fix the original issue entirely. (by original, I mean the issue my changes were fixing)

Kindly suggest.

iloveeclipse commented 3 months ago

4.33 M3 build is planned for 15 August.

The fix (or revert) should be done before, so we could deliver M3 for testing and could revert the fix if it would be still not OK.

ShahzaibIbrahim commented 3 months ago

I will create a new PR for this fix.

merks commented 3 months ago

Thanks folks!

HeikoKlare commented 3 months ago

To me, it look as if the introduced method simply uses the wrong zoom value. This line:

return OS.GetSystemMetricsForDpi(nIndex, DPIUtil.mapZoomToDPI(getZoom()));

should instead be:

return OS.GetSystemMetricsForDpi(nIndex, DPIUtil.mapZoomToDPI(nativeZoom));

That's why the issue only appear at 125% (and other quarter scales): getZoom() return 100% instead of the proper 125%.

In preliminary tests, this fixes the problem for me.

@ShahzaibIbrahim can you please further investigate whether that might be the cause?

ShahzaibIbrahim commented 3 months ago

To me, it look as if the introduced method simply uses the wrong zoom value. This line:

return OS.GetSystemMetricsForDpi(nIndex, DPIUtil.mapZoomToDPI(getZoom()));

should instead be:

return OS.GetSystemMetricsForDpi(nIndex, DPIUtil.mapZoomToDPI(nativeZoom));

That's why the issue only appear at 125% (and other quarter scales): getZoom() return 100% instead of the proper 125%.

In preliminary tests, this fixes the problem for me.

@ShahzaibIbrahim can you please further investigate whether that might be the cause?

You are right, this is exactly the issue. The higher the zoom level goes, the smaller scrollbar became because getZoom() returned 100% in case the zoom is 125% or 150% and 200% in case of 175%. Using nativeZoom has fixed the issue (I checked in all zoom levels)

iloveeclipse commented 3 months ago

Is this issue fixed? If yes, please close.

HeikoKlare commented 3 months ago

Has been fixed via:

I revalidated with latest I-Build (I20240826-2120): the original issue is not present