espruino / BangleApps

Bangle.js App Loader (and Apps)
https://banglejs.com/apps
MIT License
495 stars 1.16k forks source link

settings , top of menu corrupted on scrolling long menus #2393

Closed hughbarney closed 1 year ago

hughbarney commented 1 year ago

Affected hardware version

Bangle 2

Your firmware version

2.16.33

The bug

Bug description

When scrolling up and down menus in settings the top part of the menu overflows the top of the screen and does not get redrawn properly.

Seems to happen when I swipe up and over the top of the screen. Screenshots included below. Settings 0.55

The widget area gets cleared agian when the lock widget kicks in. Can take up to 20 swipes before the widget area get corrupted. When on the Settings/Utils menu , the menu items seem to become like a circular menu, with the first menu item joining the last when swiping from the bottom to top.

download (1) download (2)

gfwilliams commented 1 year ago

I can't seem to reproduce this one here :(

Please could you try and do another video? It might also help if you added:

g._scroll = g.scroll;
g.scroll = function(x,y) {
  console.log("Scroll",x,y);
  return g._scroll(x,y);
};

and then try it, and maybe when the issue happens you can post up the list that appeared on the console and roughly which one you think the corruption occurred at.

hughbarney commented 1 year ago

Tried that code it did not print anything at all. Tried it a few times in the left hand side of the IDE.

