Fix #148

[lovasoa]

Avoid creating output buffers of the wrong size for grayscale images with a width and a height that are multiple of 8.

Fixes #148

[lovasoa]

This PR fixes the issue, but it may be worth investigating why the output data wasn't of the expected size in the first place. The single decoded Component for the image in the issue is :

[
    Component {
        identifier: 1,
        horizontal_sampling_factor: 2,
        vertical_sampling_factor: 2,
        quantization_table_index: 0,
        dct_scale: 8,
        size: Dimensions {
            width: 800,
            height: 280,
        },
        block_size: Dimensions {
            width: 100,
            height: 36,
        },
    },
]

So, the image itself is 800x280, but the block_size is 100x36, where one might have expected 100x35 (block_size.width/dct_scale x block_size.height/dct_scale)

[HeroicKatora]

Would you try out (https://github.com/image-rs/image/issues/1165) and (https://github.com/image-rs/image/issues/1157) as well? Very likely the same issue.

[lovasoa]

@HeroicKatora : OK, will do.

I am currently investigating the code, and found that. There is some dirty business going on in there.

[HeroicKatora]

Ahh floating point computation. That may well be the underlying reason.

[lovasoa]

Would you try out (image-rs/image#1165) and (image-rs/image#1157) as well? Very likely the same issue.

These are both color images, this PR only fixes a bug in the greyscale image handling. I tried to add one to the tests and it works fine. Maybe there is a bug in image-rs ?

[lovasoa]

@HeroicKatora : I switched to integer calculations in update_component_sizes and added tests. However, the bug wasn't caused by update_component_sizes. The reported image indeed has 100x36 blocks, because of the sampling factor of 2x2.

[lovasoa]

So, is it ok to merge?

[lovasoa]

The other image is from the original issue: https://github.com/image-rs/jpeg-decoder/issues/148

[HeroicKatora]

Also, seems like the keyboard image is fine to use with attribution: https://github.com/ArturKovacs/emulsion/issues/6#issuecomment-619320012

[lovasoa]

Also, seems like the keyboard image is fine to use with attribution: ArturKovacs/emulsion#6 (comment)

I have already created multiple images that reproduce the issue and several variants of it, so I don't think we need the original image anymore. Do you want me to add it again anyway ?

[lovasoa]

The way git works, you need to rebase to remove the commit that introduced the images and not only add an extra commit.

You can simply use "Squash and merge".

[HeroicKatora]

You can simply use "Squash and merge".

True. Usually I try to preserve the history, gpg signatures, etc but that is an alternative.

[lovasoa]

Awesome, thanks for merging ! 🙏