lvgl / lvgl

Embedded graphics library to create beautiful UIs for any MCU, MPU and display type.
https://lvgl.io
MIT License
16.66k stars 3.26k forks source link

indev callback not workming correctly #6771

Closed kdschlosser closed 1 month ago

kdschlosser commented 2 months ago

LVGL version

master branch

What happened?

When an indev callback is made there is a structure that gets passed and that structure is supposed to get filled by the callback function is there has been user input. There is a field in that structure names continue_reading. This flag causes the indev code to continually loop reading the input over and over again. That part works as it should but what is missing from it is updating the display. It loops without ever doing a display refresh.

I would have imagined the purpose of that field is to offer better performance when scrolling or moving a slider. Issue is if the display doesn't get updated it simply jumps once the touch is released. I am not sure if the continue_reading flag had some other purpose and if it does we can tweak the API slightly so it's purpose doesn't change, We add a bool return value form the callback function. Returning true would instruct LVGL to refresh the display immediately processing the data from the callback. It doesn't matter if there is a change to the user input or not. The continue reading would function as it should. This would only apply if there is some use case for continue reading where the indev driver should be running in a loop without ever updating the display.

There needs to some way to continually loop the indev otherwise it is only going to get called once every 33 milliseconds and that is not going to provide the best in terms of smooth scrolling. we don't want to return until the user has released the display. having a way to control this aspect of LVGL give the user the ability to made a decision as to what is important to them. Personally I hate it when a UI doesn't respond quickly when scrolling or moving a slider.

I personally like the idea of of being able to control the loop and also the refresh separately and using a return value is a simple way to achieve that.

How to reproduce?

No response

kisvegabor commented 2 months ago

The purpose of continue_reading is to make LVGL read out all the buffered data (if any). E.g. if you have 3 data buffered, you can set continue_reading twice and leave it false on the last call.

For the behaviour you have described you can call lv_timer_ready(lv_indev_get_read_timer()); in the read_cb but note that, the coordinate and speed calculations are not considering the read period (which is a problem and should be fixed at some point).

For example if previously y way 100 and now it's 110 LVGL thinks the your finger moved 10px. However within what time frame? 10ms? 20ms? 5 sec? They all result in different speed and result in different scroll speed too.

So for now, be careful with pushing new coordinates in random time frames.

kdschlosser commented 2 months ago

If I am looking to loop to remove buffered data why would it not just be done in the callback function for the indev? what would be the purpose of setting continue_reading other than consuming resources to make the loop? it can be done using a for or a while loop in the callback. This is why I had thought the purpose of the continue_reading was to provide a much better user experience when doing things like scrolling. by running in a loop for the indev when there is touch it removed all other aspects of the program that would normally run to give priority to use interaction and display updates based on that interaction. having to come full circle in the main thread could take a considerable amount of time depending on the application and I have found this does 100% impact things like scrolling.

Take a look at this.. This is a 320x480x16 display using an SPI connection running MicroPython. I had altered the reading of the indev to provide priority to user input and updating the display above any other running code. I am not incrementing the tick to do this. All that is being done if is continue_reading is set after the input has been processed lv_refr_now gets called. That's it.

https://youtu.be/35j4p78OJAs?t=24

This is running on a single core of of an ESP32. It is not an S3. It is the bottom of the pile ESP32. IDK what you think bit I can tell you that it is really responsive to input and it is really seen when scrolling. I wish I had a video of what it's like without it looping like that.

There is nothing that can be done from inside the indev callback to provide this behavior because the touch input needs to be processed by LVGL before doing the refresh and that is only going to happen after the function returns.

kisvegabor commented 2 months ago

What about using an event driven indev?

kdschlosser commented 2 months ago

not all touch screens have interrupts that work properly plus they are an interrupt so nothing is able to be done in the ISR. It has to be done in the same thread that LVGL is running in. you end up back at square one when going that route The point to it is to stop the processing of all other code except for the user input and updating the display. interrupts only stop the code from running for a moment and then you have to wait for the main thread to come full circle to pick up that there is touch input. It gets messy dealing with that in user land when all of the mechanics are already in place in LVGL. it is just a matter of a return value from the callback to trigger a display refresh. I know you mentioned the tick but that is not so much an issue because most people handle that in either a FreeRTOS task or by using an ISR so the tick gets incremented.

kisvegabor commented 1 month ago

Hmm, I still don't understand something. Let's say the display refresh period is 16 ms (60 FPS). So LVGL will read the indev and update the display in every 16ms like this:

0ms: render (takes 10ms)
10ms: indev read

16ms: render (takes 3ms)
26ms: indev read 

32ms render (takes 8ms)
42ms: indev read 

So render is always followed by an indev read in < 16 ms. How could it be improved further?

kdschlosser commented 1 month ago

in order for LVGL to read input from the indev it is going to happen once every 33 milliseconds right? It's on a timer. so it will read input and update LVGL and the task handler will exit and allow user code to run. we don't know how long it is going to take for that user code to run. It could be 1 millisecond or it could be 50 milliseconds. then the task handler will get called again after incrementing the tick, so the minimum time between indev reads at a minimum is going to occur once every 33 milliseconds.

by having the loop with the indev reading it would update LVGL and refresh the display and then go right back to reading the indev. It would not exit the loop until it was told to by the indev callback which would be when there is no touch input. This places priority on the GUI performance and user input. That behavior would be easily user changeable by setting the continue flag in the indev data structure to false instead of True.

I am proposing a return value be added for the indev read callback. That return value is what would instruct the indev processing code to update LVGL and then refresh the display after processing the user input.. The continue_reading flag would work exactly how it does right now where it would continue to read the indev.

You mentioned the time being increments. Most of the time this is done using an interrupt or using a FreeRTOS task or a thread, whatever it is the tick is being incremented like it should be regardless of the looping indev code.

Let me make up a PR that you can test it out with on an MCU..

kdschlosser commented 1 month ago

6840

It's not 100% completed and will more than likely fail the CI build. You can look at the code to see the general idea of what I am talking about. It will be easier to understand if you see the code.

Just keep in mind the user code that is going to be running as part of the main loop and not knowing how long that code is going to take to run which will have a direct impact on how the UI responds to user input. This code will remove that user code from impacting user input response times.

It also negates the 33 millisecond default for when the indev gets read. It only does this if there is continued user input otherwise the default timeout period applies. This will greatly improve things like scrolling.

lvgl-bot commented 1 month ago

We need some feedback on this issue.

Now we mark this as "stale" because there was no activity here for 14 days.

Remove the "stale" label or comment else this will be closed in 7 days.

lvgl-bot commented 1 month ago

As there was no activity here for a while we close this issue. But don't worry, the conversation is still here and you can get back to it at any time.

Feel free to comment if you have remarks or ideas on this topic.