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

Allow rgbgfx -c embedded to accept embedded palettes with extra unused colors #1064

Closed
Rangi42 opened this issue Sep 28, 2022 · 8 comments
Closed
Assignees
Labels
enhancement Typically new features; lesser priority than bugs rgbgfx This affects RGBGFX

Comments

@Rangi42
Copy link
Contributor

Rangi42 commented Sep 28, 2022

Continuing discussion from #1062:

Currently rgbgfx -c embedded expects the PNG's indexed PLTE chunk to define exactly four colors. Some users may have PNGs with more than that (e.g. 256 colors), but only use colors 0-3. Arguably this should be a supported use case.

@Rangi42 Rangi42 added enhancement Typically new features; lesser priority than bugs rgbgfx This affects RGBGFX labels Sep 28, 2022
@ISSOtm
Copy link
Member

ISSOtm commented Sep 29, 2022

No, because they may be expecting the extra colours to end up in the generated palette even if they aren't used. Ignoring extra colours if both -n and -s are specified, maybe.

@aaaaaa123456789
Copy link
Member

How would that be a reasonable expectation when the palette is four colors?

@ISSOtm
Copy link
Member

ISSOtm commented Sep 29, 2022

If it's four colours, then they would be used, and emitted in any order unless -c is passed. -c is the opt-in for "actually, I manipulate colours by index, so it's important that you respect this". Otherwise, they just want to display something, so they would just copy the binary palette data to CRAM.

Grayscale sorting makes more sense by default, since on DMG, palettes are a single byte—not something you'd want to INCBIN—so sorting in a way compatible with the common $E4 palette is a good default.

@aaaaaa123456789
Copy link
Member

I was talking about the GBC palette. How would it be reasonable to expect palette data to contain more than four colors (even if the image does) when the GBC palettes can only contain four colors?

@pinobatch
Copy link
Member

Test case: testcase2.png has 8 colors and uses the first 4. testcase3.png has 4 colors. Converting them with -c embedded should produce the same character data.
testcase2
testcase3

@ISSOtm
Copy link
Member

ISSOtm commented Sep 29, 2022

I was talking about the GBC palette. How would it be reasonable to expect palette data to contain more than four colors (even if the image does) when the GBC palettes can only contain four colors?

If it contains 8, it means 2 palettes of 4 colours, for example.

@pinobatch
Copy link
Member

Though it'd be impossible to actually have pixels using those additional 4-color subpalettes without -c modulo as described in #1067.

@Rangi42
Copy link
Contributor Author

Rangi42 commented Oct 1, 2022

This is the same non-issue as #1074: -c embedded does accept embedded palettes with extra unused colors, and just ignores them.

@Rangi42 Rangi42 closed this as completed Oct 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Typically new features; lesser priority than bugs rgbgfx This affects RGBGFX
Projects
None yet
Development

No branches or pull requests

4 participants