GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — lv_img_conv: support other modes like 'P' by NeroBurner · Pull Request #1989 · InfiniTimeOrg/InfiniTime

Conversation

@NeroBurner

@NeroBurner NeroBurner commented Jan 21, 2024

Copy link
Copy Markdown
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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@NeroBurner
lv_img_conv: support other modes like 'P'
84e9425 
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 the feature request label Jan 21, 2024
@NeroBurner NeroBurner added this to the 1.15.0 milestone Jan 21, 2024
@NeroBurner

NeroBurner commented Jan 21, 2024

Copy link
Copy Markdown
Contributor Author

@ekirchman please review
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@NeroBurner
NeroBurner requested a review from a team January 21, 2024 20:51
@github-actions

github-actions Bot commented Jan 21, 2024

Copy link
Copy Markdown

Build size and comparison to main:

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
FintasticMan
FintasticMan approved these changes Jan 22, 2024
View reviewed changes

@FintasticMan FintasticMan left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ekirchman

ekirchman commented Jan 22, 2024

Copy link
Copy Markdown

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@NeroBurner

NeroBurner commented Jan 22, 2024

Copy link
Copy Markdown
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

InfiniTime/src/resources/lv_img_conv.py

Line 141 in 84e9425

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 :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@FintasticMan

FintasticMan commented Jan 22, 2024

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ekirchman
ekirchman reviewed Jan 22, 2024
View reviewed changes

@ekirchman ekirchman left a comment

Copy link
Copy Markdown

There was a problem hiding this 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@NeroBurner
NeroBurner merged commit a481af0 into main Jan 23, 2024
@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

Reviewers

@FintasticMan FintasticMan FintasticMan approved these changes
+1 more reviewer
@ekirchman ekirchman ekirchman left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

feature request

Projects

None yet

Milestone

1.15.0

Development

Successfully merging this pull request may close these issues.

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

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@NeroBurner @ekirchman @FintasticMan
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.