RFR: 8351108: ImageIO.write(..) fails with exception when writing JPEG with IndexColorModel [v4]
Alexey Ivanov
aivanov at openjdk.org
Mon Mar 10 19:37:09 UTC 2025
On Mon, 10 Mar 2025 18:22:49 GMT, Jeremy Wood <duke at openjdk.org> wrote:
>> Previously ImageTypeSpecifier treated all TYPE_BYTE_INDEXED as if they were opaque.
>>
>> In this ticket's case: an ImageWriter received the wrong ImageTypeSpecifier and mistakenly indicated it *could* support a BufferedImage. But when the actual BufferedImage arrived (which was translucent), the ImageWriter threw an exception.
>>
>> Instead:
>> Now the ImageTypeSpecifier accurately describes the given BufferedImage.
>
> Jeremy Wood has updated the pull request incrementally with one additional commit since the last revision:
>
> 8351108: changing Error to Exception, adding javadoc
>
> This is partly in response to:
> https://github.com/openjdk/jdk/pull/23884#discussion_r1987733487
Marked as reviewed by aivanov (Reviewer).
test/jdk/javax/imageio/plugins/jpeg/JpegWriterWriteNonOpaqueIndexColorModelTest.java line 24:
> 22: */
> 23:
> 24: /**
Suggestion:
/*
I prefer regular block comments over javadoc-style comments for jtreg tags. Otherwise, IDE shows a warning for _dangling javadoc comment_, or warns about bad javadoc tags.
I don't insist here.
test/jdk/javax/imageio/plugins/jpeg/JpegWriterWriteNonOpaqueIndexColorModelTest.java line 55:
> 53: * @param name the name of the Transparency constant
> 54: * @param expectedWriteReturnValue the expected return value of <code>ImageIO.write(..)</code>
> 55: * @return true if <code>ImageIO.write(..) == expectedWriteReturnValue</code>, or false otherwise.
Using `{@code ...}` is the preferred way to mark-up code fragments.
Perhaps, a javadoc is too much… The method isn't that long, and it's rather self-documenting.
test/jdk/javax/imageio/plugins/jpeg/JpegWriterWriteNonOpaqueIndexColorModelTest.java line 81:
> 79: }
> 80: }
> 81: IndexColorModel indexColorModel = new IndexColorModel(8, 256, gray, gray, gray, alpha);
Suggestion:
}
IndexColorModel indexColorModel = new IndexColorModel(8, 256, gray, gray, gray, alpha);
I think a blank line here will visually separate the block of preparing the arrays for `IndexColorModel` from their usage.
Creating an instance of `IndexColorModel` marks the start of the *real test*.
-------------
PR Review: https://git.openjdk.org/jdk/pull/23884#pullrequestreview-2671974299
PR Review Comment: https://git.openjdk.org/jdk/pull/23884#discussion_r1987882658
PR Review Comment: https://git.openjdk.org/jdk/pull/23884#discussion_r1987896914
PR Review Comment: https://git.openjdk.org/jdk/pull/23884#discussion_r1987901996
More information about the client-libs-dev
mailing list