Skip to content

Conversation

@nedtwigg
Copy link

@nedtwigg nedtwigg commented Mar 12, 2025

  • Use monitor-aware coordinates in multi-zoom coordinate system #1711 made Point and Rectangle part of a sealed class hierarchy which added MonitorAware variants
  • this new hierarchy causes two problems
    • users cannot instantiate Point or Rectangle from Kotlin, because Kotlin does not allow the base of a sealed hierarchy to be one of the elements
    • even for Java users, equality is now muddled. Two points can have the same x, y values, but not be equal. For example, take a point, map it to a control, map it back, you'll get the same coordinates you started with, but .equals will be false now because one of the points will be a MonitorAware. This behavior only happens on Windows.

I'm proposing two PRs. This, the first one, doesn't fix the muddled equality bug, but it makes MonitorAware a package-private implementation detail that only affects the win32 platform.

The second one (#1904) removes MonitorAware and replaces it with logic that is contained within the MultiZoomCoordinateMapper. As of SWT in the release 4.35, these new MonitorAware variants are created in exactly two places.

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

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

Unfortunately, this is not possible for multiple reasons:

  1. The MonitorAware... classes are used from another package, so this change does not compile. We also wanted to make both the coordinate system mappers as well as the monitor-aware classes internal, but there is unfortunately no JLS-conforming way to do that, so we decided to make the monitor-aware classes public but marking them as @noreference.
  2. Point and Rectangle were final and are not supposed to be subclassed in an unexpected way (i.e., outside SWT). That's why sealed types are a valid replacement but just removing the final modifier is not.

@nedtwigg
Copy link
Author

@nedtwigg nedtwigg closed this Mar 13, 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 this pull request may close these issues.

2 participants