I made a video but it was 6 minutes long ~690MB :( You might be best to download it, rather than try and stream it. https://www.dropbox.com/s/ke3pis12lmsxjel/VID_20221213_211411.mp4?dl=0

I have reproduced it on 2 Banges, both running 2.16.34.

Also found a new way to reproduce, this time using the standard launcher and just scroll down to the bottom then up again and down again a few times. Within 10 repetitions you will see something like the attached screenshot.

It feels like it is something to do with the widget area handling.

This observation may / may not be related. I have noticed inconsistant beahviour with the icon launcher with the display of widgets with icon launcher in full screen mode. This depends on what clock is loaded ( a fast load clock or a non fast load clock) the widgets will display. For example select Pastel from the icon launcher, launch the launcher from the button, no widgets displayed, now select Anton clock and display the clock. When you press the button for the launcher the widgets are now displayed. The difference is whether the clock is a fast load clock or not. This might be a different defect. 2 screenshots also attached.

download

download (3)

download

gfwilliams commented 1 year ago

Thanks for the video!

I have noticed inconsistant beahviour with the icon launcher with the display of widgets with icon launcher in full screen mode. This depends on what clock is loaded

This does sound like a different issue. Should the widgets be loaded for the icon launcher normally? I think most likely it's a bug in whatever clock that fast load was enabled for - it probably doesn't call require("widget_utils").show() on the remove handler if it also hid the widgets first.

It's something I was afraid of - everyone rushes to add fast load to their apps, but then they don't get tested properly and we end up with inconsistent behaviour :( If it's the case that widgets should be shown, and then they're hidden after fast-loading from a clock, please can you file an issue for that clock?

With this code - I just tried it again and it displays stuff fine for me:

g._scroll = g.scroll;
g.scroll = function(x,y) {
  console.log("Scroll",x,y);
  return g._scroll(x,y);
};

Looking at your video, I think you need to run the code when you are in the launcher/app you wanted to test as it'll get removed when switching apps - so navigate to settings->utils and then run it (but just once - doing more than once would break it).

I also notice that you have a widget in the top left - step count? I'm not sure if you have other non-standard things there too.

Please could you try a Bangle that's reset to the factory-default apps and see if it happens? I've been trying here for ages and I just can't get the corruption you see, so maybe it's the combination of apps/widgets?

@BartS23 do you see this happening on yours after you update?

BartS23 commented 1 year ago

The crash/hang as reported in #2382 no longer exists.

The drawing in the upper area is still there.

Tested with 2v16.34 after a factory reset and without applying an update and after all apps were updated.

Steps to follow:

gfwilliams commented 1 year ago

Wow, thanks again @BartS23!

I've been trying for ages to reproduce by dragging, but this does it perfectly! I'll try and see what's going on.

Interestingly g.setClipRect(0,24,175,175).scroll(0,-170) which would have reproduced before doesn't cause any problems, so I think this one might be scroller related

gfwilliams commented 1 year ago

Ok, great - should now be fixed with https://github.com/espruino/Espruino/commit/2aa394040ca531b53d40cb7ff9fe8fb2b2a2037a

Just out of interest, since I can't reproduce by scrolling, I was trying to think of how you could get to a stage where you can scroll pretty much the screen's entire height in one go.

And I was wondering, if you try and scroll smoothly, does it 'stutter'? It might be something you have installed is taking a long time to execute ( ~100ms?) and that's enough that your finger can get all the way bottom->top before the next draw call?

BartS23 commented 1 year ago

@gfwilliams I can confirm the fix.

I have been thinking about your thoughts. The problem is probably to be found in a completely different place.

To bring y to 248 on a "Bangle JS 2" you have to be pretty special. :)

gfwilliams commented 1 year ago

Interesting - thanks! Have you ever done a touchscreen calibration (via Settings?)

I can't seem to do this on mine, which might explain why I can't get this to happen! I'm not sure how we'd see, but I wonder if this is actually in the value reported by the touch controller

BartS23 commented 1 year ago

It also happens after a calibration.

In jswrap_bangle.c it says that x or y can be negative. Could it be that there is a negative rollover here and it should actually be -8?

I tried a bit with the different firmwares and found that since 2v15.45 (https://github.com/espruino/Espruino/commit/94170863aed7464f707252999f0b5e94012db69c) the value is > 180. Before that, however, there were also large values when swiping from the top.

gfwilliams commented 1 year ago

Well, we did have a problem before, but I really thought I'd sorted that. But values just below 256 sure look like a rollover to me.

Just in case, I have just made a change: https://github.com/espruino/Espruino/commit/8c83923bb8928e8ec441c4a5f1a72d33acdd5343

I'd be interested to see if the latest builds fix it for you

BartS23 commented 1 year ago

The last commit changed nothing.

The 248 is apparently just a random thing. When I filter the events (>250) and swipe several times, I get values up to 280, which is far from a rollover.

Tested with 2v16.35, 2v16.36 and 2v16.37

I think with the visible effect fixed, we should leave it at that, or what do you think?

gfwilliams commented 1 year ago

It'd be nice to find out why though... When we get these big scroll amounts it'll cause the whole screen to be redrawn which'll make things a bit more jerky/flickery than they should be

Please could you post require("Storage").readJSON("setting.json").touch for your Bangle? The coordinates do appear after they've been tweaked with the calibration so they could end up appearing that big if the rollover happened before.

I've also just had another dig into the touch controller, and it does report back 12 bit coordinates, but I was only ever using 8 bits because originally that's all the touch IC reported.

However I think the touchscreen firmware has changed (nobody ever complained about needing Touchscreen calibration before, and recently they have) so it's possible the touchscreen can now report >255, which would have really screwed things up.

... So if you try the absolute latest firmware, maybe it'll be better?

hughbarney commented 1 year ago

ITS FIXED !

I have tested 2v16.39 and can no longer reproduce. Thank you for all the great work you guys have done on this and the hang issue 2382.

When I was trying to get clock_info and fast load to work these issues put a lot of doubt in my mind that I had coded things wrong. I had to strip right back to basics to narrow these down. Both were nasty bugs and I am so glad my Bangle 2 feels stable again.

This is my main smart watch now I dont bother with my AmazFit ones anymore.

Have you ever done a touchscreen calibration (via Settings?)

Yes I had and I was really pleased to see that this option had been added and the old Calibration App had been marked as Not recommended. I would much rather use the supported way of doing things.

hughbarney commented 1 year ago

The difference in behaviour in Icon Launcher is still there. I will open a seperate thread for that as I am not sure what the expected behaviour is now.

BartS23 commented 1 year ago

It'd be nice to find out why though... When we get these big scroll amounts it'll cause the whole screen to be redrawn which'll make things a bit more jerky/flickery than they should be

Please could you post require("Storage").readJSON("setting.json").touch for your Bangle? The coordinates do appear after they've been tweaked with the calibration so they could end up appearing that big if the rollover happened before.

I've also just had another dig into the touch controller, and it does report back 12 bit coordinates, but I was only ever using 8 bits because originally that's all the touch IC reported.

However I think the touchscreen firmware has changed (nobody ever complained about needing Touchscreen calibration before, and recently they have) so it's possible the touchscreen can now report >255, which would have really screwed things up.

... So if you try the absolute latest firmware, maybe it'll be better?

Tested with 2v16.40 on factory defaults

I get values up to 280

After calibration require("Storage").readJSON("setting.json").touch = { x1: 5, y1: 2, x2: 183, y2: 179 } the values are limited to 251.

gfwilliams commented 1 year ago

Thanks! That is interesting...

So, it looks a lot like the value reported back is/was 255. But I can't seem to reproduce it on mine at all

Please could you run the attached build on yours. It'll print lines like T 3 1 128 13 0 102 when the touchscreen is touched.

If you could get it to output those high values again, and send me the 'T' values reported around that time, that would be amazingly helpful.

espruino_2v16.40_banglejs2.zip

It's possible that it might be a type of message that I'm not handling properly that I could then filter out.

BartS23 commented 1 year ago

Here is the output:

 ____                 _
|  __|___ ___ ___ _ _|_|___ ___
|  __|_ -| . |  _| | | |   | . |
|____|___|  _|_| |___|_|_|_|___|
         |_| espruino.com
 2v16.40 (c) 2021 G.Williams
>
>T 0 1 0 86 0 254
Y: 250
>T 0 1 128 86 0 254
T 0 1 128 87 0 1
T 1 1 128 87 0 56
T 1 0 64 87 0 56
T 0 1 0 29 0 254
Y: 250
>T 0 1 128 29 0 254
T 0 1 128 29 0 254
T 0 0 64 29 0 254
Y: 250
>T 5 0 0 29 0 254
T 0 1 0 120 0 251
Y: 247
>T 0 1 128 120 0 251
T 0 1 128 121 0 1
T 0 1 128 122 0 130
T 0 1 128 122 0 145
T 1 1 128 122 0 152
T 1 1 128 122 0 156
T 1 1 128 122 0 158
T 1 1 128 122 0 158
T 1 0 64 122 0 158
T 0 1 0 138 0 252
Y: 248
T 0 1 128 138 0 252
T 0 1 128 138 0 1
T 0 1 128 138 0 90
T 0 0 64 138 0 90
T 5 0 0 138 0 90
T 0 1 0 43 0 255
Y: 251
T 0 1 128 43 0 255
T 0 1 128 43 0 255
T 0 0 64 43 0 255
Y: 251
T 5 0 0 43 0 255
T 0 1 0 63 0 252
Y: 248
T 0 1 128 63 0 252
T 0 0 64 63 0 252
Y: 248
T 5 0 0 63 0 252

The lines starting with Y are the output from Bangle.on("drag", event => { if (event.y > 200) console.log("Y:", event.y); });

By the way, I did not get any more values above 251. even after another factory reset.

BartS23 commented 1 year ago

I did another try:

>T 0 1 0 39 0 251
Y: 247
>T 0 1 128 39 0 251
T 0 1 128 37 0 1
T 0 1 128 37 0 121
T 1 1 128 37 0 143
T 1 1 128 37 0 151
T 1 1 128 37 0 155
T 1 1 128 37 0 156
T 1 1 128 37 0 156
T 1 1 128 37 0 154
T 1 1 128 37 0 149
T 1 1 128 37 0 139
T 1 1 128 37 0 125
T 1 1 128 37 0 110
T 1 1 128 37 0 90
T 1 1 128 37 0 71
T 1 1 128 37 0 55
T 1 1 128 37 0 44
T 1 1 128 37 0 35
T 1 1 128 37 0 28
T 1 1 128 37 0 20
T 1 1 128 37 0 16
T 1 1 128 37 0 13
T 1 1 128 37 0 11
T 1 1 128 37 0 10
T 1 1 128 37 0 9
T 1 1 128 37 0 8
T 1 1 128 37 0 7
T 1 1 128 37 0 6
T 1 1 128 37 0 6
T 1 1 128 37 0 6
T 1 1 128 37 0 6
T 1 1 128 37 0 6
T 1 1 128 37 0 6
T 1 1 128 37 0 6
T 1 1 128 37 0 6
T 1 0 64 37 0 6

T 0 1 0 160 0 255
Y: 251
>T 0 1 128 160 0 255
T 0 1 128 160 0 255
T 0 1 128 160 0 255
T 0 1 128 160 0 255
T 0 1 128 160 0 255
T 0 1 128 160 0 255
T 0 1 128 160 0 255
T 0 1 128 159 0 1
T 1 1 128 158 0 33
T 1 1 128 158 0 49
T 1 1 128 158 0 58
T 1 0 64 158 0 58

T 0 1 0 32 0 251
Y: 247
>T 0 1 128 32 0 251
T 0 1 128 31 0 1
T 0 1 128 31 0 121
T 1 1 128 31 0 143
T 1 1 128 31 0 151
T 1 1 128 31 0 155
T 1 1 128 31 0 156
T 1 1 128 31 0 156
T 1 1 128 31 0 154
T 1 1 128 31 0 149
T 1 1 128 31 0 140
T 1 1 128 31 0 126
T 1 1 128 31 0 109
T 1 1 128 31 0 91
T 1 1 128 31 0 74
T 1 1 128 31 0 58
T 1 1 128 31 0 45
T 1 1 128 31 0 34
T 1 1 128 31 0 26
T 1 1 128 31 0 19
T 1 1 128 31 0 14
T 1 0 64 31 0 14

My finger was always only in the upper area of the display, so actually all values >60 are wrong.

BartS23 commented 1 year ago

Next try without swipe. I only touched the top corners and did not move my finger:

>T 0 1 0 6 0 251
Y: 247
>T 0 1 128 6 0 251
T 0 1 128 6 0 251
T 0 1 128 7 0 1
T 0 1 128 7 0 121
T 1 1 128 7 0 143
T 1 1 128 7 0 151
T 1 1 128 7 0 155
T 1 1 128 7 0 156
T 1 1 128 7 0 156
T 1 1 128 7 0 154
T 1 1 128 7 0 148
T 1 1 128 7 0 137
T 1 1 128 7 0 122
T 1 1 128 7 0 104
T 1 1 128 7 0 85
T 1 1 128 7 0 67
T 1 1 128 7 0 52
T 1 1 128 7 0 39
T 1 1 128 7 0 28
T 1 1 128 7 0 19
T 1 1 128 7 0 14
T 1 1 128 7 0 11
T 1 1 128 7 0 10
T 1 1 128 7 0 9
T 1 1 128 7 0 8
T 1 1 128 7 0 7
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 1 128 7 0 6
T 1 0 64 7 0 6

>T 0 1 0 12 0 254
Y: 250
>T 0 1 128 12 0 254
T 0 1 128 12 0 254
T 0 1 128 12 0 254
T 0 1 128 13 0 1
T 1 1 128 13 0 56
T 1 1 128 13 0 81
T 1 1 128 13 0 92
T 1 1 128 13 0 97
T 1 1 128 13 0 98
T 1 1 128 13 0 98
T 1 1 128 13 0 98
T 1 1 128 13 0 97
T 1 1 128 13 0 94
T 1 1 128 13 0 89
T 1 1 128 13 0 82
T 1 1 128 13 0 73
T 1 1 128 13 0 64
T 1 1 128 13 0 55
T 1 1 128 13 0 47
T 1 1 128 13 0 39
T 1 1 128 13 0 32
T 1 1 128 13 0 26
T 1 1 128 13 0 22
T 1 1 128 13 0 18
T 1 1 128 13 0 15
T 1 1 128 13 0 12
T 1 1 128 13 0 10
T 1 1 128 13 0 9
T 1 1 128 13 0 8
T 1 1 128 13 0 7
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 1 128 13 0 6
T 1 0 64 13 0 6

T 0 1 0 145 0 251
Y: 247
>T 0 1 128 145 0 251
T 0 1 128 145 0 251
T 0 1 128 145 0 251
T 0 1 128 145 0 251
T 0 1 128 145 0 251
T 0 1 128 146 0 1
T 0 1 128 146 0 119
T 1 1 128 146 0 143
T 1 1 128 146 0 151
T 1 1 128 146 0 155
T 1 1 128 146 0 156
T 1 1 128 146 0 156
T 1 1 128 146 0 153
T 1 1 128 146 0 145
T 1 1 128 146 0 132
T 1 1 128 146 0 115
T 1 1 128 146 0 95
T 1 1 128 146 0 76
T 1 1 128 146 0 59
T 1 1 128 146 0 45
T 1 1 128 146 0 36
T 1 1 128 146 0 26
T 1 1 128 146 0 19
T 1 1 128 146 0 14
T 1 1 128 146 0 12
T 1 1 128 146 0 10
T 1 1 128 146 0 9
T 1 1 128 146 0 8
T 1 1 128 146 0 7
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 1 128 146 0 6
T 1 0 64 146 0 6
gfwilliams commented 1 year ago

Great, thanks! So now I'm wondering, from what I see in these dumps:

T 0 1 128 160 0 255
T 0 1 128 160 0 255
T 0 1 128 159 0 1
T 1 1 128 158 0 33
T 1 1 128 158 0 49
T 1 1 128 158 0 58

Do you think that it makes sense to interpret 'Y' values >240 as if they were negative - so y-256

It seems as if you get them when right at the very top of the screen?

Or do you think they should just be ignored or treated at 0?

... and does a similar thing happen for X?

BartS23 commented 1 year ago

I have never seen that for X.

I think setting it to 0 is the better solution, because it could also occur with a tap. If we ignore it, a tap at the top might not work.

Negative values could in turn have other effects.

gfwilliams commented 1 year ago

I think negative should be ok as with calibration we'd get them anyway, but yes, I guess it's safer :) I'll get a fix in. Looks like you never see anything below 250?

BartS23 commented 1 year ago

It depends. The "T" values of your build were never below 250 (except 1-170 of course).

The values from the drag event are slightly below that.

gfwilliams commented 1 year ago

yes, drag values should be ok as they'll have been adjusted.

Ok - I've just pushed an update, so hopefully - finally - this will be fixed for you on new builds