On Tue, 29 Oct 2024 10:26:26 GMT, Jayathirth D V <j...@openjdk.org> wrote:
>> Michael Strauß has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 21 additional >> commits since the last revision: >> >> - Merge branch 'master' into feature/ximageloader >> - Merge branch 'master' into feature/ximageloader >> - review changes >> - scanline stride always measured in bytes >> - remove unused code >> - fix line endings >> - fix line endings >> - add support for palette-based image formats >> - review changes >> - review changes >> - ... and 11 more: https://git.openjdk.org/jfx/compare/81d24aa6...9dba94c1 > > modules/javafx.graphics/src/main/java/com/sun/javafx/iio/ImageStorage.java > line 342: > >> 340: >> 341: if (loader != null) { >> 342: images = loadAll(loader, width, height, >> preserveAspectRatio, pixelScale, 1, smooth); > > If this is constant for fixed-density images we should use variable/macro > instead of literal value 1 Or add a comment mentioning why we are passing > value 1. I added a comment. > modules/javafx.graphics/src/main/java/com/sun/javafx/iio/bmp/BMPImageLoaderFactory.java > line 496: > >> 494: >> 495: int[] outWH = ImageTools.computeDimensions( >> 496: bih.biWidth, hght, (int)(w * imagePixelScale), (int)(h * >> imagePixelScale), preserveAspectRatio); > > I see possibility of these calculations overflowing. `ImageTools.computeDimensions()` will not allow the dimensions to be less than 0. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1593#discussion_r1823193097 PR Review Comment: https://git.openjdk.org/jfx/pull/1593#discussion_r1823190384
