[Win32] Improve size calculation for cropped and scaled image drawing#3431
Open
HeikoKlare wants to merge 2 commits into
Open
[Win32] Improve size calculation for cropped and scaled image drawing#3431HeikoKlare wants to merge 2 commits into
HeikoKlare wants to merge 2 commits into
Conversation
Contributor
HeikoKlare
force-pushed
the
gc-drawimage-rounding
branch
from
July 20, 2026 20:20
31181b5 to
a839f6f
Compare
For images where width and height differ greatly (e.g. 500x2), computing the required zoom independently per axis using integer division can give a larger zoom estimate for the smaller axis. The old code selected the maximum of both estimates, which reliably picks the less accurate value and creates a handle at the wrong zoom level, causing noticeably incorrect scaling. The axis with more pixels has a proportionally smaller integer-division truncation error. The fix therefore derives the zoom from the axis whose pixel hint is larger, giving the most accurate zoom estimate. Contributes to eclipse-platform#3454 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When drawing cropped and scaled images, the calculation for the source rectangle to draw is quite error prone: - It does not distinguish between different scale factors in X and Y direction, leading to large rounding errors if the extents in one direction are highly different from the extents in the other direction - It does not apply rounding that is consistent to the scaling done by the Image class, thus leading to differently rounded sizes when scaling an image in the Image class and drawing that same image in the GC - The "error correction" to deal with rounding at fractional scale factors is too restrictive, in particular when the scale factor is less than 1, and is not applied when only one axis has a non-unity scale factor This change reimplements the source rectangle calculation as follows: - It treats the scale factors for both axes independently - It applies the same rounding method to the rectangle extents as done by the Image scaling implementation - It rounds up the scale factor when checking for the allowed size error on fractional scaling, such that a scale factor less than 1 still allows for an error of 1 in size - It applies the bounds correction whenever either axis has a non-unity scale factor, not only when both axes do A regression test is added to GCWin32Tests that verifies all three drawImage overloads (3-arg, 5-arg, and 9-arg) produce identical pixel output for a 500-wide image across a matrix of small prime heights and fractional zoom levels (100%, 125%, 150%, 175%, 200%). Fixes eclipse-platform#3454 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
HeikoKlare
force-pushed
the
gc-drawimage-rounding
branch
from
July 21, 2026 07:55
a839f6f to
41371f8
Compare
HeikoKlare
marked this pull request as ready for review
July 21, 2026 09:36
There was a problem hiding this comment.
Pull request overview
This PR addresses Win32 high-DPI inconsistencies in GC.drawImage(...) when using explicit source/destination rectangles, especially for asymmetric image dimensions and fractional zoom levels. It adjusts both image-handle zoom selection and source-rectangle pixel mapping, and adds regression tests to validate consistent output across zoom/size matrices.
Changes:
- Fix
Imagehandle zoom selection to prefer the axis with the larger pixel hint (reducing integer-division rounding bias for asymmetric images). - Fix
GCsource-rectangle mapping to compute X/Y scale factors independently and apply endpoint-subtraction rounding per axis. - Add Win32 JUnit parameterized regression tests for asymmetric images across multiple zoom levels and heights.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java |
Adjusts best-fitting handle zoom computation to avoid wrong zoom selection for asymmetric dimensions. |
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java |
Reworks source-rectangle pixel conversion and bounds tolerance logic for asymmetric scaling/cropping. |
bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/ImagesWin32Tests.java |
Adds parameterized test to verify asymmetric images select the correct handle zoom. |
bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/graphics/GCWin32Tests.java |
Adds parameterized regression test comparing pixel output across drawImage overloads at multiple zoom/height combinations. |
Comment on lines
1267
to
1271
| int errX = srcPixels.x + srcPixels.width - imageHandle.width(); | ||
| int errY = srcPixels.y + srcPixels.height - imageHandle.height(); | ||
| if (errX != 0 || errY != 0) { | ||
| if (errX <= closestZoomOfHandle / 100 && errY <= closestZoomOfHandle / 100) { | ||
| if (errX <= Math.ceil(scaleFactorX) && errY <= Math.ceil(scaleFactorY)) { | ||
| srcPixels.intersect(new Rectangle(0, 0, imageHandle.width(), imageHandle.height())); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GC.drawImagewith explicit source and destination rectangles has several bugs in how it maps the logical source rectangle to the pixel coordinates of the image handle used for rendering. The image handle selection inImagealso picks the wrong zoom level for asymmetric images. This PR fixes both.Important
This change is based on and should be merged after (or together with):
Fixes #3454
May also fix eclipse-gef/gef-classic#1146
Reproduction / How to Test
See #3454
The snippet should show an image rendered equally two times (in particular at 125% monitor zoom).
Before this change
After this change
Problems and fixes
Wrong image handle zoom for asymmetric images (
Image.java)When selecting which zoom-level handle to use, the code estimated the required zoom separately for each axis and then took the maximum. For asymmetric images the smaller axis produces a larger integer-division truncation error, so
Math.maxreliably picks the wrong zoom.Fix: use the zoom estimate from the larger hint dimension, which has more pixels and therefore less relative rounding error.
X and Y axes treated as a single scale factor (
GC.java)When converting the source rectangle from logical points to handle pixels, the old code collapsed the independent per-axis scale factors into one value using
Math.min, then applied it to both axes. For asymmetric images the X and Y factors diverge because each axis is rounded independently when the handle is created.Fix: compute and apply
scaleFactorXandscaleFactorYindependently for each axis, using the same endpoint-subtraction rounding thatImageuses when scaling image data — guaranteeing consistent results between the two.Error tolerance collapses to zero at sub-100% zoom (
GC.java)After computing the pixel source rectangle, the code checks whether it slightly overflows the handle bounds (an expected rounding side-effect) and clamps it within a tolerance. The tolerance was computed as an integer division, which truncates to zero for any zoom below 100%, turning every normal rounding artifact into a spurious
ERROR_INVALID_ARGUMENT.Fix: use
Math.ceil(scaleFactor)instead, which always allows at least 1 pixel of tolerance regardless of zoom level. Additionally, the bounds correction was previously guarded so it was skipped entirely when only one axis had a non-unity scale factor; the guard is corrected to apply whenever either axis is scaled.Tests
A new
@ParameterizedTestinGCWin32Testsverifies that the 3-arg, 5-arg, and 9-argdrawImageoverloads all produce identical pixel output for a 500-wide source image across a 5×9 matrix of zoom levels (100%–200%) and prime image heights (1–19). Prime heights maximise the chance of exposing per-axis rounding divergence.