Skip to content

Conversation

akoch-yatta
Copy link
Contributor

To remove the memory footprint of Widget this PR removes the value for NATIVE_ZOOM from the data attribute and replaces it by calculating the value in Control#getData on demand.
This does not change the value that is returned by NATIVE_ZOOM.

@akoch-yatta akoch-yatta force-pushed the win32-remove-nativezoom-from-data branch from 07fe3d9 to 4b8c0fd Compare September 17, 2025 11:33
@akoch-yatta akoch-yatta linked an issue Sep 17, 2025 that may be closed by this pull request

@Override
public Object getData(String key) {
if (DATA_NATIVE_ZOOM.equals(key)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need DATA_NATIVE_ZOOM at all?

Cant all places that currently use that key simply perform the code itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently it is some kind of a hidden feature, that should be replaced in the long run by an API solution. We want to evolve the usage of this data point (e.g. in GEF as an example) with this before we introduce a unnecessary or not suitable API. But until there is a proper usable solution, we would like to at least reduce the memory footprint of that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain how/where it is used and why a "hidden key" is better than a (preliminary) API? One would suspect both to be internal and subject to change so the benefit is a bit unclear. Especially as it seems only ever a property of the shell itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general this data is currently needed for an experimental feature for GEF to disable the SWT autoscaling for the diagram canvas and incorporate all scaling to GEF. There we still need the information about the zoom, that would have been applied by SWT if it was not disabled.
So, I did some discussion with @HeikoKlare and we discussed assuming the solution (which is currently there for windows) can be applied in the same way to GTK and cocoa. But we are unsure about this, so we would like to prevent adding perhaps unnecessary API. The adaption to GTK and cocoa will most likely not happen in this release cycle.

Copy link
Contributor

github-actions bot commented Sep 17, 2025

Test Results

  118 files    118 suites   10m 48s ⏱️
4 583 tests 4 547 ✅ 36 💤 0 ❌
  330 runs    307 ✅ 23 💤 0 ❌

Results for commit 29f0f3c.

♻️ This comment has been updated with latest results.

@akoch-yatta akoch-yatta force-pushed the win32-remove-nativezoom-from-data branch 2 times, most recently from 735458c to f739d6a Compare October 2, 2025 13:41
To remove the memory footprint of Widget this commit removes the value
for NATIVE_ZOOM from the data attribute and replaces it by calculating
the value in Control#getData on demand. Additionally the naming is changed
to SHELL_ZOOM.
@akoch-yatta akoch-yatta force-pushed the win32-remove-nativezoom-from-data branch from f739d6a to 29f0f3c Compare October 2, 2025 13:56
@HeikoKlare HeikoKlare merged commit 648bb97 into eclipse-platform:master Oct 2, 2025
17 checks passed
@HeikoKlare HeikoKlare deleted the win32-remove-nativezoom-from-data branch October 2, 2025 14:11
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.

Remove native zoom data from widgets
3 participants