Package: emacs;
Reported by: Konrad Podczeck <konrad.podczeck <at> univie.ac.at>
Date: Fri, 17 Jul 2020 15:37:02 UTC
Severity: normal
Message #246 received at 42406 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: Eli Zaretskii <eliz <at> gnu.org> Cc: alan <at> idiocy.org, konrad.podczeck <at> univie.ac.at, 42406 <at> debbugs.gnu.org Subject: Re: bug#42406: Mouse-wheel scrolling can be flickering Date: Thu, 17 Dec 2020 16:07:19 -0500
> That probably just means abbrev-mode should be added to the list at > the end of frame.el. Or maybe that we need some new mechanism to > trigger update of the lighter on the mode line when a mode is turned > on or off. Don't know about "new" but the old mechanism is that the standard minor-mode code ends up calling `force-mode-line-update` (this now mostly comes from `define-minor-mode`, but in the past it was present in most "manual" definitions as well). > And while we are talking about force-mode-line-update: can you explain > why we need to set the prevent_redisplay_optimizations_p flag of the > buffer, in addition to setting update_mode_lines to a magic value? I wish I could, but that bit predates me, and I have no idea what `prevent_redisplay_optimizations_p` means or does, really. I just removed it from my local Emacs, to see if I notice any difference. > And btw, redisplaying the mode line in general could mean you need to > redisplay the text area as well, for example when the mode line > changes its height. So setting update_mode_lines to REDISPLAY_SOME > under the assumption that only the mode line needs to be considered is > not necessarily true and can cause redisplay bugs. I don't see why you relate this problem to REDISPLAY_SOME: when setting update_mode_lines to other values, xdisp.c should suffer from the same problem (it presumably updates the mode-lines of all windows without updating the corresponding window's contents). >> > We should stop lumping heuristics one on top another, and instead >> > redesign this from scratch and make sure that every flag we set is >> > acted upon as intended, and only in situations we intend them to be >> > acted upon. E.g., we should be able to set f->redisplay to a value >> > that means "update only the frame title". >> >> The `redisplay` bit is not supposed to be a heuristic at all. It just >> tried to keep track more precisely of which part of the redisplay may >> have changed. `fset_redisplay` marks the frame to be redisplayed at the >> next redisplay, setting `update_mode_lines` to a non-zero value means >> that when redisplaying a window we also redisplay its mode line, so >> the suggested hunk definitely doesn't rely on any kind of heuristic. > > Then what is this bit of redisplay_internal about: > > consider_all_windows_p = (update_mode_lines > || windows_or_buffers_changed); > [...] > if (consider_all_windows_p) > { > FOR_EACH_FRAME (tail, frame) > XFRAME (frame)->updated_p = false; > > propagate_buffer_redisplay (); > > FOR_EACH_FRAME (tail, frame) > { > [...] > > If the redisplay flag is all we need, how come we must also set > update_mode_lines or windows_or_buffers_changed to get Emacs to > consider anything beyond the selected window? The `redisplay` bits were designed to reduce the set of windows that we consider at each redisplay. Before them, there were the 2 modes you described in the TODO: either only consider the selected window or consider all windows. The `redisplay` bits only come into play when we get to the "all windows" case. > Why does it have to be so complicated to say "this frame needs to have > all of its windows reconsidered for redisplay"? Is it? AFAIK `fset_redisplay (f)` is all it takes, which doesn't seem particularly complex (and neither does its code). >> The main problems I see with my suggested patch are: >> - I don't know if it actually fixes the original problem. > And this is exactly my problem: this is the "heuristic" part I was > talking about. Instead of knowing exactly which flag does what and > why, we have a combination of flags and global variables, and try > tweaking them until we get the desired result. This can only work up > to a point, and I think we are well beyond that point. Not sure what you're suggesting here. [ At least I know what the `redisplay` bits are *supposed* to do. What I meant by "I don't know if it actually fixes the original problem" is that I can't reproduce it locally so I need someone else to confirm that it fixes the original problem, and this is not because I don't understand what the code I changed does, but because I don't know enough about the problem to be sure that my fix addresses the actual problem. ] >> - It can cause *more* redisplay work because it will force redisplay of >> all the windows in the current frame (rather than only their mode >> lines). > > See, we have a single set of conditions that controls when we consider > the frame title, when we consider the mode line, the header-line, the > tab-line, the tool bar, and the menu bar. It makes very little sense > to me to use the same condition for all of these. I think it makes a lot of sense from the point of view of managing code complexity. But indeed, it leaves open optimization opportunities, so we could refine the info used to keep track of what needs to be redisplayed. >> > I'm not against experimenting with replacing 42 by 32 or by >> > REDISPLAY_SOME etc., but I don't think we should install anything >> > along these lines, except if we need to fix a clear bug (i.e. a >> > redisplay glitch), which this one isn't. >> I don't know what you mean by "along these lines". > "Along these lines" means playing more games with "special" values of > update_mode_lines and windows_or_buffers_changed. I don't know what you mean by "special values". And I'm not playing any games here. The meaning of those vars is as follows: - update_mode_lines == 0 means that none of the mode lines (and relatives) needs to be updated. - update_mode_lines > 2 means that all the mode lines in all windows need to be updated. - update_mode_lines == 2 means that all the mode lines need to be updated in the set designated by the `redisplay` bits (where the `redisplay` on a frame means that all of its windows are also part opf the set, and where the `redisplay` bit of a buffer means that all the windows that display this buffer are also part of the set). - windows_or_buffers_changed == 0 means that only the selected window's content may need to be updated. - update_mode_lines > 2 means that the contents in all windows may need to be updated. - update_mode_lines == 2 means that the contents in all windows in the set designated by the `redisplay` bits may need to be updated. Stefan
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.