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

Use 8-bit color indices consistently #66

Merged
merged 1 commit into from
Apr 11, 2022
Merged

Conversation

Holzhaus
Copy link
Owner

@Holzhaus Holzhaus commented Apr 7, 2022

Before, we sometimes used 8-bit indices and sometimes 16-bit ones,
because the reverse-engineered format documentation described it that
way.

However, these does not seem to be a good reason why there should be
16-bit color indices when the value it contains is just 8-bit. It's
likely that the most significant byte actually belongs to another field.

Also see:

Deep-Symmetry/crate-digger#27

Before, we sometimes used 8-bit indices and sometimes 16-bit ones,
because the reverse-engineered format documentation described it that
way.

However, these does not seem to be a good reason why there should be
16-bit color indices when the value it contains is just 8-bit. It's
likely that the most significant byte actually belongs to another field.

Also see:

  Deep-Symmetry/crate-digger#27
Copy link
Contributor

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Though we should wait with merging until this was discussed "upstream" (Deep-Symmetry/crate-digger#27)

Copy link
Contributor

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

I guess there is no upstream controversy regarding the change, so LGTM

@Holzhaus Holzhaus merged commit 36e526c into main Apr 11, 2022
Holzhaus added a commit that referenced this pull request Apr 12, 2022
Holzhaus added a commit that referenced this pull request Apr 12, 2022
@Holzhaus Holzhaus deleted the drop-16-bit-color-index branch October 6, 2022 16:27
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.

2 participants