longlene / cl-raylib

Common Lisp binding of raylib
MIT License
145 stars 21 forks source link

Efficient handling of RGBA translation #31

Closed stacksmith closed 2 years ago

stacksmith commented 2 years ago

Thank you for cl-raylib, and please consider what follows as completely constructive.

In C rgba values map nicely as a struct packed into a 32-bit value. A literal translation to Lisp CFFI (such as yours) results in repeated packing/unpacking of color data, stored on the Lisp side as a list of four values. Unnecessary packing and unpacking of color values, in every drawing primitive in the hotpath reduces performance, and unnecessary consing results in GC delays.

It would be much more efficient to handle colors as 32-bit integers in Lisp, and have all CFFI calls requiring color take it as a U32 integer. On rare occasions when manipulation of color components is needed, extraction/insertion is trivial and efficient (shift/mask in compiled code).

The component packing, performed in every call with color parameters, is hoisted into the usually non-critical-path code which prepares the colors. And any color return values will no longer get unpacked and cons up lists.

I believe the result will be a tangible performance improvement, and a lot of consing in the main loop eliminated, avoiding GC issues. And allowing for more work to be done inside the main loop.

Work plan

If you are interested, I could give it a whack.

The work scope covers:

The change would be transparent to all code that uses color constants.

stacksmith commented 2 years ago

PR #32 implements the work described above. Everything seems to work the same.

I can now read colors into lisp from foreign memory, as uints, and invoke draw functions without expanding colors into lists and back! I hope you merge this, as it makes a big difference in some cases.

Let me know if I can do anything to make it work for you. I will put together some simple component extractors for color components when I get a chance.

longlene commented 2 years ago

@stacksmith Thank you, I've merged it! Excellent change!

stacksmith commented 2 years ago

Great, thank you.