mk-fg / infinite-image-scroller

Python/GTK desktop app to scroll images across the window carousel-style
Do What The F*ck You Want To Public License
15 stars 3 forks source link

Scrolling jumps #3

Closed Flurrywinde closed 3 years ago

Flurrywinde commented 3 years ago

After our discussion about the scrolling images stuttering here, I've been looking into it more (thanks for all the advice over there too), and I don't think what I'm experiencing is a pause/stutter from the images having to load anymore. It was that the images actually jump, sometimes a little, sometimes a lot, but always forward, i.e. up, ahead in time.

In image_set_scroll() I found this comment: # This seem to cause some scroll-jumps, not sure why, maybe wrong gtk event?, so could that have something to do with it?

It happens in exactly the same way regardless of whether it's using pixbuf_proc.so or not, however, so maybe not, since image_set_scroll() isn't called when pixbuf_proc.so is being used.

It even happens when only a single image is scrolling over and over, and it seems related to the image's dimensions in relation to the window's dimensions.

Say I start with an image with these dimensions: 500x375

Then, I keep decreasing the window's width until the scrolling starts to jump. In this case, it happened when the window dimensions became around 876x1034 pixels.

If I change the window's height to, say, 500 pixels, the jumping began when the window dimensions were 410x500.

At a height of 765 pixels: 635x765.

Next, say I use another image which has these dimensions: 500x750

Then, here's two example window dimensions for when the jumping begins to happen:

426x1024 281x674

mk-fg commented 3 years ago

Ah, right, don't think I've ever bothered testing resizing the window after app starts, as it's never been relevant to my "set it up to start with preset dimensions" (or typically full screen) use-case. So I'd guess that some values calculated from window size do not get updated properly after initial start, breaking the scrolling math somewhat.

Simple fix might be just to make window non-resizable, but guess resizing can be useful for some other use-cases or in tiling layouts, where WM messes with window sizes constantly.

Flurrywinde commented 3 years ago

I've been looking into this, and I believe I've determined when it happens. In scroll_update(), when pos is calculated by pos += self.image_cycle(), if it's negative, then the jumping happens. The more negative, the bigger the jump.

So I've been trying to figure out how to fix this, but I'm having trouble following the code. Any hints? What needs to be done in the case pos is negative?

mk-fg commented 3 years ago

Hm, I just tried to reproduce the issue and seem to have failed: https://e.var.nz/2020-11-27.image-scroller-test.mp4 (~17M, 2m26s) ("image-test-500x750.jpg" used there is https://e.var.nz/2020-11-27.image-scroller-test.image-test-500x750.jpg )

I can see some jitter when resizing the window, i.e. in an instant window goes from AxB -> MxN scrolling jumps a bit, do you mean that? Some of jank on resizing is from WM here, as it only sends new dimensions to gtk occasionally, not on every pixel of change, and tries to scale/fit some stuff in-between these updates, and don't think I've accounted for an actual resizing moment at all.

As you didn't mention that it's only at the moment of resizing specifically, guess you meant that scrolling itself stutters, and don't think I can reproduce it here, unless I still don't know what exactly to look for, so maybe you can point out at which seconds/frames it happens in the linked video? (barring some compression artifacts, any >2px jump should probably be visible with -r 60 there)

mk-fg commented 3 years ago

So I've been trying to figure out how to fix this, but I'm having trouble following the code. Any hints?

self.dim_* stuff there are helpers to get various dimensions used in scroll-time calculations.

I think idea behind all these is to avoid having like 4 "if" cases in every single calculation for every scrolling direction, which might be how it was done somewhere in git history here, and was more confusing on the other level (had to think how to adjust everything 4x and with confusing different signs and flag checks for each case).

What needs to be done in the case pos is negative?

Certainly looks like a bug somewhere, as dim_scroll_translate should have it be monotonic from 0, and "0" should only happen in the very beginning of the scroll process, I think, otherwise position should stay somewhere in/near the middle of the box.

But if you have some weird queue size for images, or there's a bug somewhere to that effect, so that they get removed in scrolled-past direction immediately somehow, then I guess pos can get to 0, but it really should not - there should always be N images to scroll in either direction (again, except at the very start of the script). And it shouldn't be negative in any case.

I think #infinite-image-scroller, #infinite-image-scroller * { background: transparent; } line in ScrollerConf.win_css might be the one that makes scrollbar caret transparent, maybe try removing it to see how that "scroll position" works visually, where scrolling position stays and how/where it jumps. Can also just put a bunch of print()'s all over the code and see if there's anything odd there, I guess.

But again, can't seem to reproduce the issue, so can't really do the same here, as it'll all be perfectly normal and expected values for me. Maybe check that video above and tell me what you're doing differently from me there to trigger issue to happen.

Flurrywinde commented 3 years ago

Wow, thanks for all that info. I'll try to see if I can understand it later, but for now, here's a screencast of the jump happening, with debugging print()s I added. https://www.dailymotion.com/video/x7xr0hi (Download link: https://ufile.io/12zfeuqh )

I think you might experience it if you make the window very narrow and tall, then leave it for long enough while the images scroll by.

