RFR: 8323108: BufferedImage.setData(Raster) should not cast float and double values to integers [v3]
Martin Desruisseaux
duke at openjdk.org
Sun Jan 21 12:20:57 UTC 2024
> The `BufferedImage` Javadoc does not mention any constraint about the data type. In practice, `BufferedImage` with floating point values can be rendered by Java2D as well as integers, provided that a compatible `ColorModel` was supplied at construction time. However calls to `setData(Raster)` unexpectedly cast floating point values to integers. For example sample value 0.8 become 0. This is demonstrated by the `SetData` test case in this pull request.
>
> An easy fix, which is proposed in this pull request, is to replace the whole `BufferedImage.setData(Raster)` method body by a simple call to `WritableRaster.setRect(Raster)`, which handles all `DataBuffer` types correctly. The method contracts documented in their respective Javadoc are compatible. Furthermore an examination of their source code shows that they are equivalent, except for the differences noted in the _Behavioural changes_ section below.
>
> # Source code comparison
> For demonstrating that delegating to `WritableRaster.setRect(Raster)` would be equivalent to the current code, one can copy the method body and apply the following changes:
>
> * Remove `dx` parameter, replaced by 0.
> * Remove `dy` parameter, replaced by 0.
> * Rename `srcRaster` parameter as `r` (like in `BufferedImage`).
> * Rename `startY` variable as `i` (like in `BufferedImage`).
> * Rename `srcOffX` variable as `startX` (like in `BufferedImage`).
> * Rename `srcOffY` variable as `startY` (like in `BufferedImage`).
> * Replace `this.minX` by 0 and simplify.
> * Replace `this.minY` by 0 and simplify.
> * Remove the `switch` statement, keeping only the `TYPE_INT` case.
>
> Then compare with `BufferedImage.setData(Raster)`. The modified block of code from `WritableRaster` is:
>
>
> int width = r.getWidth();
> int height = r.getHeight();
> int startX = r.getMinX();
> int startY = r.getMinY();
>
> We can see that above code is identical to `BufferedImage`. The next modified block of code from `WritableRaster`:
>
>
> int dstOffX = startX;
> int dstOffY = startY;
>
> // Clip to this raster
> if (dstOffX < 0) {
> width += dstOffX;
> startX -= dstOffX; // = 0 because dstOffX == startX
> dstOffX = 0;
> }
> if (dstOffY < 0) {
> height += dstOffY;
> startY -= dstOffY; // = 0 because dstOffY == startY
> dstOffY = 0;
> }
> if (dstOffX+width > this.width) {
> width = this.width - dstOffX;
> }
> if (dstOffY+height > this.height) {
> height = this.height - dstOffY;
> }
> if (width <= 0 || height <= 0) {
> return;
> }
>
>
> Above is equivalent to the following code from `BufferedImage`, ex...
Martin Desruisseaux 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 four additional commits since the last revision:
- Merge branch 'master' into fix/set-data
- Update copyright years.
- Use Rectangle.intersection(Rectangle) in WritableRaster.setRect(int, int, Raster) for safety against integer underflow/overflow.
That commit reproduces the safety that existed in the BufferedImage.setData(Raster) method which has been replaced in previous commit.
- `BufferedImage.setData(Raster)` should not cast float and double values to integers.
The easiest fix is to delegate to `WritableRaster.setRect(Raster)`.
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/13797/files
- new: https://git.openjdk.org/jdk/pull/13797/files/5d69f631..fd0b8045
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=13797&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=13797&range=01-02
Stats: 1431853 lines in 13587 files changed: 555220 ins; 698691 del; 177942 mod
Patch: https://git.openjdk.org/jdk/pull/13797.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/13797/head:pull/13797
PR: https://git.openjdk.org/jdk/pull/13797
More information about the client-libs-dev
mailing list