On Mon, 10 Mar 2025 18:22:49 GMT, Jeremy Wood <d...@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
