Use accurate RGB 8-to-5 conversion

[Rangi42]

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

[aaaaaa123456789]

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

[Rangi42]

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

[ReiquelApplegate]

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

[Rangi42]

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

[aaaaaa123456789]

Not really a lot to save:

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.

[Rangi42]

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.

[aaaaaa123456789]

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?

[ReiquelApplegate]

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

[Rangi42]

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. :)

[ISSOtm]

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.

[Rangi42]

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]

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? 🙃)