Add `rgbgfx -c modulo` to accept embedded palettes with extra used colors

[pinobatch]

Say I have an indexed image with 16 input colors, and I want each input color to be reduced modulo 2**bit_depth in the output. For example, input colors 1, 5, 9, and 13 would become 1 in the output. I have relied on this behavior of a different tool in a previous project targeting an 8-bit console with 2bpp character graphics. Would it be reasonable to add -c modulo, which is -c embedded without the fatal error on an out-of-bounds color value?

If there's no serious objection, I plan to attempt the PR myself.

Test case: testcase1.png has 8 colors and uses all 8. testcase2.png has 8 colors and uses the first 4; all color index values have been reduced modulo 4. testcase3.png has 4 colors. Converting them with -c modulo should produce the same character data as converting testcase3.png with -c embedded. (I've used two of them as test cases for #1064 as well.)

[ISSOtm]

Isn't it sufficient to generate the palette with -c embedded, ignore the attributes, and truncate the palette data?

$ rgbgfx -c embedded -p >(dd of=testcase1.pal bs=8 count=1 status=none) -o testcase1.2bpp testcase1.png

[pinobatch]

Most tiles in the test case contain both pixels of color 0 and pixels of color 4. I had planned to try the command line snippet in ISSOtm's reply to observe how rgbgfx master reacts to testcase1.png. However, I do not understand the snippet.

1. What does >( do? I don't know how to search the manual for punctuation. Web search engines strip punctuation, and searching within man bash on Ubuntu 22.04 gives Invalid pattern (press RETURN).
2. Why does this snippet appear to overwrite testcase1.png? Or am I misunderstanding the of= option of dd?

[ISSOtm]

1. What does >( do? I don't know how to search the manual for punctuation. Web search engines strip punctuation, and searching within man bash on Ubuntu 22.04 gives Invalid pattern (press RETURN).

The first result for Ctrl+F'ing >( in the single-page Bash manual explains "process substitution".

man pipes to a pager, and modern pagers tend to treat the provided search pattern as a regex. Entering >\( should work.

2. Why does this snippet appear to overwrite testcase1.png? Or am I misunderstanding the of= option of dd?

Yes, I had a brainfart. Edited the line to be correct.

[pinobatch]

It is not sufficient, at least with this particular test case.

$ /path/to/rgbgfx --version
rgbgfx v0.6.0-rc2-12-g023884d2
$ /path/to/rgbgfx -c embedded -p >(dd of=testcase1.pal bs=8 count=1 status=none) -o testcase1.2bpp testcase1.png
error: Could not fit tile colors [$0000, $6400] in specified palettes
error: Could not fit tile colors [$0000, $0019, $6400] in specified palettes
error: Could not fit tile colors [$0000, $0320, $6400] in specified palettes
error: Could not fit tile colors [$0000, $0339, $6400] in specified palettes
error: Could not fit tile colors [$0000, $6400, $6419] in specified palettes
error: Could not fit tile colors [$0000, $6400, $6720] in specified palettes
error: Could not fit tile colors [$0000, $6400, $6739] in specified palettes
note: The following palette was specified:
        [$0000, $0019, $0320, $0339]
Conversion aborted after 7 errors

I have made an additional testcase4.png, in which color 4's RGB value has been changed to equal that of color 0 and the values have not been changed. This gives fewer errors though still aborts the conversion.

$ /path/to/rgbgfx -c embedded -p >(dd of=testcase4.pal bs=8 count=1 status=none) -o testcase4.2bpp testcase4.png
error: Could not fit tile colors [$0000, $6419] in specified palettes
error: Could not fit tile colors [$0000, $6720] in specified palettes
error: Could not fit tile colors [$0000, $6739] in specified palettes
note: The following palette was specified:
        [$0000, $0019, $0320, $0339]
Conversion aborted after 3 errors

[pinobatch]

Two additional test cases:

• testcase5.png has all color 4 pixels overwritten with color 0, where colors 0 and 4 still have distinct RGB values
• testcase6.png has all color 4 pixels overwritten with color 0, and color 4's RGB values have been changed to those of color 0

testcase5.png is probably the most similar statistically to the images that I encountered when building character-replacement animation in the previous project.

[ISSOtm]

I think the errors are caused by -c embedded only accepting 4 input colours, instead of all. I'll have to look into that.

[ISSOtm]

This is why:

https://github.com/gbdev/rgbds/blob/a47da5f71f1b755b3a2fa03736cda60534dfb639/src/gfx/process.cpp#L567-L569

So if I'm understanding correctly, this should instead be:

if (embPalSize > options.maxOpaqueColors() * options.nbPalettes) {
    fatal("Too many colors");
}

, right?

[ISSOtm]

I think my rationale for this condition was that it's unclear what should be done with more than 4 colours but the number cannot be divided by 4.

[pinobatch]

If there are 7 colors 0-6, then colors 4-6 should become colors 0-2 in the output. If there are 7 colors 0-3 and 5-7, as in test cases 5 and 6, then colors 5-7 should become 1-3 in the output.

[pinobatch]

Or to put it another way: -c embedded is like -Werror=excess-colors and -c modulo is like -Wno-excess-colors

[ISSOtm]

The problem is that we don't know where the colours are being used at this point, so "0–3 and 5–7" is a non-starter.

[Rangi42]

The three test cases, for reference:

[Rangi42]

-c embedded without -n 1 should fit this use case. Edit: Apparently it doesn't.

[pinobatch]

I am using -c embedded without -n 1, and it is still raising errors for four of the six test cases attached to this issue.

$ /path/to/rgbgfx --version
rgbgfx v0.6.0-rc2-27-gbbe28faa
$ /path/to/rgbgfx -c embedded -o testcase1.2bpp testcase1.png
error: Could not fit tile colors [$0000, $6400] in specified palettes
error: Could not fit tile colors [$0000, $0019, $6400] in specified palettes
error: Could not fit tile colors [$0000, $0320, $6400] in specified palettes
error: Could not fit tile colors [$0000, $0339, $6400] in specified palettes
error: Could not fit tile colors [$0000, $6400, $6419] in specified palettes
error: Could not fit tile colors [$0000, $6400, $6720] in specified palettes
error: Could not fit tile colors [$0000, $6400, $6739] in specified palettes
note: The following palette was specified:
        [$0000, $0019, $0320, $0339]
Conversion aborted after 7 errors
$ /path/to/rgbgfx -c embedded -o testcase2.2bpp testcase2.png
$ /path/to/rgbgfx -c embedded -o testcase3.2bpp testcase3.png
$ /path/to/rgbgfx -c embedded -o testcase4.2bpp testcase4.png
error: Could not fit tile colors [$0000, $6419] in specified palettes
error: Could not fit tile colors [$0000, $6720] in specified palettes
error: Could not fit tile colors [$0000, $6739] in specified palettes
note: The following palette was specified:
        [$0000, $0019, $0320, $0339]
Conversion aborted after 3 errors
$ /path/to/rgbgfx -c embedded -o testcase5.2bpp testcase5.png
error: Could not fit tile colors [$0000, $6419] in specified palettes
error: Could not fit tile colors [$0000, $6720] in specified palettes
error: Could not fit tile colors [$0000, $6739] in specified palettes
note: The following palette was specified:
        [$0000, $0019, $0320, $0339]
Conversion aborted after 3 errors
$ /path/to/rgbgfx -c embedded -o testcase6.2bpp testcase6.png
error: Could not fit tile colors [$0000, $6419] in specified palettes
error: Could not fit tile colors [$0000, $6720] in specified palettes
error: Could not fit tile colors [$0000, $6739] in specified palettes
note: The following palette was specified:
        [$0000, $0019, $0320, $0339]
Conversion aborted after 3 errors

Thus I don't understand "closed this as completed".

[Rangi42]

@pinobatch Okay, looking at this again I see what the intended goal is.

I think "-c collapse" would be a more clear name than "-c modulo" for users.

This would require PNG pixels to initially be read as their palette indexes, collapsed with index %= 4, and only then mapped to the corresponding RGBA palette color. After that it would work just like -c embedded. (So in your testcase1.png, the -p output would include both palettes.)

That's possible to add of course, but nontrivial since currently indexed PNGs are entirely converted with libpng's png_set_palette_to_rgb before iterating over their pixels.

[ISSOtm]

This is further rendered difficult by the palette being determined before any image processing is done. Doing so being inherent to RGBGFX's processing structure (how do you even read pixel indices if you don't even have palettes yet?), this is not something that can be changed.

[Rangi42]

A preprocessing script could modulo all the PNG's palette indexes by 4 before applying rgbgfx -c embedded:

#!/usr/bin/env python

import sys, png # PyPNG

if len(sys.argv) != 3:
    print('Usage:', sys.argv[0], 'in.png out.png', file=sys.stderr)
    sys.exit(1)
with open(sys.argv[1], 'rb') as file:
    reader = png.Reader(file)
    width, height, rows, info = reader.read()
    rows = list(rows)
if 'palette' not in info:
    print(sys.argv[1], 'does not have an indexed palette!', file=sys.stderr)
    sys.exit(1)
rows = [[idx % 4 for idx in row] for row in rows]
writer = png.Writer(width, height,
    palette=info['palette'], bitdepth=info['bitdepth'], compression=9)
with open(sys.argv[2], 'wb') as file:
    writer.write(file, rows)

Then in your Makefile:

%.mod.png: %.png
    python modulo.py $< $@

%.mod.2bpp: %.mod.png
    rgbgfx -c embedded -o $@ $<