-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow image_frombuffer to understand cairos premultiplied ARGB32 #2972
Comments
Hello, I've started to do some research on this issue. I found a couple people with various integration solutions: (They all seem complicated 😅) I also found a Cairo page on the matter: In terms of byte swapping, I devised a test program and it seems like Cairo is doing the swapping. import pygame
import cairo
surf2 = cairo.ImageSurface(cairo.FORMAT_ARGB32, 2, 1)
ctx = cairo.Context(surf2)
ctx.rectangle(0, 0, 2, 1)
ctx.set_source_rgb(0, 0.5, 0.5)
ctx.fill_preserve()
print(bytes(surf2.get_data()))
print([int(i) for i in bytes(surf2.get_data())])
null = bytes(8)
surf3 = pygame.image.frombuffer(null, (2, 1), "ARGB")
surf3.fill((0, 128, 128))
print(null)
print([int(i) for i in null]) Output (Windows 10, Pygame latest, PyCairo latest):
In terms of alpha premultiplication, I found that SDL does not have support for it, as Cairo's SDL integration page stated. However, pygame has a special blend flag for |
pygame would need a "XRGB" here that would give an RGB surface? Because reasons:
aside: With SDL 1, messing around with the masks could get you there. I wonder if from* should support mask twiddling. |
These both seem like slightly annoying but solvable, let's look at byte order. Is the order of the bytes from Cairo just a result of x86 being little-endian ? (I always found this a bit confusing !) If I was setup to build pygame I'd have a poke at this, and try flipping the logic at #if SDL_BYTEORDER == SDL_LIL_ENDIAN and see what the output looks like (not a correct solution, but could point at something). |
A bit more info here on cairo Image Surfaces in ARGB32 format: https://cairographics.org/manual/cairo-Image-Surfaces.html
|
I was/am having trouble wrapping my head around it, but I found a very helpful wiki page for this: https://handwiki.org/wiki/RGBA_color_space
There's also some tables and diagrams on the wiki page beyond these quotes, so check those out. So I think pygame is using the byte order scheme, and Cairo is using the word order scheme. |
So maybe SDL uses the word order scheme, and that's why pygame's functions can reverse the order on different endianness? We could close this issue by adding new format arguments specifically for the word order scheme. Like Have any other libraries faced this yet and implemented something? What's PIL doing? Edit, this is what PIL is doing: https://pillow.readthedocs.io/en/stable/handbook/concepts.html#concept-modes Maybe what Cairo wants from ARGB is PIL's mode |
It looks like PIL has lowercase Cairo also has just RGB which is 3 bytes and 1 for padding, this might be less confusing to test with initially. I should probably write some tests to ensure the examples on the Cairo integration page are correct. There's PIL to Cairo there (but not the other way round), in the example it's called BGRa, but as mentioned I should test if it's correct. |
In SDL 2 setting the masks is not allowed on Surfaces. Previously this issue could have been handled with a Surface.set_mask call. In pygame 2 set_mask doesn't work. We may be able to get it to work by creating a new underlying SDL Surface on the python object. pygame 2 broke various adapters including a opencv one because set_masks stopped working. As well, we may want to consider adding a mask argument to frombuffer/tobuffer. |
I wonder if we could make sort of quick mapping from a python function on surface called something like 'shuffle_channels(from, to)' then pass it over to:
(An AVX2 intrinsic) and pretty efficiently rejig the channels for any surface. Essentially at the important part you have:
Would do 8 pixels per loop of the surface, and you'd need a one pixel version to finish off any rows. We could likely quickly make a bunch of these as the rest of the code around it would be a fairly boilerplate operation. Then you'd need the slow non-SIMD version for the non-SIMD platforms. An idea anyway. |
Cairo's ARGB32 format is BGRA in pygame format. Using BGR as the example, BGRA would look something like,
|
I compiled pygame with the changes to image.c, as I outlined above. However, not fully tested!! pygame/pycairo code to use will be,
bandicam.2022-07-23.02-54-07-117.mp4 |
Fantastic :) I wonder it's possible to make That would make alpha compatible. Cairo also has non alpha version, in effect - BGR, which I believe is basically the same 32 bit format, but the alpha channel is not populated. |
Using the example for
and switching R & B values. Seems straightforward. But that is code for, To do this within, is not as simple. The existing code isn't manipulating the data in any way. Therefore a premult, would require adding code to multiply each R, G, & B value by the A value prior to feeding it to It is costly, that is looping over the entire data and performing the multiplication. Alternatively, but same cost, have standalone generic This is along the lines of what was proposed by MyreMylar e.g. The nice thing about that is it would be one function for all formats. |
Looks like some great work @rlatowicz! |
Looking at this, It might be useful to some to offer an
|
You mathematically can't unpremultiply reliably with only 8 bit colour channel data. See:
You either keep a second surface worth of data with the original channel data (wasteful of memory, so not going to happen) or you would have to store channel data at a much higher resolution - and even then it wouldn't be perfectly reversible. Also - alpha premultiplied is a perfectly usable (in fact superior) format in pygame anyway so there is no real need to attempt a reversal. Just use the |
I agree. In regards to the converter, it should probably offer both an inplace conversion (faster) as well as the option to return a new buffer. Or separate functions - e.g. |
Using @Starbuck5 's example above, this is a minimal workbench for testing a premult,
output is,
current |
The inplace version,
output again,
|
This was added to SDL in Nov 2021, and sdl-added-sdl-premultiplyalpha-to-premultiply-alpha-on-a-block-of-sdl-pixelformat-argb8888-pixels Note that ARGB32 is an alias for ARGB8888, But it hasn't been wrapped by pygame code. Code is here, |
It's hackish but it was the path of least resistance. I added a I added to I added to
Then added the following code to the
Apologies for lack of comprehensive error checking. This was a quick and dirty test.
Output,
|
Here is the full image.c Like I said, not a finished product and this is only the inplace premult. SDL_PremultiplyAlpha() isn't optimized. |
There is a PR open already to add an optimised Uses SSE2 at the minute but I will likely also add an AVX2 version before the PR gets merged. |
I'm not sure what I am trying to do :) I started simply wanting to have have pygame receive a pycairo image without having to do an intermediary step. Adding the 'BGRA' code solved that. I thought the 'BGRA_PREMULT' was wanted by others in this thread but it makes no sense to me :) Why on earth would you want to premult again? I guess I want clarification on the use case. |
I'm not sure :) but you seemed to be trying to do that. I'm sure we'd be happy to accept a PR for |
I have tentatively added But a simply addition to
passes!
However, in The way that function is written, I'm doing it the slower way for now,
as opposed to something like,
Having looked at it a bit closer, I see how to adapt For another time. |
This might be my fault for coming back and not getting myself back up to speed again quickly enough, apologies ! Amazing work on all this, I always liked the idea these libraries could interoperate. |
The PR has been merged. Does that mean this issue is resolved? |
Yes. BGRA format added to frombuffer, tostring (tobytes) & fromstring (frombytes). A usage e.g.,
|
One thing I could see potentially remaining in this issue is documenting this somewhere. Not sure if there would be a good to place it. Maybe a small pygame - cairo interoperability demo in |
Is this too elaborate/busy?
also shows that pygame events work as normal and can be used in interacting with the cairo drawing. I just added a comment showing the alternate usage with frombytes() or fromstring() |
^ Just a little nit - did you mean to have .paint() and .fill() ? |
After a bunch of edits that happens :) Thanks. |
|
Nice :) There are a lot of possibilities for things like interesting HUDs using cairo, or games using curves - there are the blend modes for filling and painting and stuff like gradient fills - I think this opens up a new world of interesting things. |
shouldn't this line use the |
There won't be a difference using any of those 3, for the example given? Why would you use it here? |
This was my thought, even if the surface has no actual alpha it is an indication of the format of the data - though perhaps a comment would also be useful? |
I wonder if it's worth having some images with alpha using layering as a demo ? This would let people see that the premultiplied alpha worked. |
Can this issue be closed now? |
I think so, though I guess an example "would be nice", that might be worth adding as another ticket. I finally tried the example and it's fun :) I'll shut this now (I'd forgotten that I opened it by now!) - it looks like there are some other tickets that have split off this, but they can probably have their own life now. |
It would be good if image_frombuffer could understand pycairos premultiplied ARGB32 format, currently it seems impossible to use, as pygame is doing some byteswapping and it's not obvious how to enable alpha premultiplication.
In detail:
@hanysz found an interesting issue using pycairo to create surfaces for pygame pygobject/pycairo#247
Cairos ARGB32 format stores ARGB starting in the high byte, with alpha premultiplied.
The docs make it look like image_frombuffer should be able to handle this format, but unfortunately there is some
undesired byteswapping happening.
The implementation calls SDL_CreateRGBSurfaceFrom, which looks like it could be given the right parameters to understand the format.
@hanysz test prog creates an image in cairo, rendering text in red, green and blue and renders in pygame - but only the "red" text is output, in blue:
The feature request:
Possibly this needs a new format: native endian RGBA32 with alpha premultiplication, or maybe it's a matter of providing a method of disabling the byteswapping.
The text was updated successfully, but these errors were encountered: