RFR: 8306320: Add @implSpec for explaining BufferedImage default behavior

Phil Race prr at openjdk.org
Wed Apr 19 17:18:51 UTC 2023


On Tue, 18 Apr 2023 09:30:07 GMT, Martin Desruisseaux <duke at openjdk.org> wrote:

> `BufferedImage` implements the `WritableRenderedImage` interface. But the Javadoc is copied from `WritableRenderedImage`, while `BufferedImage` does something quite different. In particular, `TileObserver` are ignored. This pull request add `@implSlec` for explaining the default behaviour.
> 
> This commit has one specification change in `isTileWritable`: the exception type is changed from `ArrayIndexOutOfBoundsException` to `IllegalArgumentException` for matching the implementation. The logical conditions is also corrected.
> 
> This commit contains a trivial code change: `new Point(0,0)` is replaced by `new Point()` for saving a few byte codes.

Changes requested by prr (Reviewer).

src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 1544:

> 1542:    * it receives multiple notifications.
> 1543:    *
> 1544:    * @implSpec

I don't think this needs to be implSpec. I think we can say : 
This method ignores its parameters and does nothing, since {@code BufferedImage} is always checked out for writing and cannot be made read-only, so there can never be events to dispatch.

src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 1560:

> 1558:    * notifications, it is now registered for one fewer notification.
> 1559:    *
> 1560:    * @implSpec

ditto

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}.

src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 1631:

> 1629:    * {@linkplain #getRaster() single tile} without checking
> 1630:    * the passed values. No listeners are notified since the
> 1631:    * returned tile is always checked out for writing.

similar to above

src/java.desktop/share/classes/java/awt/image/BufferedImage.java line 1655:

> 1653:    * without checking the passed values. No listeners are notified
> 1654:    * since the {@linkplain #getRaster() single tile} is always
> 1655:    * checked out for writing.

similar to above

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

PR Review: https://git.openjdk.org/jdk/pull/13506#pullrequestreview-1392518605
PR Review Comment: https://git.openjdk.org/jdk/pull/13506#discussion_r1171625687
PR Review Comment: https://git.openjdk.org/jdk/pull/13506#discussion_r1171626413
PR Review Comment: https://git.openjdk.org/jdk/pull/13506#discussion_r1171636385
PR Review Comment: https://git.openjdk.org/jdk/pull/13506#discussion_r1171636760
PR Review Comment: https://git.openjdk.org/jdk/pull/13506#discussion_r1171636811



More information about the client-libs-dev mailing list