eclipse-platform / eclipse.platform.ui

Eclipse Platform
https://projects.eclipse.org/projects/eclipse.platform
Eclipse Public License 2.0
71 stars 155 forks source link

jface High-DPI: FileImageDescriptor's autoscaling rounding different from that of DPIUtil#autoScaleImageData #378

Open Dani-Hub opened 1 year ago

Dani-Hub commented 1 year ago

Within the getxPath method FileImageDescriptor.java#L176 during zoom scaling, a truncation of double to int is performed, whereas SWT's DPIUtil.java#L228 uses Math.round instead.

This can lead to different results when the zoom value is not a multiple of two (for example for 150%): Consider a file name "Hula-hup-16x57.png" where we compute the 150% getxPath value, we get with the current truncation rounding the result

"Hula-hup-24x85.png"

while when using the DPIUtil#autoScaleImageData approach we would get the result

"Hula-hup-24x86.png"

I suggest that for consistency reasons the same approach should be used in both places. Since the SWT approach seems to be more fundamental, I suggest to change FileImageDescriptor.java#L176 to use the same Math.round approach.

If that change suggestion would be considered controversial because it could in theory change the outcome for existing clients it would be possible to implement a more complicated (but backward-compatible) approach where we split the getxPath algorithm in (potentially) two steps, where the first step uses the new calculation form while the second step uses the old calculation form (If that would lead to a different result).

I would like to get a preferred direction before I'm providing a corresponding PULL request.

laeubi commented 1 year ago

@Dani-Hub following SWT is fine. I think currently most often only "even" values are used, so if it works for the usual 16x16 I think it is fine.

Dani-Hub commented 1 year ago

@Dani-Hub following SWT is fine. I think currently most often only "even" values are used, so if it works for the usual 16x16 I think it is fine.

Thanks for the feedback. I will now wait for the review of PULL request #375, because that touches the same code area.