Skip to content
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

lv_img_conv: support other modes like 'P' #1989

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

NeroBurner
Copy link
Contributor

Support other image modes like P, which uses 8 bits per pixel and a color palette to save space.

Luckily the Pillow module can do the mode conversion for us.

Fixes: #1985

Support other image modes like `P`, which uses 8 bits per pixel and a
color palette to save space.

Luckily the Pillow module can do the mode conversion for us.

Fixes: #1985
@NeroBurner NeroBurner added this to the 1.15.0 milestone Jan 21, 2024
@NeroBurner
Copy link
Contributor Author

@ekirchman please review

@NeroBurner NeroBurner requested a review from a team January 21, 2024 20:51
Copy link

Build size and comparison to main:

Section Size Difference
text 369768B -16B
data 940B 0B
bss 63516B 0B

Copy link
Member

@FintasticMan FintasticMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@ekirchman
Copy link

Is there any reason we don't want to use try and handle each mode separately? GIMP and Gwenview can even can tell that these images have transparency to them. And at least before they're converted to RGBA, 'P' type images are smaller in file size than RGBA images

@NeroBurner
Copy link
Contributor Author

CF_INDEXED_1_BIT is a special case, that is essentially a P image with just one bit, so a color palette with two colors, in our case hard coded to black and white

elif args.color_format == "CF_INDEXED_1_BIT": # ignore binary format, use color format as binary format

Other than that I don't really know why we "only" have ARGB8565_RBSWAP and CF_INDEXED_1_BIT. I just converted the nodejs script to python because I was tired of the nodejs dependencies to break 😅

You may look at the original javascript based lv_img_conv script here: https://github.com/lvgl/lv_img_conv

My guess is, that any binary format this script can produce is in some way usable by LVGL. Maybe only a subset of those are usable by PineTime, but I honestly don't know


In any case, I hope your original issue is resolved with this. If so please approve the PR :)

@FintasticMan
Copy link
Member

The output is controlled entirely by the color_format and binary_format options, so converting the input shouldn't have any effect on the output. This means that you don't need to be concerned about the file size being bigger with the "RGBA" mode.

Copy link

@ekirchman ekirchman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @FintasticMan that due to the color format being the same the file size shouldn't different if at all

@NeroBurner NeroBurner merged commit a481af0 into main Jan 23, 2024
5 checks passed
@NeroBurner NeroBurner deleted the lv_img_conv_support_P_mode branch January 23, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lv_img_conv.py doesn't handle 8-bit pixel mode
3 participants