Fix GC#drawImage + ImageGcDrawer for Cropping and Scaling #2913
Conversation
| ImageHandle imageHandle = lastRequestedHandle.getOrCreateImageHandleAtClosestSizeAtZoom(scaledZoom); | ||
| handleAtSizeConsumer.accept(imageHandle); | ||
| } | ||
| void executeOnImageHandleAtSizeOrZoom(BiConsumer<ImageHandle, Point> handleAtSizeConsumer, int targetWidth, int targetHeight, int zoom) { |
There was a problem hiding this comment.
is this used anywhere?
| return Optional.empty(); | ||
| } | ||
|
|
||
| public ImageHandle getOrCreateImageHandleAtClosestSizeAtZoom(int scaledZoom) { |
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) -> { |
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(); |
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
al-noori
force-pushed
the
al-noori/FixSizingWithIGCD
branch
from
53c83c8 to
804f78d
Compare
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 width/height of the full image scaled by the scaledImageZoom. This is a space on where scaled src coordinates/width/height lie. A callback then creates a new handle for the height/width and respects 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.
al-noori
force-pushed
the
al-noori/FixSizingWithIGCD
branch
from
804f78d to
6ae439d
Compare
|
@arunjose696 i edited the PR such that the Regarding that hack: it seems to me that i does not make sense. The hack is motivated by potential rounding errors, but without further investigation, i assume that passing correct source coordinates/width/height which lie in the space of the full image, the statement |
2 participants
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 width/height of the full image scaled by the scaledImageZoom. This
is a space on where scaled
srccoordinates/width/height lie. A callbackthen creates a new handle for the height/width and respects 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).