GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — lv_img_conv_py: minimal python port of node module by NeroBurner · Pull Request #1863 · InfiniTimeOrg/InfiniTime Conversation
|
|
Build size and comparison to main:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
fa92ff9 to
4edfbae
Compare
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Looks great overall, performance is a lot better as well. Haven't tested on hardware but according to your earlier tests I trust the output
Mostly code cleanliness suggestions/doc suggestions, only architectural one is whether you want to import the file directly or run it as a subprocess
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| uses: actions/checkout@v3 | ||
| with: | ||
| submodules: recursive | ||
| - name: Install python3-pil |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Maybe "Install resource build dependencies" would be better?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
once the docker file is updated this install should be removed as it is already done in the used docker image 🤔
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
renamed as recommended, probably should remove in future 🤔
but keeping it for now to allow a smooth CI until docker image is updated
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
partial fixes, gonna do the rest at a later time
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| uses: actions/checkout@v3 | ||
| with: | ||
| submodules: recursive | ||
| - name: Install python3-pil |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
once the docker file is updated this install should be removed as it is already done in the used docker image 🤔
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'm far from a Python expert, so I trust you on that regard 👍
This scripts makes the setup of the build environment much easier and it works fine on my setup and in the build Docker container so... LGTM :-)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Create a minimal python port of the node.js module `lv_img_conv`. Only the currently in use color formats `CF_INDEXED_1_BIT` and `CF_TRUE_COLOR_ALPHA` are implemented. Output only as binary with format `ARGB8565_RBSWAP`. This is enough to create the `resources-1.13.0.zip`. Python3 implements "propper" "banker's rounding" by rounding to the nearest even number. Javascript rounds to the nearest integer. To have the same output as the original JavaScript implementation add a custom rounding function, which does "school" rounding (to the nearest integer) Update CMake file in `resources` folder to call `lv_img_conf.py` instead of node module. For docker-files install `python3-pil` package for `lv_img_conv.py` script. And remove the `lv_img_conv` node installation. --- gen_img: special handling for python lv_img_conv script Not needed on Linux systems, as the shebang of the python script is read and used. But just to be sure use the python interpreter found by CMake. Also helps if tried to run on Windows host. --- doc: buildAndProgram: remove node script lv_img_conv mention Remove node script `lv_img_conv` mention and replace it for runtime-depency `python3-pil` of python script `lv_img_conv.py`.
Used by script `lv_img_conv.py`, should be provided by docker image, but until then explicitly install in workflow.
b511b93 to
672e305
Compare
|
code review incorporated, squashed and pushed, gonna merge as soon as possible |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
User MorsMortium informed me on Matrix, that there is a full port available at https://github.com/W-Mai/lvgl_image_converter Still using this minimum port, and if we need more we could either switch to the full port or use the full port as inspiration to implement the newly needed parts |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Looks great 👍
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Install new build dependency introduced with move to minimum python implementation of `lv_img_conv` node js script InfiniTimeOrg/InfiniTime#1863
Install new build dependency introduced with move to minimum python implementation of `lv_img_conv` node js script InfiniTimeOrg/InfiniTime#1863
Install new build dependency introduced with move to minimum python implementation of `lv_img_conv` node js script InfiniTimeOrg/InfiniTime#1863
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.