pygobject / pycairo

Python bindings for cairo
https://pycairo.readthedocs.io
Other
613 stars 83 forks source link

"Unused bits" seem to be actively used for storing alpha values #287

Closed page200 closed 1 year ago

page200 commented 1 year ago

The documentation says here for cairo.Format.RGB24 that "the upper 8 bits unused". But looking at them (with numpy) reveals that they seem to behave like with cairo.Format.ARGB32, i.e. with different alpha values being written there based on fill() and stroke(). So what does "unused" mean, that the values depend on what was easy to implement or fast to run in the current version, and the user shouldn't rely on having any specific values there?

stuaxo commented 1 year ago

This seems like something that cairo itself does (either a bug or undefined behaviour).

It's worth asking on the cairographics mailing list, I'm interested in the answer - we may need to update the pycairo docs at the very least.

page200 commented 1 year ago

https://gitlab.freedesktop.org/cairo/cairo/-/issues/602

psychon commented 1 year ago

Hi @stuaxo

My guess is as good as yours, but I would interpret "unused" as having undefined content and it seems like pixman (which cairo uses do actually modify the contents of an image surface) seems to optimise PIXMAN_OP_CLEAR to a call to memset(). So, at this point all the unused bytes would be zero.

The only implementation for PIXMAN_OP_OVER that I found seems to do alpha blending for the value in the alpha channel. Dunno how that works for "sources with an alpha channel".

For PIXMAN_OP_SRC, I found calls to memcpy.

So, my TL;DR: Unused bytes are used for whatever is the fastest to write to them. There value seem to be undefined.

stuaxo commented 1 year ago

I'll add a caveat to the docs here ... I think this task is small enough that even in my slightly ADD style of development I can get to it before getting to one of the other tasks on the large roster ;)

stuaxo commented 1 year ago

Going by the unscientific concept of "feel" ... it "feels" like perhaps cairo shouldn't be clobbering those bits.

Practically though, it's sounds like a medium size task to make sure that was the case, needing tests, and maybe replacing the memcpy calls with something still fast that leaves the unused bytes (probably some SIMD librart), so I guess fairly unlikely to happen any time soon.

stuaxo commented 1 year ago

@page200 @psychon do you think this sounds about right ?

https://github.com/pygobject/pycairo/pull/289

stuaxo commented 1 year ago

@page200 now the PR is merged that clarifies this in the docs, I think this is probably addressed

page200 commented 1 year ago

Thank you! Maybe call them "undefined" or so rather than "unused" (because they are being actively used for storing alpha values). And add that the user shouldn't rely on having any specific values there.

stuaxo commented 1 year ago

Feel free to submit this as a PR, or put some wording here - I should have double checked the wording with you before the first one.

page200 commented 1 year ago

What about something like "Each pixel is a 32-bit quantity. The upper 8 bits can be ignored - they are filled by Cairo operators (for example CLEAR and SRC) for the sake of convenience/speed in a manner that might not remain consistent across different use cases and Cairo versions. Red, Green, and Blue are stored in the remaining 24 bits in that order."

psychon commented 1 year ago

Cross-link to the cairo issue where Emmanuele responded to this suggestion: https://gitlab.freedesktop.org/cairo/cairo/-/issues/602#note_1673175

The documentation says that the bits are unused, which means "unused by application code". The more accurate description should be that their contents are "undefined", in the same spirit as C's "undefined behaviour".