Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect source subsampling for bmp #694

Closed
sghpjuikit opened this issue Aug 14, 2022 · 6 comments
Closed

Incorrect source subsampling for bmp #694

sghpjuikit opened this issue Aug 14, 2022 · 6 comments

Comments

@sghpjuikit
Copy link

sghpjuikit commented Aug 14, 2022

Describe the bug
IIOParam.setSourceSubsampling(2, 2, 0, 0) causes incorrect reading of image if sourceXSubsampling or sourceXSubsampling is not 1. Image is rendered twice, scaled and overlapped. Happens only for bmp images. I suspect a typo in x and y coordinates somewhere in the algorithm.

Version information

  1. The version of the TwelveMonkeys ImageIO library in use.
    3.8.2

  2. The exact output of java --version (or java -version for older Java releases).
    For example:

IBM Semeru Runtime Open Edition 17.0.3.0 (build 17.0.3+7)
Eclipse OpenJ9 VM 17.0.3.0 (build openj9-0.32.0, JRE 17 Windows 10 amd64-64-Bit Compressed References 20220422_155 (JIT enabled, AOT enabled)
OpenJ9   - 9a84ec34e
OMR      - ab24b6666
JCL      - dc07fd49b92 based on jdk-17.0.3+7)
  1. Extra information about OS version, server version, standalone program or web application packaging, executable wrapper, etc. Please state exact version numbers where applicable.
    Win 10, JDK17 IBM Semeru

To Reproduce
Steps to reproduce the behavior:

  1. Read bmp image with source subsampling > 1

Expected behavior
Unscaled uncropped image

Example code

val stream = ImageIO.createImageInputStream(File("path to image"))
val reader = ImageIO.getImageReaders(stream).asSequence().first() 
reader.input = stream
reader.read(0, ImageReadParam().apply { setSourceSubsampling(2, 2, 0, 0) })

On my system, the reader's class is com.twelvemonkeys.imageio.plugins.bmp.BMPImageReader

Sample file(s)
I'll upload image if necessary. It seems this concerns every bmp image.

@haraldk
Copy link
Owner

haraldk commented Aug 15, 2022

Hi,

Thanks for reporting!

I think can reproduce the issue you describe, but only for some BMPs (24 bit/pixel BMPs).

Using the test file imageio/imageio-bmp/src/test/resources/bmpsuite/g/rgb24.bmp:

image

I get this result:

image

While the expected result should be:

image

(The problem is that the subsampling don't take into account that the samples are wider than the data type, ie. 24 bits/pixel is 3 bytes/pixel. This is why you see 1/3rd of the image horizontally subsampled, then the next 2/3rds non-subsampled but truncated.)

@haraldk
Copy link
Owner

haraldk commented Aug 17, 2022

Closing for now, let me know if you still have issues using the latest snapshot release.

@haraldk haraldk closed this as completed Aug 17, 2022
@sghpjuikit
Copy link
Author

Hello. I didn't think you would fix this so soon, Anyway:
I couln't find the snapshot jars anywhere, so I built it on my own and used in my project. I did have some issues but in the end, the project used the snapshot version and the issue is gone. Thx.

Btw, the logic seem suspect to me

            switch (bitCount) {
                case 1:
                case 2:
                case 4:
                case 8:
                case 24:
                    byte[] rowDataByte = ((DataBufferByte) rowRaster.getDataBuffer()).getData();
                    int bitsPerSample = bitCount == 24 ? 8 : bitCount;
                    int samplesPerPixel = bitCount == 24 ? 3 : 1;

Why are you testing the bitcount when it already is confirmed to be 24?

@sghpjuikit
Copy link
Author

Oh, my bad, I didn't realize switch with fall-through is still a thing in older Java versions. Nvm

@haraldk
Copy link
Owner

haraldk commented Aug 19, 2022

Snapshots can be found here (link on the front page). But building yourself is also good! 😀

And yes, I considered having completely separate branches for the <= 8 bit case and the 24 bit case, but it would just be more duplicated code (and scope issues) so I kept the fall-troughs as it was.

haraldk added a commit that referenced this issue Aug 19, 2022
haraldk added a commit that referenced this issue Aug 19, 2022
haraldk added a commit that referenced this issue Aug 19, 2022
(cherry picked from commit d1872ce)
@sghpjuikit
Copy link
Author

I have never realized until today that the badges on top of README file are clickable! That's why I could not find the snapshots.
Thx for pointing me to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants