Skip to content

Conversation

@akoch-yatta
Copy link
Contributor

This PR wraps the GC initialization for the ImageGcDrawer to directly pass the zoom from the wrapper when the GC is initialized. In all other cases the GC will be initialized with 100 zoom.

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2025

Test Results

   545 files  ±0     545 suites  ±0   28m 4s ⏱️ - 4m 59s
 4 376 tests ±0   4 358 ✅ ±0   18 💤 ±0  0 ❌ ±0 
16 643 runs  ±0  16 503 ✅ ±0  140 💤 ±0  0 ❌ ±0 

Results for commit 004719a. ± Comparison against base commit cb6e7a6.

♻️ This comment has been updated with latest results.

*/
@Override
public long internal_new_GC (GCData data) {
return configureGC(data, 100);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for clarification: why do we use 100 instead of DPIUtil.getNativeDeviceZoom() here? Wouldn't the latter be more consistent with existing behavior (i.e., in case monitor-specific scaling is disabled an images are drawn this way)?

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 know GC works better with 100% then with e.g. 125 or 175 that could be provided with DPIUtil.getNativeDeviceZoom() so I would like to prevent using DPIUtil.getNativeDeviceZoom() as default

Copy link
Contributor

Choose a reason for hiding this comment

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

I was concerned that this changes behavior if now for the same calling code a GC at a different zoom is created. Isn't that the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it sure does. Before DPIUtil.getNativeZoom was used, but not for a specific reason, especially not for the GC, but just for creating the probably most suitable initial handle here So, for this default case I would argue, no value is really the correct value here. I chose 100 out of two reasons

  • drawing on the GC works best with 100% - e.g. having GEF in mind.
  • all "easy" use cases involving GC and Images should be covered with ImageGcDrawer, leaving the complex ones (like. GEF) which brings me back to the upper point

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the clarification!
Just like in #2108 (comment), I was mistakenly thinking that the image data may be at a wrong size and that may lead to issues when drawing on them, but since the drawing happens in point coordinates it will always work.
The decision makes sense to me.

This commits wraps the GC initialization for the ImageGcDrawer to directly
pass the zoom from the wrapper when the GC is initialized. In all other
cases the GC will be initialized with 100 zoom.
@HeikoKlare HeikoKlare force-pushed the win32-improve-image-gcdrawer-initialization branch from cb53cbc to 004719a Compare May 6, 2025 14:43
@HeikoKlare HeikoKlare merged commit f10721a into eclipse-platform:master May 6, 2025
17 checks passed
@HeikoKlare HeikoKlare deleted the win32-improve-image-gcdrawer-initialization branch May 6, 2025 15: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.

2 participants