selection: once again highlight non-trailing empty cells

Foot 1.13.0 introduced a regression where non-trailing empty cells
were highlighted inconsistently (cells that shouldn’t be highlighted,
were, seemingly at random).

86663522d5 “fixed” this by never
highlighting *any* empty cells.

This meant the behavior, compared to foot 1.12 and earlier,
changed. In foot 1.12 and older versions, non-trailing empty cells
were highlighted, as long as the selection covered at least one of the
trailing non-empty cells.

This patch restores that behavior.

To understand how this works, lets first take a look at how selection
works:

When a selection is made, and updated (i.e. the mouse is dragged, or
the selection is extended through RMB etc), we need to (un)tag and dirty
the cells that are a) newly selected, or b) newly deselected. That is,
we look at the diff between the “old” and the “new” selection, and
only update those cells.

This is for performance reasons: iterating the entire selection is not
feasible with large selections. However, it also means we cannot
reason about empty cells; we simply don’t know if an empty cells is a
trailing empty cell, or a non-trailing one.

Then, when we render a frame, we iterate all the *visible*
and *selected* cells, once again tagging them as selected (this is
needed since a selected cell might have lost its selected tag if the
cell was written to, by the client application, after the selection
was made).

At this point, we *can* reason about empty cells.

So, to restore the highlighting behavior to that of foot 1.12, we do
this:

When working with the selection diffs when a selection is updated, we
don’t special case empty cells at all. Thus, all empty cells covered
by the selection is highlighted, and dirtied.

But, when rendering the frame, we _do_ special case them. The only
difference (compared to foot 1.12) is that we *must*
explicitly *clear* the selection tag, and dirty the empty cells. This
is to ensure the empty cells that were incorrectly highlighted by the
selection update algorithm, isn’t rendered as that.

This does have a slight performance impact, as empty cells are now
always re-rendered. The impact should however be small.
This commit is contained in:
Daniel Eklöf 2022-08-22 20:14:06 +02:00
parent 3cf11bfea9
commit 21ab16239d
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
2 changed files with 38 additions and 8 deletions

View file

@ -51,8 +51,6 @@
### Changed
* Window is now dimmed while in Unicode input mode.
* Selected empty cells are **never** highlighted as being
selected. They used to be, when followed by non-empty cells.
* Default color theme from a variant of the Zenburn theme, to a
variant of the Solarized dark theme.

View file

@ -713,7 +713,8 @@ pixman_region_for_coords(const struct terminal *term,
static void
mark_selected_region(struct terminal *term, pixman_box32_t *boxes,
size_t count, bool selected, bool dirty_cells)
size_t count, bool selected, bool dirty_cells,
bool highlight_empty)
{
for (size_t i = 0; i < count; i++) {
const pixman_box32_t *box = &boxes[i];
@ -737,7 +738,9 @@ mark_selected_region(struct terminal *term, pixman_box32_t *boxes,
row->dirty = true;
for (int c = box->x1, empty_count = 0; c < box->x2; c++) {
if (row->cells[c].wc == 0) {
struct cell *cell = &row->cells[c];
if (cell->wc == 0 && !highlight_empty) {
/*
* We used to highlight empty cells *if* they were
* followed by non-empty cell(s), since this
@ -757,7 +760,36 @@ mark_selected_region(struct terminal *term, pixman_box32_t *boxes,
* cells (they still get converted to spaces when
* copied, if followed by non-empty cells).
*/
/* empty_count++; */
empty_count++;
/*
* When the selection is *modified*, empty cells
* are treated just like non-empty cells; they are
* marked as selected, and dirtied.
*
* This is due to how the algorithm for updating
* the selection works; it uses regions to
* calculate the difference between the old and
* the new selection. This makes it impossible
* to tell if an empty cell is a *trailing* empty
* cell (that should not be highlighted), or an
* empty cells between non-empty cells (that
* *should* be highlighted).
*
* Then, when a frame is rendered, we loop the
* *visibible* cells that belong to the
* selection. At this point, we *can* tell if an
* empty cell is trailing or not.
*
* So, what we need to do is check if a
* selected, and empty cell has been marked as
* selected, temporarily unmark (forcing it dirty,
* to ensure it gets re-rendered). If it is *not*
* a trailing empty cell, it will get re-tagged as
* selected in the for-loop below.
*/
cell->attrs.clean = false;
cell->attrs.selected = false;
continue;
}
@ -810,10 +842,10 @@ selection_modify(struct terminal *term, struct coord start, struct coord end)
pixman_box32_t *boxes = NULL;
boxes = pixman_region32_rectangles(&no_longer_selected, &n_rects);
mark_selected_region(term, boxes, n_rects, false, true);
mark_selected_region(term, boxes, n_rects, false, true, true);
boxes = pixman_region32_rectangles(&newly_selected, &n_rects);
mark_selected_region(term, boxes, n_rects, true, true);
mark_selected_region(term, boxes, n_rects, true, true, true);
pixman_region32_fini(&newly_selected);
pixman_region32_fini(&no_longer_selected);
@ -1078,7 +1110,7 @@ selection_dirty_cells(struct terminal *term)
int n_rects = -1;
pixman_box32_t *boxes =
pixman_region32_rectangles(&visible_and_selected, &n_rects);
mark_selected_region(term, boxes, n_rects, true, false);
mark_selected_region(term, boxes, n_rects, true, false, false);
pixman_region32_fini(&visible_and_selected);
pixman_region32_fini(&view);