Add transform.hsl()

[itzpr3d4t0r]

Closes #2310.
This PR implements a general purpose function that lets users modify Surface's hue, saturation and lighness values.

[PurityLake]

Looks fairly good, only thing really needed now is tests, and it should be good to go.

[itzpr3d4t0r]

Bulk of the work is done. I changed the algorithm for a 19% perf improvement, added tests and docs.
What is still missing:

• Checking that the algorithm always works without strange quirks
• Possibly adding more tests (for the various bit depths)
• Fixing the reason why the tests are failing, which is a one-value-off issue (so like the expected rgb is (20, 0, 0) but the algorithm sometimes does (21, 0, 0)).

I still don't know the cause of the issue as it could be multiple sources, but the algorithm performs as expected for what I tested.

[MyreMylar]

Overall this works well, I just have a couple of questions about surface locking and unlocking and as this seems to be done in several different ways all over the transform module. Looking up the python C API it seems to say:

Note Calling system I/O functions is the most common use case for releasing the GIL, but it can also be useful before calling long-running computations which don’t need access to Python objects, such as compression or cryptographic functions operating over memory buffers. For example, the standard zlib and hashlib modules release the GIL when compressing or hashing data.

Which I think all our transforms would qualify for.

[MyreMylar]

I spent a bit of time looking into surface locking in SDL. Basically it is only needed when the surface has been RLE encoded.

As far as I can tell (though there is an open bug about it, but I agree with the last comment), MUST_LOCK(surf) tells us whether the surface needs locking - which, if true, is something we should do before any direct pixel access.

I've added two suggestions to the code here that will lock the surface before accessing the pixels and then gate the call to unlock only if the surface is already locked.

Untested, but you could probably create a surface with RLE - blitting it to activate the RLE encoding on the surface - and then try running the hsl function post blit to create a good test of locking/unlocking.

[itzpr3d4t0r]

I have an update to share. I’ve managed to achieve a substantial performance improvement of approximately 43% for this Pull Request when dealing with 4bpp surfaces. This enhancement could be significant for real-time usage before we get AVX or SSE in. For instance, in a test case with an 800x800 resolution, the frame rate increased from 51 fps to 74 fps.

The idea is bypassing SDL’s functions for 4bpp surfaces, which are notably slow without any apparent reason. Instead, I’ve replaced the get/set sections with faster versions, ensuring no loss of functionality.

[Starbuck5]

Itzpr brought up a bug on discord where only changing lightness would lead to unexpected results. I see he's given this PR a "dont merge" label to reflect this, but I'm going to move it to draft just to take it off the review docket while this is resolved.

[itzpr3d4t0r]

I have an update to share on this. After some experiments I found out that the current strategy for lightness isn't actually bugged but rather it's considered a naive way of handling lightness change. Take this picture for example:

If we use my algorithm for just increasing its lightness by 60% we get this:

Wich seems weird but it's actually correct.

If we take an image manipulation software such as paint.net (or Krita) and we increase the surface lightness by 60% we get this:

Now, fortunately I've found out that Krita offers 2 modes for changing lightness and one is so called "legacy" which actually matches up with my implementation's result:

Fortunately Krita is open source so I can look up the implementation and correct this issue now.

[Gabryel-lima]

Wow, very good!