Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43358 )
Change subject: cbgfx: Add blend functions to calculate transparency ......................................................................
Patch Set 5:
(4 comments)
LGTM except for style and documentation nits
https://review.coreboot.org/c/coreboot/+/43358/5/payloads/libpayload/drivers... File payloads/libpayload/drivers/video/graphics.c:
https://review.coreboot.org/c/coreboot/+/43358/5/payloads/libpayload/drivers... PS5, Line 189: nit: should technically indent one space less here, if you're trying to match parentheses
https://review.coreboot.org/c/coreboot/+/43358/5/payloads/libpayload/include... File payloads/libpayload/include/cbgfx.h:
https://review.coreboot.org/c/coreboot/+/43358/5/payloads/libpayload/include... PS5, Line 215: * Set alpha values to setup transparency calculations This isn't a super good explanation of what it really does (and also no relation to the explanation for clean_blend(), which says "disabling dimming" while this says "transparency calculations"). Something here should explain the whole process clearly, that after you call this all future drawing calls (boxes and images) will be alpha blended with the supplied values, until you turn it off again.
https://review.coreboot.org/c/coreboot/+/43358/5/payloads/libpayload/include... PS5, Line 220: * 256 = completely opaque Same here (see below)... 255 is the largest legal value.
https://review.coreboot.org/c/coreboot/+/43358/5/payloads/libpayload/include... PS5, Line 236: * 256 = 100% opacity Except that that one is illegal and will be treated as 0 because the parameter to set_blend() can only fit a uint8_t. ;)
Actually, probably better to put a MAX() in the macro just in case.