On Wed, 16 Oct 2024 13:08:57 GMT, John Hendrikx <[email protected]> wrote:

>> 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 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 `[email protected]` then this function will return 
> `testimg@[email protected]`
> 
> 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.

This technically translates to the for-loop higher up that tries to fetch other 
scaled versions of an image - if you want to explicitly load `[email protected]` it 
will look for `foo@[email protected]` etc.

Should we just assume that with `@Nx` provided in the name we will only load 
that one specific scale?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1598#discussion_r1804587885

Reply via email to