8369129: Raster createPackedRaster methods specification clean up

[prrace]

Updates the specification of Raster.createPackedRaster(..) methods and also the implementation of these to explicitly check the specified conditions rather than relying on them being passed up from some other API.

Also one each of the createInterleavedRaster(..) and createBandedRaster(..) methods have an update

createInterleavedRaster(...) to address that even if WxH does not exceed Integer.MAX_VALUE, the storage needed still might.

createBandedRaster(..) to address

• a not explicitly specified or tested case that WxH must not overflow
• a not explicitly tested NPE case if bandMasks is null. This was relying on a de-reference and the actual check for it being null would have thrown the wrong exception type (AIOBE) - meaning not the one NPE actually specified and thrown.
  There's an unfortunate oddity with this that of bankIndices is null it specifies and throws AIOBE, but I didn't think it worth changing either of these to make them consistent.

The existing CreateRasterExceptionTest.java is updated to verify all these assertions.
In almost all cases the behaviour is unchanged, it is just now properly specified and verified up front.

There are only 2 sub-tests which would fail on JDK 25
The test can also be run (and pass) there without this fix because it explicitly handles those 2 cases which are reasonable behavioral changes

• One for the createInterleavedRaster() update where the unchecked case could end up causing a NegativeArraySizeException. This seems like a merited change.
• One for an unlikely case where if an app specified 0 app to createPackedRaster, divide by zero would happen.

The CSR is now ready for review https://bugs.openjdk.org/browse/JDK-8369131

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change requires CSR request JDK-8369131 to be approved
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issues

• JDK-8369129: Raster createPackedRaster methods specification clean up (Bug - P4)
• JDK-8369131: Raster.createPackedRaster methods specification clean up (CSR)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27627/head:pull/27627
$ git checkout pull/27627

Update a local copy of the PR:
$ git checkout pull/27627
$ git pull https://git.openjdk.org/jdk.git pull/27627/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27627

View PR using the GUI difftool:
$ git pr show -t 27627

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27627.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back prr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@prrace The following label will be automatically applied to this pull request:

• client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 03: Full - Incremental (b95c4ad0)
• 02: Full - Incremental (f00d5bc6)
• 01: Full - Incremental (7ba88492)
• 00: Full (3408acfa)

[mrserb]

Is it still a valid point even for a single-line image, where scanlineStride would not be used?

[prrace]

Yes.

[mrserb]

Isn't it strange that in this method we reject "empty" images by requiring w and h to be greater than 0, but at the same time accept 0 as scanlineStride and pixelStride? This may result in empty image as well (size == 0), I think it will be rejected later but still should we check it here as well?

[prrace]

We seem to have a number of APIs that allow the strides to be zero.
I find them odd, but I don't see how they can cause an empty image and they need careful consideration before changing. It seems very deliberate.
Here's a sampling of other cases (there are likely more, I searched very briefly and crudely)

 * @throws IllegalArgumentException if {@code scanlineStride} is less than 0
public BandedSampleModel(int dataType,
                         int w, int h,
                         int scanlineStride,
                         int[] bankIndices,
                         int[] bandOffsets) 

=====

 * @throws IllegalArgumentException if {@code pixelStride} is less than 0
 * @throws IllegalArgumentException if {@code scanlineStride} is less than 0

public ComponentSampleModel(int dataType,
int w, int h,
int pixelStride,
int scanlineStride,
int[] bandOffsets) {

=====
* @throws IllegalArgumentException if {@code pixelStride} is less than 0
* @throws IllegalArgumentException if {@code scanlineStride} is less than 0
public ComponentSampleModel(int dataType,
int w, int h,
int pixelStride,
int scanlineStride,
int[] bankIndices,
int[] bandOffsets) {

[prrace]

PS, also negative strides are something that we've been asked to support too - in the distant past.
But I don't have any plans to do it now.