Long pressing paint app moves the screen

[ShimonHoranek]

Verification

• [X] I searched for similar bug reports and found none was relevant.

What happened?

When long the paint app is long pressed it moves the screen a bit up and shows content under

What should happen instead?

It should open the app

Reproduction steps

Scroll through the app menu and long press the paint app

More details?

Version

1.8.0

Companion app

No response

[NeroBurner]

can reproduce on latest develop commit https://github.com/InfiniTimeOrg/InfiniTime/commit/7e0b053b38d0491eb4ee666be34d0ea2ab029d19

Can't reproduce on Simulator

The mysterious thing is that the screen stays moved just a little up until the pinetime is restarted.

This up-movement doesn't happen on long-pressing other apps

[SteveAmor]

It's like it starts painting on the canvas before it scrolls the previous screen fully out of the way. You always get the single dot in the top left where the paint icon was that you long pressed. On launching paint, perhaps checking there is no touch before starting to paint would prevent this. I suspect it is more the screen hardware scroll than lvgl.

[SteveAmor]

Linked to #445

[SteveAmor]

Essentially, if the display is on and while a hardware, vertical, scroll is in progress, if you write to the display, it stops the scrolling and you end up with a displaced display until the PT is rebooted.

I believe that the fix should be to only update the display if there is no hardware scrolling in progress (or hardware scrolling has finished). I have no idea if there is a flag available to test for this condition.

[Gernomaly]

can reproduce on latest develop commit 7e0b053

Can't reproduce on Simulator

The mysterious thing is that the screen stays moved just a little up until the pinetime is restarted.

This up-movement doesn't happen on long-pressing other apps

Well, as far as I am aware, there is no other app with an "extra feature" which uses a long tab. So the problem is probably there and with that explanation, it solves the mystery, why no other app has this problem. (As it was explained above)

[JF002]

That's an interesting bug :-) Here are my findings !

When a new app is open (ex : from the menu to the InfiniPaint app): https://github.com/InfiniTimeOrg/InfiniTime/blob/develop/src/displayapp/DisplayApp.cpp#L332

• The previous app (app menu) is closed by called its destructor
• LVGL is configured to apply the vertical scrolling transition
• The new app (InfiniPaint) is created, meaning that the constructor of InfiniPaint (https://github.com/InfiniTimeOrg/InfiniTime/blob/develop/src/displayapp/screens/InfiniPaint.cpp#L13) is called.
• Then, DisplayApp::LoadApp returns into DisplayApp::Refresh() which continues its execution.
• During the next call to DisplayApp::Refresh() (it's called into an infinite loop), lv_task_handler() is called. This function handles the whole LVGL processing. The display is refreshed and scrolled in this function.

Most of the time, it works fine, the transition is applied, the new app is displayed and the old one is closed.

But, in this case, InfiniPaint handles the LongTap gesture. Here's what happens when the app is opened with a long tap:

• The menu app is destroyed
• LVGL is configured to apply the vertical scrolling transition
• InfiniTime is created
• It immediately responds to the LongTap event, which calls FlushDisplay, the LVGL driver for the display. As LVGL was previously configured to apply the transition, a first scroll of a 10 lines is applied (because InfiniPaint writes a 10x10 square).
• Then LVGL refreshes the display and scrolls 240 lines
• In the end, the scroll was a 250 lines scroll instead of a 240 lines scroll.

I think the main issue is that InfiniPaint bypasses LVGL by calling the display driver directly. This is not how we're supposed to use LVGL. In fact, InfiniPaint was designed as a simple test app for the touchpanel, and I decided to include it in InfiniTime because I thought it was fun but yeah... it conflicts with LVGL.

Now, I might have found a fix:

In DisplayApp::Refresh() swap the touch management with the loading of a new app. This way, we ensure that LVGL will have the opportunity to refresh the whole display before a touch event is sent to the app.

Now, I'm not sure why the loading was done before the touch management, and there might be a good reason for that so I don't know yet if we can safely apply this patch.