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

Fixed invalid jpeg buffer width compliment for scalar color converters #2427

Merged
merged 2 commits into from
Apr 4, 2023

Conversation

br3aker
Copy link
Contributor

@br3aker br3aker commented Apr 2, 2023

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Partially fixes #2414 for jpeg and tiff with jpeg compressions tests for scalar path. According to #2414 (comment) we have some more failing tests for GIF and PNG.

Problem was with how jpeg decoder determined buffer size compliment for color converters as those required buffer size to aligned to their batch size, e.g. for avx with batch size 8 buffer size should be divisible by 8, 204 => 208. For scalar encoders any buffer size is complient as any number is divisible by 1 without a remainder.

Decoder was adding (batch_size - remainder) to the buffer size which led to extra 1 pixel for scalar implementations- that's why decoded image had this diagonal color shift:
image

This PR is not covered by tests as I want us to discuss possible solutions of how actually we should cover all scalar paths. I think solution in #2414 (comment) could be quite handy

Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job working this one out!

@JimBobSquarePants JimBobSquarePants merged commit db49134 into SixLabors:main Apr 4, 2023
@br3aker br3aker deleted the dp/jpeg-scalar-path-fix branch April 6, 2023 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some unit tests fail with SSE disabled (i.e. scalar path only)
3 participants