RFR: 8329098: Support "@1x" image naming convention as fallback
Lukasz Kostyra
lkostyra at openjdk.org
Wed Oct 16 10:11:17 UTC 2024
On Mon, 14 Oct 2024 23:48:32 GMT, Kevin Rushforth <kcr 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.
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java line 392:
>
>> 390: // last fallback, try to see if the file exists with @1x suffix
>> 391: String scaled1xName = ImageTools.getScaledImageName(input, 1);
>> 392: theStream = ImageTools.createInputStream(scaled1xName);
>
> The fallback code looks good with one comment. Because you catch and ignore the previous exception and let this one propagate, if neither the original image name nor any `@1x`, `@2x`, ... name can be found, the exception thrown will no longer have the original name in the exception message, but will instead have the `@1x` name.
>
> You can see this if you run with `-Dprism.verbose=true` and try to load a non-existent file or URL.
>
> Maybe catch the exception on line 386 and then rethrow that so the error message is more understandable (alternatively, wrap the exception in a message that has the original input String as part of its message)?
Capturing the earlier message and rethrowing it uses the original stack trace:
java.io.FileNotFoundException: C:\Users\Lukasz\dev\code\jfx\rt\modules\javafx.graphics\build\resources\test\test\com\sun\javafx\iio\checkers.png (The system cannot find the file specified)
at java.base/java.io.FileInputStream.open0(Native Method)
at java.base/java.io.FileInputStream.open(FileInputStream.java:213)
...
at javafx.graphics at 24-internal/com.sun.javafx.iio.ImageStorage.loadAll(ImageStorage.java:385)
I figured wrapping it with an additional message mentioning original path is a better solution, that way the stack trace is not misleading.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1598#discussion_r1802796745
More information about the openjfx-dev
mailing list