Only iterate visible cells when marking selected cells before rendering a frame #1114

Closed
opened 4 months ago by dnkl · 2 comments
dnkl commented 4 months ago
Owner

When rendering a frame, we need selected cells to be marked as such, to get the correct coloring.

This is done by iterating the entire selection, each time a frame is rendered. This causes foot to slow to a crawl when the selection gets large enough. Eventually, this leads to a socket overflow on the compositor side, which is trying to send pointer motion events to foot. Foot being busy with iterating the selection has no time to handle those events, leading to a disconnect. Sometimes we shut down "cleanly", and sometimes we crash. Regardless, this is obviously really bad.

The fix is to only iterate the visible cells.

First reported on IRC:

17:06           Arsen  I'm able to get foot to consistently crash if I hold shift+pgup (scroll up) and move my mouse around while grabbing text     
                Arsen  can repro on master also
                Arsen  #0  0x00005555555a1e47 in term_mouse_grabbed (term=term@entry=0x5555555f9950, seat=seat@entry=0x5555556d2d00) at ../terminal.c:2889
                Arsen  top frame in the backtrace
17:07           Arsen  kwin 5.24.5
17:08            dnkl  Arsen: thanks, let me see what I can do
                Arsen  ah, I also needed somewhat long lines, so I turned to "yes aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" to fill the scrollback
17:09           Arsen  (nothing up my sleve there, I just held the A key for a few seconds)
17:10           Arsen  I'm pretty sure I need to move the mouse so that foot decides to update what area it grabs as a selection
                Arsen  which makes me wonder, what if I just made a really large selection?
17:11           Arsen  yeah, it's due to the size of the selection, if I stop scrolling and just select, it does the same
17:12           Arsen  oh, hmm, this could be a red herring
17:20           Arsen  I had a feeling it might be related to the primary selection, so I early-returned false in text_to_primary, but that'd appear to not be it
18:21           Arsen  oh, I just noticed, before foot crashes, there's a disconnect: warn: wayland.c:1176: disconnected from Wayland
                Arsen  and that's a noticable delay before the segfault also
When rendering a frame, we need selected cells to be marked as such, to get the correct coloring. This is done by iterating the **entire** selection, **each** time a frame is rendered. This causes foot to slow to a crawl when the selection gets large enough. Eventually, this leads to a socket overflow on the compositor side, which is trying to send pointer motion events to foot. Foot being busy with iterating the selection has no time to handle those events, leading to a disconnect. Sometimes we shut down "cleanly", and sometimes we crash. Regardless, this is obviously really bad. The fix is to only iterate the **visible** cells. First reported on IRC: ``` 17:06 Arsen I'm able to get foot to consistently crash if I hold shift+pgup (scroll up) and move my mouse around while grabbing text Arsen can repro on master also Arsen #0 0x00005555555a1e47 in term_mouse_grabbed (term=term@entry=0x5555555f9950, seat=seat@entry=0x5555556d2d00) at ../terminal.c:2889 Arsen top frame in the backtrace 17:07 Arsen kwin 5.24.5 17:08 dnkl Arsen: thanks, let me see what I can do Arsen ah, I also needed somewhat long lines, so I turned to "yes aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" to fill the scrollback 17:09 Arsen (nothing up my sleve there, I just held the A key for a few seconds) 17:10 Arsen I'm pretty sure I need to move the mouse so that foot decides to update what area it grabs as a selection Arsen which makes me wonder, what if I just made a really large selection? 17:11 Arsen yeah, it's due to the size of the selection, if I stop scrolling and just select, it does the same 17:12 Arsen oh, hmm, this could be a red herring 17:20 Arsen I had a feeling it might be related to the primary selection, so I early-returned false in text_to_primary, but that'd appear to not be it 18:21 Arsen oh, I just noticed, before foot crashes, there's a disconnect: warn: wayland.c:1176: disconnected from Wayland Arsen and that's a noticable delay before the segfault also ```
dnkl added the
bug
performance
labels 4 months ago
Poster
Owner

Turns out this isn't the only time we iterate the entire selection. When the selection is updated, we also do it. This time to dirty newly selected cells, or cells that was selected but aren't anymore.

Turns out this isn't the only time we iterate the entire selection. When the selection is **updated**, we also do it. This time to dirty newly selected cells, or cells that was selected but aren't anymore.
Poster
Owner

This is tougher... we must dirty these cells, even if they're not currently visible. Otherwise, they'll not get rendered once they do become visible.

We'll have to figure out a way to do this without iterating all the selected cells.

This is tougher... we **must** dirty these cells, even if they're not currently visible. Otherwise, they'll not get rendered once they do become visible. We'll have to figure out a way to do this without iterating all the selected cells.
dnkl closed this issue 4 months ago
Sign in to join this conversation.
No Milestone
No Assignees
1 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dnkl/foot#1114
Loading…
There is no content yet.