On Thu, 15 May 2025 11:27:21 GMT, Lukasz Kostyra <lkost...@openjdk.org> wrote:
> This follow-up change finishes the earlier changes to
> `ImageStorage.loadAll()` and adds support for loading specific scale
> requested in the input.
>
> `loadAll()` will now first check if the input path ends with a scaling level
> specified, and if that is the case it will attempt creating a Stream. If
> requested resource does not exist it will throw an Exception, skipping the
> rest of the load process. If the resource does _not_ have a scaled name in
> its path, it will continue loading as normal - looking for all scale levels,
> trying to load the main resource and falling back to trying to load "@1x"
> variant.
>
> Added tests to check the new `ImageTools.hasScaledName()` method + new
> behavior.
Changes requested by jhendrikx (Reviewer).
modules/javafx.graphics/src/main/java/com/sun/javafx/iio/common/ImageTools.java
line 166:
> 164: return true;
> 165: }
> 166:
1. Is looking for a slash going to be compatible on all platforms? Where is
the path string coming from?
2. Catching `NumberFormatException` to "check" if something is a number is bad
form
3. It will allow `@0x` and `@-1x` etc...
4. Consider using a regular expression, it is much more concise and intended
for this kind of matching
Here's a regular expression for this:
Pattern SCALED_PATTERN = Pattern.compile(".*@[1-9][0-9]?x(\.[^\.]+)?");
The above will match any path that ends with `@` followed by a number from 1 to
99, followed by an `x`, optionally followed by an extension that does not
contain a dot. No need to check for slashes.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1809#pullrequestreview-2844864600
PR Review Comment: https://git.openjdk.org/jfx/pull/1809#discussion_r2091889766
