adamdruppe / arsd

This is a collection of modules that I've released over the years. Most of them stand alone, or have just one or two dependencies in here, so you don't have to download this whole repo.
http://arsd-official.dpldocs.info/arsd.html
530 stars 125 forks source link

color: Replace color components union with property functions #406

Closed 0xEAB closed 1 month ago

0xEAB commented 6 months ago

In its current form, this is a breaking change suitable for arsd 12 or a later major release.

getter+setter vs. union

Color.components is turned into ubyte[4]. Unlike an anonymous union, this works with CTFE as well.

r, g, b, a and asUint are turned into getter + setter functions.

A sufficiently good optimizing compiler will generate the same code for these as it would previously have with the union. (Note that this cannot be confirmed with DMD’s built-in profiler as it’s implementation will poison any getter/setter functions.)

alias this

The alias this stems from my codebase which did use it to simplify certain implementation details. I’m not sure whether it’s a good thing or even worth it.

/+ code sample +/
foreach (immutable idx, ref component; pixelDestination) {}
/+ vs +/
foreach (immutable idx, ref component; pixelDestination.components) {}
0xEAB commented 6 months ago

I’d like to hear your opinion on this, @adamdruppe.

My motivation behind this change is to get arsd to act more or less like a drop-in replacement and make it more convenient to use. While I tried to make it work like the previous implementation for most cases, I don’t have any data on this change’s actual impact on backwards compatibility, to be honest.

0xEAB commented 6 months ago

I’ve updated this patch. Not sure what to do with it. No longer sure whether it adds value.

0xEAB commented 2 months ago

No longer sure whether it adds value.

Well, it removes an anonymous union. Could be an advantage.

0xEAB commented 2 months ago

@adamdruppe Any comments/suggestions? :)

adamdruppe commented 2 months ago

i kinda forgot about this one lol lemme look at it again

adamdruppe commented 2 months ago

well looking it back over it looks ok enough. i can go ahead and merge if you wanna

0xEAB commented 1 month ago

Any suggestions to improve it?