rgbds icon indicating copy to clipboard operation
rgbds copied to clipboard

Use accurate RGB 8-to-5 conversion

Open Rangi42 opened this issue 3 months ago • 13 comments

See https://threadlocalmutex.com/?p=48. Instead of just truncating with >> 3, we should do the equivalent of rounding * 31.0 / 255.0.

Rangi42 avatar Sep 06 '25 17:09 Rangi42

(value * 31 + 127) / 255 does the same as the float calculation while keeping it in integers.

aaaaaa123456789 avatar Sep 06 '25 17:09 aaaaaa123456789

Yep, and it's more intuitive than the blog post's (x * 249 + 1024) >> 11.

Rangi42 avatar Sep 06 '25 17:09 Rangi42

I dunno how the rest of you feel about this, but if avoiding the divide by 255 saves some clock cycles I am ok with using the >> 11 version and having a comment explain that it's equivalent to (value * 31 + 127) / 255

ReiquelApplegate avatar Sep 07 '25 16:09 ReiquelApplegate

This isn't about code running on a Game Boy; clock cycle counting isn't relevant.

Rangi42 avatar Sep 07 '25 16:09 Rangi42

Not really a lot to save:

Image

I have no idea of why it's doing that pointless movzx eax, ax (it's a complete no-op), but you can see how the compiler is converting the division into multiplication. This is standard when dividing by a constant.

aaaaaa123456789 avatar Sep 07 '25 16:09 aaaaaa123456789

I feel frustrated and disappointed that nobody's discussing the issue of accuracy here. Some PNG colors being converted to GB colors would change by ±1. Is that useful? Is that disruptive to anyone? Feedback on that would really help. Seeing the word "fast" in that blog post and leaping to GB-style cycle-counting, on a program that runs on your PC, does not. All it does is clutter this thread and notifications with godbolt code golfing.

Rangi42 avatar Sep 07 '25 16:09 Rangi42

I thought that was settled? If the conversion is inaccurate, make it accurate. It's a bug, albeit minor. Is there really a lot to discuss about that?

aaaaaa123456789 avatar Sep 07 '25 16:09 aaaaaa123456789

I do not have a problem with some GB colors changing by +/-1 so long as they are more accurate to the source PNGs.

ReiquelApplegate avatar Sep 07 '25 16:09 ReiquelApplegate

If anyone said "yeah, that sounds good, make it accurate" on Discord (clearly not here), I must have missed it in all the discussion of shifts and modulos.

Got it, we can make this change then. :)

Rangi42 avatar Sep 07 '25 16:09 Rangi42

It's unclear to me what the impact of such a small change would be. Reproducibility requires pinning a specific version of the toolchain, for example; and I'm not sure any disassembly relies on this either? Worth doing a test run, at least.

ISSOtm avatar Sep 07 '25 16:09 ISSOtm

Yeah, we'll see if any of the projects in the test suite break when this is implemented. (And if there are any other projects out there which are compatible with RGBDS 0.9.x, they could be tested too.)

Rangi42 avatar Sep 07 '25 17:09 Rangi42

Well, #1827 didn't break any of the CI test projects, so I'm optimistic that this will be more like a "bugfix" update for users than a "breaking change". (People aren't relying on the exact equivalence of e.g. 8-bit $B9 and $BB to fuse to the same 5-bit GB channel, right? right? 🙃)

Rangi42 avatar Sep 07 '25 21:09 Rangi42

As was pointed out on Discord (regarding creditsSprTilesVaria.png+pal), the new 5-to-8 formula affects the exact colors of a .gbc palette input file, which can change or break some valid and reasonable use cases.

So 1.0.0 will stick with the traditional shift-based conversions, and we can reconsider this rounding-based formula for 2.0.

Rangi42 avatar Oct 24 '25 17:10 Rangi42