Package: emacs;
Reported by: Po Lu <luangruo <at> yahoo.com>
Date: Sat, 18 Sep 2021 12:24:01 UTC
Severity: normal
Found in version 28.0.50
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Message #32 received at 50660 <at> debbugs.gnu.org (full text, mbox):
From: Po Lu <luangruo <at> yahoo.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: larsi <at> gnus.org, 50660 <at> debbugs.gnu.org Subject: Re: bug#50660: 28.0.50; Text artifacting when the cursor moves over text under mouse face that originally displayed a box Date: Mon, 20 Sep 2021 09:00:57 +0800
Eli Zaretskii <eliz <at> gnu.org> writes: > I admit that I'm confused. I don't think I understand what did you > find was the problem, and how it came into existence. Can you explain > it in detail, step by step, with references to the current code on > master? In set_cursor_from_row, the cursor's X position is calculated by summing up the pixel_width of each glyph, from the start of the row (if not reversed), or from the end (if reversed). Inside the produce_XXX_glyph series of functions, the widths of each box line is added to the iterator's pixel_width (take this snippet from produce_image_glyph) if (face->box != FACE_NO_BOX) { if (face->box_horizontal_line_width > 0) { if (slice.y == 0) it->ascent += face->box_horizontal_line_width; if (slice.y + slice.height == img->height) it->descent += face->box_horizontal_line_width; } if (face->box_vertical_line_width > 0) { if (it->start_of_box_run_p && slice.x == 0) it->pixel_width += face->box_vertical_line_width; if (it->end_of_box_run_p && slice.x + slice.width == img->width) it->pixel_width += face->box_vertical_line_width; } } But when part of the row gets highlighted, it becomes drawn in the mouse face, which might have greatly different box widths (or none at all) compared to the face in which it was originally drawn. This causes issues when moving the point (and thus the cursor) over the highlighted area, because the glyph_width of the glyphs reflect the state before the area was highlighted, and not after, and the cursor will be offset by the original width of the box lines, which may be incorrect under mouse face. For example, the face `custom-button' has a vertical line 2 pixels wide, but `highlight' has no vertical line. If an area under the face `custom-button' is becomes highlighted and under the face `highlight', and the cursor is moved into the area, the cursor will be 2 pixels too far towards the end of the row (assuming it is not reversed), and potentially more, if there are more glyphs between the cursor and the start/end of the highlighted area with left and/or right box lines! > You said the problem happens when one moves the mouse pointer over > text whose face has a box. You said explicitly that clicking the > mouse or dragging it over the text was not required. Did I understand > you correctly? If I understood you correctly, then I don't think I > see how drawing the cursor could be involved, because moving the mouse > pointer without clicking doesn't move point, and thus the cursor > doesn't need to be moved. Yes, but if you move the point into the highlighted area with other editing commands, such as the arrow keys, the problem will manifest. >> Unfortunately, previously, this information would not be updated when >> the the glyphs become displayed under mouse face > What information was not updated, exactly? The pixel_width of each glyph in the row that was highlighted and has left_box_line_p, or right_box_line_p. > When the text becomes highlighted, the face in effect is mouse-face, > which usually doesn't have the box attribute, and so the text moves > horizontally, which is expected. Yes, that isn't the problem I'm talking about. > And the only glyphs whose pixel_width is affected in this situation > are the glyphs where the box face begins or ends. What was missing in > handling this situation? Basically, the pixel_width of these glyphs at the beginning or the end of the box face have to be updated. Otherwise, if the cursor moves over the box it is highlighted with the mouse-face, it might appear at the wrong position, as explained above. > In any case, I'm surprised that fixing such a minor issue needed so > much code, including changes to data structures. Are you sure you > used all the information we store in glyph_row and in each glyph? > (Once I understand the problem better, I will be able to answer this > question myself, but I'm not there yet.) I hope I have, but please tell me if I haven't. > Some minor comments and questions below. > >> @@ -17131,6 +17131,8 @@ set_cursor_from_row (struct window *w, struct glyph_row *row, >> x = -1; >> } >> } >> + if (row->have_glyph_with_box_p) >> + x = -1; > > Here, I don't understand why the cursor position is affected, and I > don't understand why you subtract the fixed value of 1 pixel. The box > thickness doesn't have to be 1, and it could be positive or negative. If I understand correctly, setting x to -1 should force it to be re-computed later, when the code enters compute_x. Is that not correct? > I don't understand why we need to do this adjustment of glyphs' width. Hopefully my explanation above should have made this clear. >> + struct face *mouse_face = >> + FACE_FROM_ID_OR_NULL (f, MOUSE_HL_INFO (f)->mouse_face_face_id); >> + >> + if (mouse_face == NULL) >> + mouse_face = FACE_FROM_ID (f, MOUSE_FACE_ID); > When is this last NULL check actually needed? I'm not exactly sure, but a lot of code seems to do that. >> + /* If there's a row with a box somewhere, by all likelyhood >> + the dimensions of the row has been changed. If that is >> + the case, and we also find a row where the phys cursor >> + is, recalculate the dimensions of the phys cursor. */ > I also don't understand this part. When is it needed and why? And > why not handle it in display_and_set_cursor (if it isn't handled > already)? I think display_and_set_cursor doesn't do any calculating. Here, what's happening is that the X-offset of the phys cursor is being re-computed, to compensate for any changes that might have happened to the pixel_width of the glyphs. >> + struct glyph *start = row->glyphs[TEXT_AREA]; >> + struct glyph *last = start + row->used[TEXT_AREA] - (intptr_t) 1; >> + >> + while (last > start && last->charpos < 0) >> + --last; > > Here you assume that only glyphs at end of the row could have negative > charpos, but that's not true. Glyphs at the start could have that as > well. Thanks.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.