Ensure DrawImage deals with scenario ImageData not linearly scaled

[arunjose696]

This change makes sure DrawImage handles the scenario where ImageData may not be linearly scaled across zooms.

[github-actions]

Test Results

   545 files  ±0     545 suites  ±0   31m 36s ⏱️ +17s
 4 406 tests +4   4 388 ✅ +4   18 💤 ±0  0 ❌ ±0 
16 736 runs  +4  16 596 ✅ +4  140 💤 ±0  0 ❌ ±0 

Results for commit 9b0b4a0. ± Comparison against base commit f92b752.

♻️ This comment has been updated with latest results.

[HeikoKlare]

I like the overall approach and I think we could achieve to properly deal with the situation of non-linearly scaled image data. But I think the implementation would need to be a bit more sophisticated. With this approach, since the corrected scaledImageZoom may be an arbitrary value, we would potentially start loading images at arbitrary zooms and thus create more and more unwanted handles. In addition, there is no guarantee that the image data that the image yields for such a zoom have fitting width/height as it depends on which original image data (such as the 100%, 150% or 200% data for PNGs) are used and scaled.
I think what we might actually need to adapt is the srcWidth and srcHeight as they must not be treated as points with an ordinary point-to-pixel conversion based on the zoom if the zoom is not appropriate. However, if you have an image with non-linear image data, you do not even know to which the original point-based srcWidth and srcHeight should fit. We currently assume that it's the 100% version of the image, but it may also be the case that someone creats an ImageDataProvider that always yields image data at the same zoom, which could, for example, also be the current device zoom. If that is not 100%, there might be unexpected results.

I have just tested again the snippet in vi-eclipse/Eclipse-Platform#313 that led to an exception before and found that it now (silently) works. So I wonder whether we should focus on improving the API (adding a note to ImageDataProvider that implementations must return linearly scaled data) and rely on the just added strict check and adaptations of consumers (if necessary) instead of making the logic here more complex.

What do you think, @akoch-yatta @arunjose696?

[arunjose696]

I wonder whether we should focus on improving the API (adding a note to ImageDataProvider that implementations must return linearly scaled data) and rely on the just added strict check and adaptations of consumers (if necessary) instead of making the logic here more complex.

I think the better approach would be to document clearly in ImageDataProvider that implementations must return linearly scaled data. Adding complexity to drawImage to "fix" bad input risks hiding these issues rather than solving them at the actual ( a incorrect ImageDataProvider implementation).

[HeikoKlare]

Fully agree. Just for the sake of completeness: the risk is of course that implementations of ImageDataProvider usually have been there for years already, so the contract improvement may stay unnoticed and the implementations will stay somehow incompatible. Still, I think that's the best we can do.
So, concrete proposal: We drop this PR and instead improve the contract of ImageDataProvider. We write a short news as a means to make aware of the potential issue and the contract adaptatino (as actually the assumption that image data is linearly scaled was always there but violations were silently accepted in a more or less resilient way).
@akoch-yatta would then also be good to have your final opinion when you are back before proceeding with that.

[akoch-yatta]

I agree. Everything else seem like a workaround, that would only cover up the overall issue, that this contract was (at least implicitly) there from the introduction of HiDPI support, but not enforced.

[arunjose696]

Closing this PR as discussed