-
-
Notifications
You must be signed in to change notification settings - Fork 968
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
Auto generate font #1097
Auto generate font #1097
Conversation
@Riksu9000 i'd like to get your thoughts about whats already here. thanks! |
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 like this solution. The README should be updated as a part of this PR by removing the old instructions and writing something about this new method.
Also, removed feature existance cheking (since it now depends on a font, so may end up being inside (only) a font not being used currently - which is an allowed usage)
797c74d
to
ebe42a4
Compare
ebe42a4
to
5a16e1b
Compare
and now the font .c files are generated at compile time with CMake. |
I've been thinking that the font "features" probably isn't going to be very useful to us, since we only ship a single firmware, and that's likely not going to be changing. The other use case would be if fork maintainers pushed their changes in this repo, which I don't think should be our responsibility to handle either. |
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.
Thank you for this PR, that's definitely something I want to add tot he project!
The generation of the fonts in CMake and then in Python and in Github Actions look good to me!
If you don't mind, I think we should make 2 addition:
- Add
lv_font_conv
(and how to install it vianpm
) as a needed dependency to build the project indoc/buildAndProgram.md
as the project won't build without it. - Add the installation of
npm
andlv_font_conv
in the Dockerfile so we can build the fonts inside the docker container. Note than I plan on switching the CI to docker so that we won't have to duplicate those changes in the future ([WIP] Github Action with docker to build the firmware #1045). - Oh and also maybe specify which version of
lv_font_conv
we support, and enforce this version in Docker/Action workflows to avoid broken builds because of a change in the tool ?
I can help with those points if you want to ;)
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 noticed that changing the font configuration will regenerate all the fonts, even if only one was changed. Not a big issue, but maybe worth considering if something can be done about that.
Added a bit of changes here (use CMAKE_CURRENT_LIST_DIR to reach things, instead of assuming paths), |
Also, I did (except when I forgot once) compile before pushing, and here watching the checks status, so it's weird that the simulator check succeeded. so I had a look, and in the simulator workflow here I manually generated the scripts - I just removed it, so it will fail here too (until InfiniTimeOrg/InfiniSim#28 is merged). |
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.
This looks totally fine to me!
There's just 1 last issue : the build in the docker container does not work properly. See this log message:
[ 35%] Generating displayapp/fonts/jetbrains_mono_42.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_76.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_76.c
[ 35%] Generating displayapp/fonts/lv_font_navi_80.c
[ 35%] Generating displayapp/fonts/open_sans_light.c
[ 35%] Generating displayapp/fonts/open_sans_light.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_extrabold_compressed.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_extrabold_compressed.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_bold_20.c
[ 35%] Generating displayapp/fonts/jetbrains_mono_42.c
[ 35%] Generating displayapp/fonts/lv_font_sys_48.c
[ 36%] Generating displayapp/fonts/jetbrains_mono_bold_20.c
[ 36%] Generating displayapp/fonts/lv_font_navi_80.c
File " File "/sources/src/displayapp/fonts/generate.py/sources/src/displayapp/fonts/generate.py", line ", line 21
21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
File "/sources/src/displayapp/fonts/generate.py File "/sources/src/displayapp/fonts/generate.py", line 21
File " def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
/sources/src/displayapp/fonts/generate.py def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
File " File "/sources/src/displayapp/fonts/generate.py", line /sources/src/displayapp/fonts/generate.py21", line
21 def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
File "/sources/src/displayapp/fonts/generate.py", line ", line 21
21 def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
^
SyntaxError : invalid syntax
^
SyntaxError ^
SyntaxError ^
File "SyntaxError : invalid syntax/sources/src/displayapp/fonts/generate.py
", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
: def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
invalid syntax
: invalid syntax
^
^
SyntaxError^
: SyntaxErrorinvalid syntaxSyntaxError:
: File "invalid syntaxinvalid syntax
/sources/src/displayapp/fonts/generate.py
", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
File " /sources/src/displayapp/fonts/generate.py ", line 21
^
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
^
SyntaxError: invalid syntax
File "/sources/src/displayapp/fonts/generate.py", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
^
SyntaxError: invalid syntax
^
SyntaxError: invalid syntax
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:65: recipe for target 'src/displayapp/fonts/jetbrains_mono_extrabold_compressed.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_extrabold_compressed.c] Error 1
make[2]: *** Waiting for unfinished jobs....
src/CMakeFiles/pinetime-app.dir/build.make:73: recipe for target 'src/displayapp/fonts/jetbrains_mono_76.c' failed
SyntaxErrormake[2]: *** [src/displayapp/fonts/jetbrains_mono_76.c] Error 1
: File " [ 36%] Generating displayapp/fonts/lv_font_sys_48.c
invalid syntaxmake[2]: *** Waiting for unfinished jobs....
/sources/src/displayapp/fonts/generate.py
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:73: recipe for target 'src/displayapp/fonts/jetbrains_mono_76.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_76.c] Error 1
", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
src/CMakeFiles/pinetime-app.dir/build.make:77: recipe for target 'src/displayapp/fonts/jetbrains_mono_42.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_42.c] Error 1
^
SyntaxError : invalid syntax
^
SyntaxError: invalid syntax
src/CMakeFiles/pinetime-app.dir/build.make:65: recipe for target 'src/displayapp/fonts/jetbrains_mono_extrabold_compressed.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_extrabold_compressed.c] Error 1
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:77: recipe for target 'src/displayapp/fonts/jetbrains_mono_42.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_42.c] Error 1
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:61: recipe for target 'src/displayapp/fonts/lv_font_navi_80.c' failed
make[2]: *** [src/displayapp/fonts/lv_font_navi_80.c] Error 1
src/CMakeFiles/pinetime-app.dir/build.make:69: recipe for target 'src/displayapp/fonts/jetbrains_mono_bold_20.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_bold_20.c] Error 1
src/CMakeFiles/pinetime-app.dir/build.make:85: recipe for target 'src/displayapp/fonts/open_sans_light.c' failed
make[2]: *** [src/displayapp/fonts/open_sans_light.c] Error 1
src/CMakeFiles/pinetime-app.dir/build.make:81: recipe for target 'src/displayapp/fonts/lv_font_sys_48.c' failed
make[2]: *** [src/displayapp/fonts/lv_font_sys_48.c] Error 1
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:85: recipe for target 'src/displayapp/fonts/open_sans_light.c' failed
make[2]: *** [src/displayapp/fonts/open_sans_light.c] Error 1
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:69: recipe for target 'src/displayapp/fonts/jetbrains_mono_bold_20.c' failed
make[2]: *** [src/displayapp/fonts/jetbrains_mono_bold_20.c] Error 1
src/CMakeFiles/pinetime-app.dir/build.make:61: recipe for target 'src/displayapp/fonts/lv_font_navi_80.c' failed
make[2]: *** [src/displayapp/fonts/lv_font_navi_80.c] Error 1
CMakeFiles/Makefile2:409: recipe for target 'src/CMakeFiles/pinetime-app.dir/all' failed
make[1]: *** [src/CMakeFiles/pinetime-app.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
File "/sources/src/displayapp/fonts/generate.py", line 21
def gen_lvconv_line(dest: str, size: int, bpp: int, sources: typing.List[Source], compress:bool=False):
^
SyntaxError: invalid syntax
src/CMakeFiles/pinetime-mcuboot-app.dir/build.make:81: recipe for target 'src/displayapp/fonts/lv_font_sys_48.c' failed
make[2]: *** [src/displayapp/fonts/lv_font_sys_48.c] Error 1
CMakeFiles/Makefile2:214: recipe for target 'src/CMakeFiles/pinetime-mcuboot-app.dir/all' failed
make[1]: *** [src/CMakeFiles/pinetime-mcuboot-app.dir/all] Error 2
I think this is caused by the version of Python : the docker image uses Python 2.7.17, while my computer is running Python 3.10.
Do you think it would cause any issue if we called python3
instead of python
in the cmake command that generates the font?
fe3fe01
to
24b7950
Compare
@yehoshuapw I've just tried the new docker image based on ubunutu 20.04, and I still got the same issues with the Python script.
I'm not sure why it works for you and not for me... Did I miss anything? PS : Please excuse me for the time it takes to finish this review... Those little details about Docker and the CI are taking more time than I expected... |
Ubuntu 22.04 is released and it has a package called |
Yes, probably! As long as it still builds the firmware and generates the DFU images, that fine for me :) EDIT : this seems to work:
|
I must have made some mistake (left the python2 version? left .c files in the build dir?), because indeed the image I built here does have the "python" version as 2.7. updated as you suggested, and seems to work. (building the image and then infinitime, from the start) (I'm also wondering if any dist except for arch (which I don't even use) doesn't have a "python3" anymore without installing some extra things. nonetheless having a newer version which works tends to be worth it) |
On Manjaro, I still have python and python3, but since python 3 is support to replace python 2, it's probably better to assume that python is Python 3
Thanks, that looks very good to me :) |
Now... How do we handle that in InfiniSim ? |
I opened a PR there too, InfiniTimeOrg/InfiniSim#28 which seems to work. |
I think nearly identical to here. Need to install npm and the lv-font generator in the CI. And need to add those generated fonts to InfiniSim. I'll need to have a thorough look at this PR and see how and where to fonts.c files are generated. Maybe I can make the font-generating CMakeLists.txt file generic enough so I just need to do |
in
I'm new at CMake, so go for it - and if you want to make any edits here, you are welcome to. |
I'll merge this PR as it's working as expected in InfiniTime. We'll follow up with the integration in InfiniSim in anothe PR (probably in the InfiniSim project).
It should be possible to create a 'font' target or a GenerateFont() function that we'll be able to include in InfiniSim, so we don't have to duplicate the whole code needed to generate the fonts. @yehoshuapw I'm happy to see you've already started work in InfiniSim too! Thanks again for this nice PR! 🥇 |
In InfiniTimeOrg#1097 new font generation capabilites were added. Generalize the font creation to make it possible to reuse the `displayapp/fonts/CMakeLists.txt` file for `InfiniSim` and just add the new cmake file to the project and link against the new `infinitime_fonts` target. In the following a list of changes. Allow non-global installed `lv_font_conv` executable installed with ```sh npm install lv_font_conv@1.5.2 ``` In CMake we search for `lv_font_conv` executable. Add the found executable to the python script `generate.py`, to remove the need for `lv_font_conv` to be in the path. Search for `python3` executable, if CMake version 3.12 is available. Otherwise use `python` as hard coded executable. Instead of adding the generated fonts to `SOURCE_FILES` variable, create a static library `infinitime_fonts` instead. Link this library to the executables instead.
Since InfiniTimeOrg/InfiniTime#1097 the fonts are generated. Support this new way of using fonts.
In InfiniTimeOrg#1097 new font generation capabilites were added. Generalize the font creation to make it possible to reuse the `displayapp/fonts/CMakeLists.txt` file for `InfiniSim` and just add the new cmake file to the project and link against the new `infinitime_fonts` target. In the following a list of changes. Allow non-global installed `lv_font_conv` executable installed with ```sh npm install lv_font_conv@1.5.2 ``` In CMake we search for `lv_font_conv` executable. Add the found executable to the python script `generate.py`, to remove the need for `lv_font_conv` to be in the path. Search for `python3` executable, if CMake version 3.12 is available. Otherwise use `python` as hard coded executable. Instead of adding the generated fonts to `SOURCE_FILES` variable, create a static library `infinitime_fonts`. Link this library to the executables instead. Use `add_custom_target()` together with `add_custom_command()` to generate the font.c files once (like the original PR does).
Since InfiniTimeOrg/InfiniTime#1097 the fonts are generated. Support this new way of using fonts. When InfiniTime isn't a subdirectory of InfiniSim we need to provide the binary dir when using `add_subdirectory()`. Use `fonts`. Update README with instructions to install `lv_font_conv`.
Since InfiniTimeOrg/InfiniTime#1097 the fonts are generated. Support this new way of using fonts. When InfiniTime isn't a subdirectory of InfiniSim we need to provide the binary dir when using `add_subdirectory()`. Use `fonts`. Update README with instructions to install `lv_font_conv`.
In #1097 new font generation capabilites were added. Generalize the font creation to make it possible to reuse the `displayapp/fonts/CMakeLists.txt` file for `InfiniSim` and just add the new cmake file to the project and link against the new `infinitime_fonts` target. In the following a list of changes. Allow non-global installed `lv_font_conv` executable installed with ```sh npm install lv_font_conv@1.5.2 ``` In CMake we search for `lv_font_conv` executable. Add the found executable to the python script `generate.py`, to remove the need for `lv_font_conv` to be in the path. Search for `python3` executable, if CMake version 3.12 is available. Otherwise use `python` as hard coded executable. Instead of adding the generated fonts to `SOURCE_FILES` variable, create a static library `infinitime_fonts`. Link this library to the executables instead. Use `add_custom_target()` together with `add_custom_command()` to generate the font.c files once (like the original PR does).
closes #964 (supersedes)
this adds a script
src/displayapp/fonts/generate.py
to generate the font files (using lv_font_conv) from a singular config file (src/displayapp/fonts/fonts.json
).the json has each font, and it's relevant options (size,bpp,compress, and "patches") and sources.
in addition there's also a "feature" feature, allowing extra sources with a extra flag (and the config has an example usage of adding hebrew, which won't do anything unless
-e hebrew
is given)currently, this manages to generate exactly the same as what is already in place.
TODO:
integrate this into the CMake (and add lv_font_conv to the build images).
(my CMake-fu is not that strong, so I'll get to that tomorrow - but opening this to get comments about the rest of it from anyone interested)