Gallery app #1384
Gallery app #1384
Conversation
|
What about simple markdown support? Just colors, no need for bigger text for titles. Would this be possible? |
|
Colours should be possible, yes. LVGL supports re-colouring. Everything else would be quite difficult I think |
|
That would be great, maybe even with links? Also a button to show/hide formatting? |
|
Now this is an app I want to have! Thank you @yannickulrich 🙂 |
|
Same, will this be in the next version? |
|
I'll make the changes to implement colour using the syntax used in LVGL Implementing links seems quite complicated as it's not directly supported by LVGL as far as I can tell. We could somehow place buttons at the correct positions or implement the touch callback. Either would be quite involved and we would have to essentially re-build the text rendering system. I don't think I understand LVGL well enough to do this but maybe someone else could give it a go..? |
|
With this on watch, experience crashes when changing brightness occasionally, not sure how it causes as also have a sealed watch. App works well though and hasn't crashed while I'm using it. I feel like its todo with more stuff being in the file system making it unstable but more testing needed to confirm that. |
|
I think the best way is to get this merged in a simple first version and then build additional PRs on top of it. I don't want this to be another PR dying over two years because of feature creep discussions. |
|
I'm willing to help finding bugs, how do you use your fork in InfiniSim? I replaced the InfiniTime folder with your fork but there's no gallery icon in the menu |
|
The "Configure and Build" section of InfiniSims readme explains the argument you have to set. You can just link to another directory and don't have to replace something. |
|
@minacode I did build it but it still doesn't show |
|
With |
I think I did |
|
@minacode, I've made the requested changes, thanks! Regarding colouring, we can do this trivially by diff --git a/src/displayapp/screens/FileView.cpp b/src/displayapp/screens/FileView.cpp
index 5026c50e..7d0e3fed 100644
--- a/src/displayapp/screens/FileView.cpp
+++ b/src/displayapp/screens/FileView.cpp
@@ -97,6 +97,7 @@ TextView::TextView(uint8_t screenID, uint8_t nScreens, DisplayApp* app, const ch
lv_obj_t* label = lv_label_create(lv_scr_act(), nullptr);
lv_label_set_long_mode(label, LV_LABEL_LONG_BREAK);
lv_obj_set_width(label, LV_HOR_RES);
+ lv_label_set_recolor(label, true);
lv_obj_align(label, lv_scr_act(), LV_ALIGN_CENTER, 0, 0);
lfs_info info = {0};However, now we can't have a # in the text because LVGL is trying to parse it. We could try to engineer around this, but in the interest of getting this merged quickly (#1384 (comment)) it might be a good idea to postpone this? |
|
I guess one would be more annoyed by a missing # than missing colors. |
| return true; | ||
| } | ||
|
|
||
| int Gallery::StringEndsWith(const char* str, const char* suffix) { |
There was a problem hiding this comment.
This looks like it belongs to some util file and have a more generic name. But maybe it is fine for now.
There was a problem hiding this comment.
I agree with this but I don't think that we currently have util file. Of course I could create one but I wouldn't want to disrupt any plans the maintainers might have. I would argue that this PR should be as minimal as possible, even if that means that code needs to be moved around later on.
There was a problem hiding this comment.
Ok, so I reviewed it. Sorry for the waiting. I am relatively new to C++ and have not done a lot with LVGL so I cannot really say something about the little details in your implementation. I seems fine to me.
But on a more conceptual level I think a separate widget for the horizontal scroll bar, or an extension of the existing vertical scrollbar, would be really nice. I am sure other PRs in the future would profit from this.
|
I added this app to my pinetime and it's really cool! Some quick feedback though:
Thank you for your work! |
|
@JackRaymondCyber how did you managed to try the app? Also, will this be included in the next release? |
@FVellu I merged the changes with the base repo and then followed the build instructions. I've just made my own fork with some extra apps if you would like to try it: https://github.com/JackRaymondCyber/InfiniTimeExtra |
|
Addressing @JackRaymondCyber's comments (thanks!)
This could potentially be improved by caching the images in LVGL though am not sure.
Yes, unfortunately I've also experienced this. No idea why though 😕. Since my device is sealed, I can't debug this efficiently. #560 should be able to help with this but I couldn't get this work either. If someone has an unsealed device and would be willing to poke around a bit, I'd appreciate it greatly!
I don't have any strong opinions either. You could get this behaviour with the following patch diff --git a/src/displayapp/screens/FileView.cpp b/src/displayapp/screens/FileView.cpp
index ec1e9d8e..734baa9c 100644
--- a/src/displayapp/screens/FileView.cpp
+++ b/src/displayapp/screens/FileView.cpp
@@ -62,7 +62,6 @@ ImageView::ImageView(uint8_t screenID, uint8_t nScreens, DisplayApp* app, const
lv_img_set_src(image, path);
lv_obj_align(image, lv_scr_act(), LV_ALIGN_CENTER, 0, 0);
- ShowInfo();
}
TextView::TextView(uint8_t screenID, uint8_t nScreens, DisplayApp* app, const char* path, Pinetime::Controllers::FS& fs)
@@ -98,5 +97,4 @@ TextView::TextView(uint8_t screenID, uint8_t nScreens, DisplayApp* app, const ch
fs.FileClose(&fp);
- ShowInfo();
} |
|
Will this be merged any time soon? |
This is probably caused by the relatively slow SPI bus that is shared with the external memory and the LCD. There's not much we can do here, except trying to keep the files as small as possible.
It seems that there's at least one crash issue with this app that will need to be diagnosed and fixed before we merge the feature. Then, it'll depend on the availability of us maintainer to review and eventually merge the branch. |
This reverts commit 8644937b11150c37cd9269759ebcfcc3a83ebb67.
Thanks to @IchbinkeinReh for pointing this out
This is really slow on the watch
Following 7c7a860
yannickulrich
force-pushed
the
dev-imageview
branch
from
7880083 to
c4e6d79
Compare
|
I have rebased everything to |
|
I would very much appreciate if this came out in the next version. I'm loving infinitime, but this would make it way more useful for me. |
|
@JF002 Please merge because adding a gallery app to InfiniTime, the open source FreeRTOS-based OS for smartwatches, could benefit the community in the following ways:
So in summary, a contributed gallery app would bring a commonly expected feature, demonstrate InfiniTime's capabilities, attract new users, grow the dev ecosystem, enable improvements, support customizations, and potentially improve security - benefitting the entire open source community. |
|
I don't think that this PR isn't merged because of a lack of good arguments. |
|
@minacode, do you have any idea what is holding this up? Are the maintainers just overworked or is there a specific problem I can address? |
|
No, sadly not. I guess it's a time issue. See how many PRs are open and get merged by a few people (thanks 🙏🏼). Currently there is a bug issue from the 1.13 release that will surely get a lot of the attention. Apart from that there is only little memory left, so merges have to be done wisely. I am tinkering with optional apps for some time now to hopefully improve this someday. I would like to see my calculator get merged as well and I think the best way to get there is to improve the whole situation around apps. |
|
Update: #1894 was merged 🥳 |
Gallery application
With this application you can add image and text files to the watches FS and view them on the watch (#1237). You can have multiple files on the watch by copying them into the directory
/gallery. This would allow for example to have QR codes (or other barcodes such as Aztec codes which are often used for airline tickets) on the watch by creating them in the companion app (related #181). Further, you can have text files on the watch that will just be shown.Usage
You first need to make the directory
/gallery. Using eg.itctl$ itctl fs mkdir /gallery # ensure folder existsFiles with the suffix
.binare treated as images and passed to lvgl for rendering. Files with suffix.txtwill be shown as text files.Image files need to have the correct size (max 240x240) and be converted using
lv_img_convand can then be uploaded using eg.itctl$ lv_img_conv code.png --output-file code.bin \ --color-format CF_TRUE_COLOR \ --output-format bin \ --binary-format ARGB8565_RBSWAP # convert image $ itctl fs write code.bin /gallery/code.bin # upload imageYou can navigate between the files by swiping left and right. Long pressing the screen hides the file name and navigation bar.
Screenshots
Status
The app works well in the simulator and I have started testing it on a physical device. However, only test this on a sealed watch if you know what you are doing!
I'd appreciate any input but especially concerning memory leaks.