Skip to content

Conversation

@HeikoKlare
Copy link
Contributor

With recent changes to the rounding of pixel-to-point conversions the calculated sizes became too small in several use cases, leading to cut-off elements. This change adapts the rounding method to reduce the number of relevant cut-offs.

Follow-up to:

How to test

Issues caused by pixel/point conversion roundings can best be seen at 125% or 175% monitor zoom.

Before at 125%:
image

After at 125%:
image

@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2025

Test Results

  118 files  + 3    118 suites  +3   14m 41s ⏱️ + 3m 40s
4 650 tests +37  4 633 ✅ +36  17 💤 +1  0 ❌ ±0 
  330 runs  +18    326 ✅ +17   4 💤 +1  0 ❌ ±0 

Results for commit c2c78ac. ± Comparison against base commit 4722846.

♻️ This comment has been updated with latest results.

@HeikoKlare HeikoKlare marked this pull request as ready for review October 30, 2025 18:45
With recent changes to the rounding of pixel-to-point conversions the
calculated sizes became too small in several use cases, leading to
cut-off elements. This change adapts the rounding method to reduce the
number of relevant cut-offs.
@HeikoKlare HeikoKlare force-pushed the pixelToPoint-no-round-down branch from ed21ba2 to c2c78ac Compare October 31, 2025 08:24
@akoch-yatta akoch-yatta linked an issue Oct 31, 2025 that may be closed by this pull request
}

public static Point pixelToPointAsSize(Point point, int zoom) {
return pixelToPoint(point, zoom, RoundingMode.DOWN);
Copy link
Contributor

@arunjose696 arunjose696 Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HeikoKlare now as pixelToPointAsLocation and pixelToPointAsSize do the same thing, do we even need these separate methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not necessarily need them, but we already embedded the distinction at all callers and we would of course still like to find a better concept for rounding the values, so to work on that it may be easier to keep the distinction for now. Otherwise, for the same reasons, you could also get rid of all the RoundingMode and just have a special handling for the case of computeSize().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds fair

@arunjose696
Copy link
Contributor

Tested this changes at 125 and 175% zoom this seems to resolve the mentioned issue, when testing around I can also see the issee vi-eclipse/Eclipse-Platform#499 seems to be resolved for me at 125% zoom

Copy link
Contributor

@akoch-yatta akoch-yatta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and the explanation makes sense. Tested it and it confirmed the observation from @arunjose696 for me

@akoch-yatta akoch-yatta merged commit 56379d5 into eclipse-platform:master Oct 31, 2025
17 checks passed
@akoch-yatta akoch-yatta deleted the pixelToPoint-no-round-down branch October 31, 2025 15:12
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 this pull request may close these issues.

Cut-off controls due to rounding down

3 participants