RFR: 8329098: Support "@1x" image naming convention as fallback [v2]
Michael Strauß
mstrauss at openjdk.org
Wed Oct 16 13:36:18 UTC 2024
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
modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java line 395:
> 393: theStream = ImageTools.createInputStream(scaled1xName);
> 394: } catch (IOException e) {
> 395: throw new IOException(e.getMessage() + " (original resource path: " + input + ")");
I don't like this exception message, since it will now say something like
> yourImage at 1x.jpeg was not found (original resource path: yourImage.jpeg)
That's confusing, because in all likelihood, I didn't specify that file name in my application code. What is the significance of mentioning `@1x.jpeg` in the exception message? We don't mention any of the other scale factors.
I'd rather just call `ImageTools.createInputStream(input)` again, and let the resulting exception bubble up.
We should _only_ mention the scale factor in the exception message if the scale factor was explicitly specified by the application developer.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1598#discussion_r1803114780
More information about the openjfx-dev
mailing list