Fix GC#drawImage + ImageGcDrawer for Cropping and Scaling #2913
+54
−24
Conversation
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
The GC#drawImage method takes (image, srcX, srcY, srcWidth, srcHeight, destX, destY, destWidth, destHeight) as arguments and crops and scales from the source region to the destination region. Passing an image drawn via ImageGCDrawer led to the following issue: The image handle from the subcall is resolved using the monitor zoom (data.nativeZoom) and the calculated scaledImageZoom (gcZoom * scaleFactor). This handle corresponds to an ImageData initialized at scaledImageZoom, whereas the drawings of the second GC are performed using the monitor zoom, subject to the auto-scale property. This mismatch results in unaligned sizing of drawings. For example, a 200% monitor zoom combined with a scale factor of 0.5 produces a scaledImageZoom of 100%. As a result, the ImageData is initialized at 100%, while drawing occurs at 200%. This exact case is demonstrated in vi-eclipse/Eclipse-Platform#554 . Furthermore, the calculation of scaledImageZoom uses fallback logic that only allows 100% and 200% as possible outcomes, which is clearly unintended in this context. The fix delegates resolving the correct handle to the Image class by passing the calculated scaledImageZoom. A callback then creates a new handle for this zoom, respecting the auto-scale property. If the returned handle matches the full image scaled to the requested scaledImageZoom in width and height, the source region coordinates/width/height are passed directly in pixels at that zoom. Otherwise, the internal zoom factor is derived from the returned handle’s width relative to the full image, and the source region coordinates are converted to pixel values using this internal zoom.
Contributor
arunjose696
reviewed
Dec 31, 2025
| ImageHandle imageHandle = lastRequestedHandle.getOrCreateImageHandleAtClosestSizeAtZoom(scaledZoom); | ||
| handleAtSizeConsumer.accept(imageHandle); | ||
| } | ||
| void executeOnImageHandleAtSizeOrZoom(BiConsumer<ImageHandle, Point> handleAtSizeConsumer, int targetWidth, int targetHeight, int zoom) { |
Contributor
There was a problem hiding this comment.
is this used anywhere?
| return Optional.empty(); | ||
| } | ||
|
|
||
| public ImageHandle getOrCreateImageHandleAtClosestSizeAtZoom(int scaledZoom) { |
Contributor
There was a problem hiding this comment.
shouldnt this be private
|
|
||
| private void drawImage(Image image, int srcX, int srcY, int srcWidth, int srcHeight, int destX, int destY, | ||
| int destWidth, int destHeight, int imageZoom, int scaledImageZoom) { | ||
|
|
| Rectangle fullImageBounds = image.getBounds(); | ||
| Rectangle targetSrc = Win32DPIUtils.pointToPixel(drawable, fullImageBounds, scaledImageZoom); | ||
| Rectangle startSrc = new Rectangle(srcX, srcY, srcWidth, srcHeight); | ||
| image.executeOnImageHandleAtBestFittingSizeAtZoom((tempHandle) -> { |
Contributor
There was a problem hiding this comment.
IMO it is better to have an additional override for Image#executeOnImageHandleAtBestFittingSize() which takes source dimensions in addition to destination dimensions so the GC#calculateZoomForImage() can be removed and the calculation of zoom becomes the responsibility of the Image class.
| } | ||
| } | ||
| drawImage(image, src.x, src.y, src.width, src.height, dest.x, dest.y, dest.width, dest.height, false, image.getHandle(scaledImageZoom, data.nativeZoom)); | ||
| Rectangle fullImageBounds = image.getBounds(); |
Contributor
There was a problem hiding this comment.
Can see image.getBounds is called above in case the zoom!=100 can you see if the hack still makes sense, It would make sense to consider if we can stream line these calls
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The GC#drawImage method takes
(image, srcX, srcY, srcWidth, srcHeight, destX, destY, destWidth, destHeight) as arguments and crops and scales from the source region to the destination region.
Passing an image drawn via ImageGCDrawer led to the following issue:
The image handle from the subcall is resolved using the monitor zoom (data.nativeZoom) and the calculated scaledImageZoom (gcZoom * scaleFactor). This handle corresponds to an ImageData initialized at scaledImageZoom, whereas the drawings of the second GC are performed using the monitor zoom, subject to the auto-scale property.
This mismatch results in unaligned sizing of drawings. For example, a 200% monitor zoom combined with a scale factor of 0.5 produces a scaledImageZoom of 100%. As a result, the ImageData is initialized at 100%, while drawing occurs at 200%.
Furthermore, the calculation of scaledImageZoom uses fallback logic that only allows 100% and 200% as possible outcomes, which is clearly unintended in this context.
The fix delegates resolving the correct handle to the Image class by passing the calculated scaledImageZoom. A callback then creates a new handle for this zoom, respecting the auto-scale property.
If the returned handle matches the full image scaled to the requested scaledImageZoom in width and height, the source region coordinates/width/height are passed directly in pixels at that zoom. Otherwise, the internal zoom factor is derived from the returned handle’s width relative to the full image, and the source region coordinates are converted to pixel values using this internal zoom.
Note: Snippet 10 used to demonstrate that method. Although the sizing issue is fixed, the method is replaced by GC#drawImage(destX, destY, destWidth, destHeight) as the use case here is only scaling and not cropping. Currently, a new snippet is in progress which will be developed to address the cropping + scaling scenario (see #2912 as a first version without drawings via the ImageGcDrawer).