[Win32] Fix image handle zoom selection for asymmetric image dimensions#3455
[Win32] Fix image handle zoom selection for asymmetric image dimensions#3455HeikoKlare wants to merge 1 commit into
Conversation
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>
| } | ||
|
|
||
| private static Stream<Arguments> provideZoomAndHeightCombinations() { | ||
| int[] zooms = { 100, 125, 150, 175, 200 }; |
There was a problem hiding this comment.
Shouldn't zooms < 100 also be considered here?
|
I am currently not sure how to best enhance the zoom calculation. The goal was to choose the axis for zoom calculation with the lower rounding error, but just doing that based on the given hint for the target dimension does not seem to be correct. Though that value is already rounded because point-to-pixel and pixel-to-point conversions, small bounds of the image in one of the dimensions may also influence the rounding error. In addition, the change optimizes precision of the calculated zoom while ignoring the dominating axis in case the image is scaled asymmetrically (e.g., by factor 5 in y direction and by factor 1 in x direction), where it is most reasonable to better use an image version that is too large rather than using one that is too small. This was intended to be a preparation step for: This is because for heights between 1 and 3 pixels and a much larger widths, the dimensions of the handle being returned here are not as expected, due to large rounding errors with such a small size. However, images of 1 to 3 pixel in height do not seem to be that relevant use cases (as you cannot see anything relevant in them anyway, not matter if a higher or lower resolution version is used) that it justifies the drawback mentioned about and the risk of other unintended changes in behavior / regressions. For the mentioned reasons, I close this PR for now. Thanks to @r-mennig for challenging the change, leading to this conclusion. |
2 participants
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 #3454