kworkflow / patch-hub

patch-hub is a TUI that streamlines the interaction of Linux developers with patches archived on lore.kernel.org
GNU General Public License v2.0
8 stars 6 forks source link

Patch preview scroll lags for some renderers when patch is "too big" #77

Closed davidbtadokoro closed 4 days ago

davidbtadokoro commented 3 weeks ago

Description:

When previewing a patch with delta (although I don't rule out that this happens w/ other renderers), when scrolling with up/down arrows (j/k keys) or left/right arrows (h/l keys), if the patch isn't "too small", the preview will lag, i.e., the scroll will still be happening after you release the arrow/key. Note that I didn't experience problems with half/full page jumps

Obs.: "Too small" isn't accurate, but I experienced it even w/ medium patches with ~50 additions/deletions.

How to reproduce:

Choose delta as the renderer, go to a patchset with a patch that isn't "too small" (sorry for the ambiguousness), and hit and hold a scroll arrow/key (up/down/left/right or k/j/h/l) for 2 to 3 seconds, then release it. You should be able to see this lag.

Expected behavior:

This unresponsiveness can drive off users and feel like a bad smell. I am speculating here, but there must be a reasonable way for us to eliminate it.

Screenshots

The below gif won't expose the behavior, as it won't show me holding the key (if any reporter can reproduce it with that key-capturing display, I would be glad), but I hope that it illustrates the issue.

Peek 2024-10-28 12-41

Setup:

davidbtadokoro commented 3 weeks ago

Update

The "lagginess" varies depending on the size of the file. I've encountered a patch with ~300 adds/deletions, and it performed really poorly (holding for ~4s resulted in ~10s lags). In these cases, holding half/full page jumps also lags.

I strongly believe that this is due to rendering the entire patch at every frame, as we can see here. At the time I thought this made sense as this doesn't relate to the sub-state of the app, but to the way we view it.

PatchsetDetailsAndActionsState is a huge type with many fields, but how about we add another one that is analogous to patches, but in its rendered version. Maybe rename patches to raw_patches and make this new one rendered_patches :sweat_smile:

Another detail that I've noticed is that because we manipulate the "raw" patch, not the rendered one when we move up/down/left/right, jump, etc. we use the "raw" offsets, which causes not so much accurate scrolls and pans (what I was trying to mean in this TODO).