Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42792 )
Change subject: libpayload: cbgfx: Replace bilinear resampling with Lanczos ......................................................................
Patch Set 16:
(3 comments)
Note that puff-cq might fail due to the size increase of depthcharge.elf. This happened to me when I tried to enable this with Groot UI, even with the lowered bmpblk screen resolution (CL:2282241).
Hmm... how tightly did you pack it? On a Trogdor this takes about 1.3KB extra for me (about half of that would be the sine stuff), which is nothing compared to the 100+KB for the whole depthcharge image or for any individual hi_res language.
In general, any libpayload/depthcharge patch could always increase your binary sizes a bit, so if your image is packed super tight there's always that risk.
https://review.coreboot.org/c/coreboot/+/42792/15//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42792/15//COMMIT_MSG@9 PS15, Line 9: This patch improves the image resampling (scaling) code in CBGFX to use : the Lanczos algorithm that is widely considered the "best" resampling : algorithm (e.g. also the first choice in Python's PIL library).
Do you have a test image, the differences can easily be seen with?
Added
https://review.coreboot.org/c/coreboot/+/42792/15//COMMIT_MSG@12 PS15, Line 12: and therefore slower than bilinear : resampling
It’d be great if you could provide some specific data: On <device> there is <1 ms> difference. […]
Well, you can measure it by applying/reverting the patch. Added an example from my tests.
https://review.coreboot.org/c/coreboot/+/42792/15/payloads/libpayload/driver... File payloads/libpayload/drivers/video/graphics.c:
https://review.coreboot.org/c/coreboot/+/42792/15/payloads/libpayload/driver... PS15, Line 751: if (equals++ >= (SSZ * SSZ))
Too many leading tabs - consider code refactoring
Just to clarify, I'm not planning to address this unless someone feels very strongly about it. I can't have less indentation levels due to the amount of nested loops this needs, I think the part over here is still about as readable as it's gonna get, and trying to factor out inner parts of the loops into separate functions (with all the different state that would have to be passed through to there) would just make this harder to follow, I think.