From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#52561: 28.0.50; [PATCH] Tall characters create scrolling stasis Date: Fri, 17 Dec 2021 15:42:46 +0200 Message-ID: <831r2be4a1.fsf@gnu.org> References: <87mtl0b1e3.fsf@dick> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="38291"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 52561@debbugs.gnu.org To: dick.r.chiang@gmail.com Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Dec 17 14:52:19 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1myDek-0009jH-Px for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 17 Dec 2021 14:52:19 +0100 Original-Received: from localhost ([::1]:47390 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1myDei-00040I-Sh for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 17 Dec 2021 08:52:16 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:53356) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1myDWk-0003HV-Rp for bug-gnu-emacs@gnu.org; Fri, 17 Dec 2021 08:44:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:54978) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1myDWk-0005wY-Hv for bug-gnu-emacs@gnu.org; Fri, 17 Dec 2021 08:44:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1myDWk-000472-GH for bug-gnu-emacs@gnu.org; Fri, 17 Dec 2021 08:44:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 17 Dec 2021 13:44:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 52561 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 52561-submit@debbugs.gnu.org id=B52561.163974859915716 (code B ref 52561); Fri, 17 Dec 2021 13:44:02 +0000 Original-Received: (at 52561) by debbugs.gnu.org; 17 Dec 2021 13:43:19 +0000 Original-Received: from localhost ([127.0.0.1]:38286 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1myDVz-00045J-6t for submit@debbugs.gnu.org; Fri, 17 Dec 2021 08:43:19 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:52404) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1myDVy-000457-BM for 52561@debbugs.gnu.org; Fri, 17 Dec 2021 08:43:14 -0500 Original-Received: from [2001:470:142:3::e] (port=44356 helo=fencepost.gnu.org) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1myDVq-0005lw-Aj; Fri, 17 Dec 2021 08:43:09 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=YXU2fcoQSBk8Aj9jCZNDrKbFdUw8PItruq8cYNfAf+s=; b=aogl5eWTnLHD Vex9BYMO6axi4DexboTxOU2IQTiRTiF2Sf9hb83cKQY7iWGUMajkMlRYbKKDg1XUH7JuAKjtAemHK yffoPVVLTJ8k3nrk7E03s/8S/1UqQCW0Tdx2+CO+kpnbBzRAHU458adnzwd4FThR0YPN6jbthnPQM 4gOp8vBeEcOPAR2ojZSOO7SrllBu8zJ4zC6FqhpZ7V8WrJKEuhl71wNSAVd1YfUrM+zCQ8YwUORmd 3/tSsZN2hB/mNjME9iIUdBA1JwE/iAhmsADZP8BzTSvx73P9Jc90VJmbVm18vu/Er6X0Rg6I0cYSx pR3lCA2ptPy72Vm2g2yWSg==; Original-Received: from [87.69.77.57] (port=4979 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1myDVm-0002jN-Nl; Fri, 17 Dec 2021 08:43:06 -0500 In-Reply-To: <87mtl0b1e3.fsf@dick> (dick.r.chiang@gmail.com) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:222571 Archived-At: > From: dick.r.chiang@gmail.com > Date: Thu, 16 Dec 2021 18:01:08 -0500 > > After this prints "all bad", try M-v (scroll-down). It never prints "all bad" in my testing, neither in Emacs 27.2 nor in Emacs 29. But the first line is not taller here, so I also tried to copy the Hindu line from etc/HELLO (which does display taller here) to try reproducing the issue, and I still get "all good". > diff --git a/src/xdisp.c b/src/xdisp.c > index 5e549c9c63..65632ff167 100644 > --- a/src/xdisp.c > +++ b/src/xdisp.c > @@ -18921,7 +18921,7 @@ redisplay_window (Lisp_Object window, bool just_this_one_p) > if (w->force_start) > { > /* We set this later on if we have to adjust point. */ > - int new_vpos = -1; > + int new_y = -1; > > w->force_start = false; > w->vscroll = 0; > @@ -18991,28 +18991,16 @@ redisplay_window (Lisp_Object window, bool just_this_one_p) > NULL, 0); > } > if (r) > - new_vpos = MATRIX_ROW_BOTTOM_Y (r); > + new_y = MATRIX_ROW_BOTTOM_Y (r); > else /* Give up and just move to the middle of the window. */ > - new_vpos = window_box_height (w) / 2; > + new_y = window_box_height (w) / 2; > } > > if (!cursor_row_fully_visible_p (w, false, false, false)) > { > /* Point does appear, but on a line partly visible at end of window. > Move it back to a fully-visible line. */ > - new_vpos = window_box_height (w); > - /* But if window_box_height suggests a Y coordinate that is > - not less than we already have, that line will clearly not > - be fully visible, so give up and scroll the display. > - This can happen when the default face uses a font whose > - dimensions are different from the frame's default > - font. */ > - if (new_vpos >= w->cursor.y) > - { > - w->cursor.vpos = -1; > - clear_glyph_matrix (w->desired_matrix); > - goto try_to_scroll; > - } > + new_y = WINDOW_BOX_HEIGHT_NO_MODE_LINE (w); > } > else if (w->cursor.vpos >= 0) > { > @@ -19052,13 +19040,18 @@ redisplay_window (Lisp_Object window, bool just_this_one_p) > > /* If we need to move point for either of the above reasons, > now actually do it. */ > - if (new_vpos >= 0) > + if (new_y >= 0) > { > struct glyph_row *row; > > - row = MATRIX_FIRST_TEXT_ROW (w->desired_matrix); > - while (MATRIX_ROW_BOTTOM_Y (row) < new_vpos) > - ++row; > + for (row = MATRIX_FIRST_TEXT_ROW (w->desired_matrix); > + row < MATRIX_BOTTOM_TEXT_ROW (w->desired_matrix, w); > + ++row) > + if (MATRIX_ROW_BOTTOM_Y (row) - row->extra_line_spacing > new_y) > + { > + row--; > + break; > + } > > TEMP_SET_PT_BOTH (MATRIX_ROW_START_CHARPOS (row), > MATRIX_ROW_START_BYTEPOS (row)); If this is supposed to fix the problem shown by the recipe, then I don't understand the rationale. For starters, a breakpoint set in the while/for loop of the last hunk never breaks for me when I run the recipe, so it seems unrelated. row->extra_line_spacing is supposed to be zero (and your recipe doesn't change that), so the original inequality and the one you propose are equivalent, I think. As for replacing window_box_height with WINDOW_BOX_HEIGHT_NO_MODE_LINE: are you saying that this could move point too much down the window, when we have a header-line and/or a tab-line? That could be true (though it's mostly harmless in the use cases this code is supposed to support), but again, it's unrelated to the recipe, which specifies neither header-line nor tab-line. And the part you propose to remove, viz.: - /* But if window_box_height suggests a Y coordinate that is - not less than we already have, that line will clearly not - be fully visible, so give up and scroll the display. - This can happen when the default face uses a font whose - dimensions are different from the frame's default - font. */ - if (new_vpos >= w->cursor.y) - { - w->cursor.vpos = -1; - clear_glyph_matrix (w->desired_matrix); - goto try_to_scroll; - } was added to fix a bug, bug#17095. Did you try the recipes provided there to make sure this doesn't reintroduce that bug back? Finally, the patch seems to contain unrelated changes to multisession.el and its test suite? In general, please make a point of better explaining the rationale for the changes you propose, and, if you remove code added to fix specific bugs, please describe the testing you have done to verify those bugs are not re-introduced. Without that, how can we possibly accept changes that potentially make Emacs worse? Thanks.