Edit: I think you might be on to something with the scrollbar. I just watched it, and it seems the jump doesn't happen when the scrollbar doesn't return all the way to the top. With the negative value for pos, the repositioning might try to move the scrollbar beyond where it can go.

mk-fg commented 3 years ago

Can you try same trick with e.g. --queue=10:0.6 parameter?

mk-fg commented 3 years ago

As you seem to have only one image above/below one being scrolled, so I think this should happen with an overly narrow window, as gtk scrolling position bumps into start/end of scrolled box as images get added/removed. (also, feel like I'm suggesting this queue option for like a 3rd-5th time to you, maybe should just bump default value to avoid that kind of weirdness... edit: bumped in 82b6723 - now 10:0.6 should be the default)

mk-fg commented 3 years ago

I think issue showcased in that screencast is best illustrated by this snapshot from it:

screencast snapshot with three images

You have default value to "only load 3 images into a scrolled box" and you have all three visible in a window!

Given that this app works by adding images on one side of the scrolled box and removed on the other, there's just no way for it to work sanely if you have all images that it can possibly load already on display - of course added/removed images will be visible (and not off-screen) and cause whatever jumps and weirdness.

If you have 50 images in a window, -q/--queue should be at least 52 or maybe more like 70 in case some take a while to load (to avoid bottlenecking the scrolling), have different dimensions, and such. And with just 3 on display, -q/--queue should be at least 5, and now-default 10 should suffice, unless you stretch window into a thin enough stripe to show all 10. (there's also that second prefetch-threshold parameter, which should influence that too, and is probably documented in the option description)

Flurrywinde commented 3 years ago

Yup, increasing conf.queue-size fixes it. Awesome! Defaulting to 10 should prevent the issue in just about all reasonable cases, but I got to thinking how to handle the edge case, like 50 images in the window all at once. LOL.

I came up with the following proof-of-concept that does work, though still isn't the right way to do it (see comment in code). In scroll_update():

pos += self.image_cycle()
if pos > 0:
    adj.set_value(self.dim_scroll_translate(pos, pos_max))
elif pos < 0 and pos != (len(self.box_images) * -1):  # Hack to not increase queue_size before images are loaded (if it takes awhile, queue_size gets huge)
    self.conf.queue_size += 1
mk-fg commented 3 years ago

Eh, you might also want to be able to scroll stuff manually, and account for various processing and fs slowdowns, without increasing number of images there.

pos != (len(self.box_images) * -1) seem to be compating image count to pixels. Also not sure if position going negative is a reliable indicator of window size being larger or close-enough to total width/height of images (depending on scroll direction) - I think with e.g. same-size images all displayed in a narrow window, it should stay at 0, and only go negative on size mismatch between removed and added image in that image_cycle func (in which way - depends on scroll direction too).

Seem to be too much complexity for an automatic solution to a rare issue which should be much easier to fix with a manual switch, which is there already. I'm kinda surprised why you were so resistant to using it, given that narrowing the window, you can easily see how the whole thing works, with just 3 images being cycled there, and despite my repeated suggestions to use the damn option.

Flurrywinde commented 3 years ago

No, no, don't get me wrong. I'll definitely use the "damn option." Sorry. I'm just trying to be helpful since a case could arise where the scroller still jumps and users might not know to make the queue bigger. If it's not worth the trouble figuring out how to do it, though, no worries.

Just in case, though, to address why I compated image count to pixels, it's because I noticed that with three images, before they load, pos was -3. I think this has to do with pos being 1 for each in this case, and the code adds them together. Obviously the right way is to determine when they've loaded the right way, but I couldn't figure that out without looking into it more. Then, add to this the other possibilities you mention, like pos being negative not being a reliable indicator, and yeah, it's a harder problem than my simple solution warrants. Again, sorry if I seemed weird about it or something. Let me know if you don't appreciate my input, cuz I do have a few more things I wanted to tell you about.

mk-fg commented 3 years ago

No, no, don't get me wrong. I'll definitely use the "damn option." Sorry. I'm just trying to be helpful since a case could arise

Yeah, that's how I understood it as well.

Mentioned you there specifically as a real-world UX case where this manual approach seem to have failed, as I guess it's not apparent how the thing works and how to tweak it accordingly, but idk, maybe it's just you :) Probably not worth worrying even this much about UI/UX design in this random weird ad-hoc script anyway.

mk-fg commented 3 years ago

I noticed that with three images, before they load, pos was -3

I think it might be an some bug where maybe between-image gap interval is subtracted from 0 or something like that. But if it just happens on startup, probably not worth worrying about.

mk-fg commented 3 years ago

Let me know if you don't appreciate my input, cuz I do have a few more things I wanted to tell you about.

Nah, it's fine, it's not hard to fix stuff here, though dunno if I'm interested in actually improving the thing much - as far as I'm concerned it worked fine for rare use-cases I had for it, and not sure if I'll really need it again. Though do use it occasionally to cover display when e.g. listening to something not far from the display to put something non-distracting on it so that I can't interact with it.

Flurrywinde commented 3 years ago

Cool.