RFR: 8329098: Support "@1x" image naming convention as fallback [v2]

[John Hendrikx]

On Wed, 16 Oct 2024 11:30:26 GMT, Lukasz Kostyra <lkostyra at openjdk.org> wrote:

>> This PR adds a title-mentioned fallback to `ImageStorage.java` and a test for that specific scenario.
>> 
>> In `ImageStorage.loadAll()` we still prefer the Image path that was provided by the user, but if it is missing we will now try to add a `@1x` suffix to Image name and give it one last try.
>> 
>> Added test includes a binary file `checkers at 1x.png`. This was added to differentiate it from the collection of `checker.png` files and their higher scale variants, but it is a direct copy of already existing `checker.png` test resource, just with a different name and suffix. Test should fail without a change in `ImageStorage.java` and pass with it.
>
> Lukasz Kostyra has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review fixes
>   
>   - Change exception message in loadAll() to include original resource
>     path
>   - Add a test case to ImageStorageTest checking if we load a correct
>     resource when both <res> and <res>@1x are present

@mstr2 as you're busy in the `ImageStorage` class as well and know how it works, perhaps you could take a peek at this PR as well.

modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java line 385:

> 383:                         try {
> 384:                             theStream = ImageTools.createInputStream(input);
> 385:                         } catch (IOException ignored) {

It looks like this abstracts away errors like `FileNotFoundException`, and it will do a fallback in all cases, even if it is not a 404 or `FileNotFoundException` -- errors like network problems, permission problems, etc, will also trigger a fallback (likely resulting in the same error).  But it could also be a different error, hiding the original problem.

In Java we have the option to add a 'surpressed' exception.  You could attach it to the 2nd exception, so the original cause of the problem (which could be a permission problem for example) is not lost.

Maybe I'm being too picky :)

modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java line 392:

> 390:                         try {
> 391:                             // last fallback, try to see if the file exists with @1x suffix
> 392:                             String scaled1xName = ImageTools.getScaledImageName(input, 1);

I think this function does not properly handle cases where the input may already have a scale specified.

If I want to load `testimg at 1x.png` then this function will return `testimg at 1x@1x.png`

I'm not sure what is normal here, but I'd think that if a scale was already specified that we shouldn't do any fallback.

-------------

PR Review: https://git.openjdk.org/jfx/pull/1598#pullrequestreview-2372422385
PR Review Comment: https://git.openjdk.org/jfx/pull/1598#discussion_r1803081373
PR Review Comment: https://git.openjdk.org/jfx/pull/1598#discussion_r1803073434