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

DiscreteAlphaIndexColorModel.getNumComponents is giving the wrong information #693

Closed
jimfcarroll opened this issue Jul 29, 2022 · 7 comments

Comments

@jimfcarroll
Copy link

jimfcarroll commented Jul 29, 2022

Describe the bug
According to my understanding, the ColorModel.getNumComponents should return the number of components/channels in the final image, not in the sample buffer. DiscreteAlphaIndexColorModel.getNumComponents' implementation is giving sample size.

Version information

  1. 3.8.2

  2. More details:

If you look at the documentation for getNumComponents you can see that this is the number of channels in the final image. E.g. RGB would be 3. RGB with Alpha would be 4. This can be verified by looking at the implementation for an IndexColorModel (which defaults to ColorModels implementation). Usually, with an IndexColorModel, the sample buffer would contain an single array of 1 byte per pixel. That means the "number of samples" per pixel is 1. However, if I have an RGB image with an IndexColorModel, the .getNumComponents is 3.

DiscreteAlphaIndexColorModel is basically a 2 "sample" per pixel model (one sample is an RGB image with an IndexColorModel and the other is the Alpha channel). Therefore, under almost all circumstances, the getNumComponents should return 4 (unless it somehow got constructed for an image without an alpha channel, in which case it should return 3 since it should act just like an IndexColorModel without an alpha channel).

Instead, it always returns 2 (potentially 1 if it doesn't have an alpha channel).

It's possible there's a misunderstanding on my part. If so, please let me know.

@jimfcarroll
Copy link
Author

ExtraSamplesColorModel has the same problem

@haraldk
Copy link
Owner

haraldk commented Aug 2, 2022

Thanks for reporting!

I'm currently on summer holiday with my family, but will look into it when I'm back in the office.

--
Harald K

@haraldk
Copy link
Owner

haraldk commented Aug 12, 2022

Hi Jim,

I agree with you on DiscreteAlphaIndexColorModel.getNumComponents, the current implementation is incorrect, and should return the number of color components in the color space + alpha samples (ie. RGB + A -> 4 in most cases). I'll fix!

I'm less sure about the ExtraSamplesColorModel (and DiscreteAlphaIndexColorModel with extra samples) cases. I think the correct number of components should include the extra samples (ie. RGB + A + x -> 5 in most cases, sometimes more). It allows us to access these samples, even if they don't affect the image visually.

Are there any specific cases where the current implementation here (ExtraSamplesColorModel) cause problems for you?

Best regards,

--
Harald K

@jimfcarroll
Copy link
Author

I have a project with a conversion from a BufferedImage to an OpenCV mat. In the test for the conversion I compare the pixel values from the original BufferedImage to the resulting Mat pixel-by-pixel. That compare fails because of the number of channels. Here is my workaround in the test code:

https://github.com/KognitionAI/pilecv4j/blob/master/lib-image/src/test/java/ai/kognition/pilecv4j/image/UtilsForTesting.java#L108

If I'm misinterpreting what that method means on the color model, then I can just use: colorModel.getNumColorComponents() + (hasAlpha ? 1 : 0) but I thought I'd just add the issue here as an FYI in case I'm not misinterpreting it.

@haraldk
Copy link
Owner

haraldk commented Aug 19, 2022

DiscreteAlphaIndexColorModel fixed in 190fe87

@haraldk
Copy link
Owner

haraldk commented Aug 19, 2022

I think your workaround with the getNumColorComponents() + (hasAlpha ? 1 : 0) makes sense, as you copy only the color components and an optional alpha channel to the OpenCV mat. The other components (extra samples) in the raster are ignored.

Wether or not the current behavior is strictly "correct", is hard to say, as this is something the Java2D API doesn't seem to take into account at all...

Any way, closing this for now.

@haraldk haraldk closed this as completed Aug 19, 2022
@jimfcarroll
Copy link
Author

Thanks. Sounds great.

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