RFR: 8306320: BufferedImage spec needs clarification w.r.t its implementation of the WritableRenderedImage interface [v2]

[Martin Desruisseaux]

On Wed, 19 Apr 2023 17:15:02 GMT, Phil Race <prr at openjdk.org> wrote:

>> Martin Desruisseaux has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update documentation for adressing comment on pull requests, with two changes to be discussed:
>>   
>>   - The "The default implementation" sentence has not yet been removed, for reason discussed on the pull request.
>>   - The discussion about (0,0) tile indices mentions the relationship with `getTileMinX()` and `getTileMinY()`.
>
> src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 1596:
> 
>> 1594:      * point with (0,0) coordinates since {@code BufferedImage}
>> 1595:      * {@linkplain #getRaster() single tile} is always checked out
>> 1596:      * for writing.
> 
> I think there's some missing punctuation there.
> 
> I'd like to slip in some  additional explanation that (0,0) is always going to be the offset because what else can it be if you only have one tile ?
> The text about "default implementation" in these updated docs might be misinterpreted as there's an alternative implementation .. I don't think there can be .. not in any compatible way.
> All these methods should have been made final. Sadly its too late for that but we need to be clear that this is what you can rely on for a BufferedImage and any subclass that changes it is breaking the contract.
> 
> How about -
> Since a {@code BufferedImage} consists of a single tile, and that tile is always checked out for writing,
> this method, will always return an array of one point.
> Further, in  any case of an image with a single tile, the offset will always be (0,0), that will always be the coordinates of the single returned {@code Point}.

I pushed a new commit which tries to address those comments. The _"default implementation"_ texts have not yet been removed, pending discussion about whether the following scenario is acceptable: A library creates a private `BufferedImage` subclass with `TileObserver` support added. That library returns instances of that subclass only through public methods having `WritableRenderedImage` return type, never exposing the `BufferedImage` type directly. It seems to me that no contract would be broken as long as the user does not cast to `BufferedImage`.

Of course we can not prevent the user to cast to `BufferedImage`, but this issue already exists elsewhere in the library. For example all `Raster.createRaster(…)` static methods may return an instance of `WritableRaster`, when they decided to return a subclass such as `sun.awt.image.ByteInterleavedRaster` optimized for a specific sample model. So we can not have a truly read-only `Raster` through the standard Java API. We already have to tell users _"Don't try to get write access by casting"_.

Regarding tile indices, in the general `RenderedImage` case they do not necessarily start at (0,0). They start at the values given by `getMinTileX()` and `getMinTileY()`. In the particular case of `BufferedImage`, the Javadoc of those two methods said _"This is always zero."_. So the text saying that the offset will always be (0,0) is an indirect constraint. I tried to explain that in the text.

I will create a third commit if the preference is still to remove the _"default implementation"_ text or for other changes.

Regarding the CRS, I do not have the power to create one on https://bugs.openjdk.org/.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13506#discussion_r1172303229