On Sun, 5 Oct 2025 20:33:26 GMT, Phil Race <[email protected]> wrote:
> Specifying the behaviour of BufferedImage constructors for invalid dimensions
> is long overdue.
>
> The behaviour for image types and sizes <= 0 is unchanged by this PR.
> Also in many cases the behaviour for sizes that are too large is also
> unchanged.
> In some cases, the behaviour is changed from "accidental"
> NegativeArraySizeException to a consistent IllegalArgumentException.
>
> In no case is anything changed that would affect the possibility to construct
> a BufferedImage.
>
> A test is provided to ensure the behaviour.
>
> A CSR is provided too : https://bugs.openjdk.org/browse/JDK-8369155
Changes requested by aivanov (Reviewer).
src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 292:
> 290: * image types. The {@code ColorSpace} for the image is the
> 291: * default sRGB space.
> 292: * {@code BufferedImage} is a type that supports only one tile.
Suggestion:
* default sRGB space.
* <p>
* {@code BufferedImage} is a type that supports only one tile.
To start a new paragraph? I think it would improve readability.
src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 297:
> 295: * Java primitive arrays so the number of samples that can be
> 296: * stored are limited by the maximum size of a Java array.
> 297: * This is at most {@code Integer.MAX_VALUE}.
Shouldn't the sentence "This is at most…", be linked to the previous sentence
with a comma. *This* refers to *the maximum size of a Java array*, it could be
not as clear when these closely related sentences are split into two separate
sentences.
src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 299:
> 297: * This is at most {@code Integer.MAX_VALUE}.
> 298: * The number of samples per-pixel for an {@code imageType} affect
> 299: * the maximum. For example if an image format uses bytes to store
Suggestion:
* the maximum. For example, if an image format uses bytes to store
A comma after “for example”.
src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 303:
> 301: * it will only be able to hold one fourth as many pixels as an image
> 302: * that uses an int to store all four samples.
> 303: * For example {@code TYPE_4BYTE_ABGR} may use 4 bytes to store a
> pixel
Suggestion:
* For example, {@code TYPE_4BYTE_ABGR} may use 4 bytes to store a pixel
src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 307:
> 305: * So the maximum number of pixels in a {@code BufferedImage} is
> 306: * format dependent.
> 307: * @param width width of the created image
Suggestion:
* format dependent.
*
* @param width width of the created image
I'd add a blank line here to visually separate the description from the
parameter list.
src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 343:
> 341: if (lsz > Integer.MAX_VALUE) {
> 342: throw new IllegalArgumentException(
> 343: "width " + width + " height " + height + " overflow
> int");
Suggestion:
"width " + width + "* height " + height + " overflow int");
Is the asterisk to denote multiplication missing?
src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 362:
> 360: if ((spp != 1) && (lsz * spp > Integer.MAX_VALUE)) {
> 361: throw new IllegalArgumentException(
> 362: "width " + width + " height " + height + " * " +
Suggestion:
"width " + width + "* height " + height + " * " +
Is the asterisk missing?
src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 672:
> 670: * {@code raster} is incompatible with {@code cm}
> 671: * @throws IllegalArgumentException if
> 672: * {@code raster} {@code minX} or {@code minY} is not zero
Suggestion:
* {@code raster}, {@code minX} or {@code minY} is not zero
Is the comma missing?
src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 677:
> 675: * @see WritableRaster
> 676: */
> 677:
Remove this blank too? So that the javadoc ends just above a constructor
declaration as in other cases?
-------------
PR Review: https://git.openjdk.org/jdk/pull/27640#pullrequestreview-3511751915
PR Review Comment: https://git.openjdk.org/jdk/pull/27640#discussion_r2565650069
PR Review Comment: https://git.openjdk.org/jdk/pull/27640#discussion_r2565663429
PR Review Comment: https://git.openjdk.org/jdk/pull/27640#discussion_r2565667499
PR Review Comment: https://git.openjdk.org/jdk/pull/27640#discussion_r2565673736
PR Review Comment: https://git.openjdk.org/jdk/pull/27640#discussion_r2565684666
PR Review Comment: https://git.openjdk.org/jdk/pull/27640#discussion_r2565710115
PR Review Comment: https://git.openjdk.org/jdk/pull/27640#discussion_r2565716561
PR Review Comment: https://git.openjdk.org/jdk/pull/27640#discussion_r2565722017
PR Review Comment: https://git.openjdk.org/jdk/pull/27640#discussion_r2565727927
