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

Try both gray and indexed low depths #594

Merged
merged 3 commits into from
Mar 11, 2024

Conversation

andrews05
Copy link
Collaborator

@andrews05 andrews05 commented Feb 18, 2024

Changes the depth reduction logic to also try low-depth indexed even if we already achieved low-depth grayscale, but only if we can do it even lower than the grayscale.

Fixes #515

@ace-dent
Copy link

There are rare situations where higher bit-depth indexed can beat lower bit-depth grayscale. In part because: 1. freedom to optimise the first colour entry (explained in original issue); 2. 8-bit symbols may form a more compressible data stream. The assumptions in the PR are quite reasonable for most cases. However, it'd be nice if there was a max level that tested all these combinations (color type & bit depths).

@andrews05
Copy link
Collaborator Author

8-bit indexed can indeed be better than grayscale and this is well covered already.

The PR is specifically for low depths (1,2,4), where indexed will necessarily reach equal or lower depth than grayscale. There was a missed reduction where the indexed might have reached a lower depth than the grayscale but we weren't checking. I haven't actually found such an image in practice though (I had to create one synthetically to test), which is why I've been so slow to add this.

It is theoretically possible that indexed could be better even if it isn't lower (e.g. 2-bit indexed vs 2-bit grayscale), but it seems unlikely. Maybe I should test that further...

@andrews05 andrews05 force-pushed the feature/indexed-depth branch 2 times, most recently from 9bda035 to 3ece693 Compare February 20, 2024 19:13
@andrews05 andrews05 force-pushed the feature/indexed-depth branch from 3ece693 to 722ba66 Compare March 7, 2024 04:56
@andrews05
Copy link
Collaborator Author

andrews05 commented Mar 7, 2024

I've updated this to attempt both reductions, but to skip the evaluation if the data comes out the same.

Copy link
Collaborator

@AlexTMjugador AlexTMjugador left a comment

Choose a reason for hiding this comment

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

I've reviewed this PR a bit some weeks ago, but didn't submit a review. Now that I get back to it, I think it looks good 👍

@andrews05 andrews05 merged commit 3e3c027 into shssoichiro:master Mar 11, 2024
10 checks passed
@andrews05 andrews05 deleted the feature/indexed-depth branch March 11, 2024 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chooses 2-bit grayscale over 1-bit indexed
3 participants