From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: martin rudalics Newsgroups: gmane.emacs.bugs Subject: bug#38181: Actual height of mode-line not taken into account Date: Tue, 5 May 2020 18:57:06 +0200 Message-ID: <86491f07-ed68-2880-17a0-a60ed40d5059@gmx.at> References: <87eeyd3ul0.fsf@bernoul.li> <83d0dt2qt6.fsf@gnu.org> <83r2290w24.fsf@gnu.org> <83pnhs6wwp.fsf@gnu.org> <83k1806qca.fsf@gnu.org> <8336en7giv.fsf@gnu.org> <81264049-4f88-fae7-6448-e0ac5d977268@gmx.at> <83a78u5s8y.fsf@gnu.org> <01cee2f7-aeb5-4eb1-b2d5-e056c91eab8b@gmx.at> <83wo5rolsc.fsf@gnu.org> <4682372c-e25a-dc62-842f-c3971f79bb16@gmx.at> <838si6mnsy.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="5342"; mail-complaints-to="usenet@ciao.gmane.io" Cc: jonas@bernoul.li, 38181@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue May 05 18:58:33 2020 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 1jW0ts-0001G0-Tz for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 05 May 2020 18:58:33 +0200 Original-Received: from localhost ([::1]:43416 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jW0tr-0001EG-Or for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 05 May 2020 12:58:32 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:36016) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jW0tO-0001D6-4D for bug-gnu-emacs@gnu.org; Tue, 05 May 2020 12:58:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:53926) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jW0tN-0004m5-Rj for bug-gnu-emacs@gnu.org; Tue, 05 May 2020 12:58:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jW0tN-0003xc-R3 for bug-gnu-emacs@gnu.org; Tue, 05 May 2020 12:58:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: martin rudalics Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 05 May 2020 16:58:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 38181 X-GNU-PR-Package: emacs Original-Received: via spool by 38181-submit@debbugs.gnu.org id=B38181.158869783715157 (code B ref 38181); Tue, 05 May 2020 16:58:01 +0000 Original-Received: (at 38181) by debbugs.gnu.org; 5 May 2020 16:57:17 +0000 Original-Received: from localhost ([127.0.0.1]:37239 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jW0se-0003wP-NY for submit@debbugs.gnu.org; Tue, 05 May 2020 12:57:17 -0400 Original-Received: from mout.gmx.net ([212.227.17.21]:51311) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jW0sc-0003w8-EA for 38181@debbugs.gnu.org; Tue, 05 May 2020 12:57:15 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1588697827; bh=ngEO6D/GN77AE+F6iDbJ5RVlPNHGpYzM2vZrIm1PmHA=; h=X-UI-Sender-Class:Subject:To:Cc:References:From:Date:In-Reply-To; b=e5jsI3HZtNgd9bQF1/Hyb+Xc3fPT04aYXSSqRqhtjTyJAIHMZPmchwWYNt8UjaNCL JtYRL025cwlm+YkkzuWredX4roZOFjQnkIU5YGiiHZBUAhyZWVwYcEgEDDyVOqulDN KftkwGPptMGMUbaPUzslTtVAGyTlaUGrEvoZRbpY= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Original-Received: from [192.168.1.101] ([46.125.249.75]) by mail.gmx.com (mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id 1Mlw3X-1ioBbE0GUV-00j4xl; Tue, 05 May 2020 18:57:07 +0200 In-Reply-To: <838si6mnsy.fsf@gnu.org> Content-Language: en-US X-Provags-ID: V03:K1:xhWCKRFIGjdoO2QQ6bsy1Do2i8y9Z0j95rafhBIU7Ra+2rwKKGu hS973gHqkJHVrKQ9AISpMzN9JCgIxgYGQWNG7GDnGMBSFwYKBJjYaPVHmIOuoYqOtGiPS+S WoljerUMIUQwN8swRE8t9HB4qQgP2vFjMsuMIAX5PrVDW2OeRspUE+CWob7uM4IQHT5NwRd T4cQI/Wcw5xxRVCuE/ptQ== X-UI-Out-Filterresults: notjunk:1;V03:K0:c1b2RoJ83RU=:9tqMmKdGiZ9CsASChv2IlK kh6FYtACXIP38xcpvFnMGvCtJ+rlovCde7vhnP1fFG5tRfTEigFIHBUL1q/o4YitOdeI8CFn+ gq2R309UZslvr7eiFbQav8AETECtniQPtTDeUZXfGYNIt2+zkvOIr7HfBjHW57m3bJ0ESeL3N LOorvRFQnZjRsh+p6sAe3mUWcN6g89aCaVYCUEWO5tRTuujL9D0x80jmMNQKogExX/aEwMXMU d7liX9w2HhLaFXXM3Druqb2T1j6lT/Wke4+tbtPW3OfpfuF6WcWGj1wuAHlZxyYt4zmk7OTwC LaNhUVKPWgSUWvocAbMquKMqNFWBL2u0qDasApagEjZl11QB5/DsTnG8fFjunmJZv7RK1ID1I WW6XgZKqliYqThUf+Zm5cuOz4pwhwIHAOIUWi5O7CCDp0QJtxlHgJNVep5q0waAxfe0LGzm+R N98zwoMtCD1agrVcmFeCtyD+GkKG55/wUDC9zuH+Q/TFR4bStsL2Eu4typrWTrpCLdWrW6/lQ wddAzBW1yF6EdXX8rqaoY6ViAAQ/zwLou+DgQEVG+eAmBnrBeTscihhaaPj7Da/Rsw7FD3qRW X9NPdNhFxfnGR8wVeKO99vd+yYj2zYU/3kIfFiFgla0FlCLoNzS8zGS0zxjTJBkmuQBSaueDp iiVBcqTqBIzT69BF+SPEr7mDNXEksJDj5yH1Ll4PZb8Mkjujy5495lDl0UH22oHjNo08Emu2q xodongrAL3tZyOeEKXFcA+hbSnI7dQM54E0NtN6UFQVLpv8H7cp8GFdaPC0Wl9UjHvMa7vP0 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:179743 Archived-At: >> Below find two backtraces with emacs -Q where PRODUCE_GLYPHS here sets >> inhibit_free_realized_faces outside the scope of redisplay_internal >> (that is, while redisplaying_p is nil) such that these will not get >> caught by the unbind form in redisplay_internal. > > Thanks. All of these enter redisplay via echo_area_display. That > calls redisplay_internal, but only after it displays the mini-window. > > However, I had my doubts regarding the accuracy of my mental model. > Namely, the part where I said that inhibit_free_realized_faces should > never be non-zero outside of redisplay. So I looked at the code and > its history, and it turns out I was wrong: the line that sets the flag > in PRODUCE_GLYPHS was there since Emacs 21, and I see the flag set to > non-zero all the way back to Emacs 22.1 (and probably earlier). C-x v l told me it was commit 18155a1d5a93cec23255becab15f26e77cace9e7 Author: Richard M. Stallman Date: Thu Aug 29 14:41:28 2002 +0000 (PRODUCE_GLYPHS): Set inhibit_free_realized_faces when iterator is adding glyphs to a glyph matrix. but I didn't dig any further. > So it sounds like we always were running like that. Therefore, I must > turn the table and ask you to please describe in more detail, > preferably with Lisp snippets to try, how the fact that this flag is > non-zero gets in the way of something we need to do with faces, both > in this bug and the other one you mentioned. I'd like to understand > better how this flag interferes in these use cases. With Bug#40639 and Bug#41071 the situation is simple: The internal border changed face but when we enter init_iterator inhibit_free_realized_faces is true and we don't handle it. Redisplay takes care of the face change for normal frames only when it evaluates their titles and that strange incident is what I used to fix these bugs on master. The situation with the present bug is that I rewrote 'window-text-pixel-size' as DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 6, 0, doc: /* Return the size of the text of WINDOW's buffer in pixels. WINDOW must be a live window and defaults to the selected one. The return value is a cons of the maximum pixel-width of any text line and the maximum pixel-height of all text lines. The optional argument FROM, if non-nil, specifies the first text position and defaults to the minimum accessible position of the buffer. If FROM is t, use the minimum accessible position that starts a non-empty line. TO, if non-nil, specifies the last text position and defaults to the maximum accessible position of the buffer. If TO is t, use the maximum accessible position that ends a non-empty line. The optional argument X-LIMIT, if non-nil, specifies the maximum text width that can be returned. X-LIMIT nil or omitted, means to use the pixel-width of WINDOW's body; use this if you want to know how high WINDOW should be become in order to fit all of its buffer's text with the width of WINDOW unaltered. Use the maximum width WINDOW may assume if you intend to change WINDOW's width. In any case, text whose x-coordinate is beyond X-LIMIT is ignored. Since calculating the width of long lines can take some time, it's always a good idea to make this argument as small as possible; in particular, if the buffer contains long lines that shall be truncated anyway. The optional argument Y-LIMIT, if non-nil, specifies the maximum text height (excluding the height of the mode- or header-line, if any) that can be returned. Text lines whose y-coordinate is beyond Y-LIMIT are ignored. Since calculating the text height of a large buffer can take some time, it makes sense to specify this argument if the size of the buffer is large or unknown. Optional argument MODE-AND-HEADER-LINE nil or omitted means do not include the height of the mode- or header-line of WINDOW in the return value. If it is either the symbol `mode-line' or `header-line', include only the height of that line, if present, in the return value. If t, include the height of both, if present, in the return value. */) (Lisp_Object window, Lisp_Object from, Lisp_Object to, Lisp_Object x_limit, Lisp_Object y_limit, Lisp_Object mode_and_header_line) { struct window *w = decode_live_window (window); Lisp_Object buffer = w->contents; struct buffer *b; struct it it; struct buffer *old_b = NULL; ptrdiff_t start, end, pos; struct text_pos startp; void *itdata = NULL; int c, max_x = 0, max_y = 0, x = 0, y = 0; CHECK_BUFFER (buffer); b = XBUFFER (buffer); if (b != current_buffer) { old_b = current_buffer; set_buffer_internal (b); } if (NILP (from)) start = BEGV; else if (EQ (from, Qt)) { start = pos = BEGV; while ((pos++ < ZV) && (c = FETCH_CHAR (pos)) && (c == ' ' || c == '\t' || c == '\n' || c == '\r')) start = pos; while ((pos-- > BEGV) && (c = FETCH_CHAR (pos)) && (c == ' ' || c == '\t')) start = pos; } else start = clip_to_bounds (BEGV, fix_position (from), ZV); if (NILP (to)) end = ZV; else if (EQ (to, Qt)) { end = pos = ZV; while ((pos-- > BEGV) && (c = FETCH_CHAR (pos)) && (c == ' ' || c == '\t' || c == '\n' || c == '\r')) end = pos; while ((pos++ < ZV) && (c = FETCH_CHAR (pos)) && (c == ' ' || c == '\t')) end = pos; } else end = clip_to_bounds (start, fix_position (to), ZV); if (!NILP (x_limit) && RANGED_FIXNUMP (0, x_limit, INT_MAX)) max_x = XFIXNUM (x_limit); if (NILP (y_limit)) max_y = INT_MAX; else if (RANGED_FIXNUMP (0, y_limit, INT_MAX)) max_y = XFIXNUM (y_limit); /* Recompute faces if needed. */ if (face_change || XFRAME (w->frame)->face_change) { Lisp_Object window_header_line_format = window_parameter (w, Qheader_line_format); Lisp_Object window_tab_line_format = window_parameter (w, Qtab_line_format); Lisp_Object window_mode_line_format = window_parameter (w, Qmode_line_format); if (((EQ (mode_and_header_line, Qheader_line) || EQ (mode_and_header_line, Qt)) && !EQ (window_header_line_format, Qnone) && (!NILP (window_header_line_format) || !NILP (BVAR (current_buffer, header_line_format)))) || ((EQ (mode_and_header_line, Qtab_line) || EQ (mode_and_header_line, Qt)) && !EQ (window_tab_line_format, Qnone) && (!NILP (window_tab_line_format) || !NILP (BVAR (current_buffer, tab_line_format)))) || ((EQ (mode_and_header_line, Qmode_line) || EQ (mode_and_header_line, Qt)) && !EQ (window_mode_line_format, Qnone) && (!NILP (window_mode_line_format) || !NILP (BVAR (current_buffer, mode_line_format))))) { if (face_change) { face_change = false; XFRAME (w->frame)->face_change = 0; free_all_realized_faces (Qnil); } else { XFRAME (w->frame)->face_change = 0; free_all_realized_faces (w->frame); } } } itdata = bidi_shelve_cache (); SET_TEXT_POS (startp, start, CHAR_TO_BYTE (start)); start_display (&it, w, startp); /* It makes no sense to measure dimensions of region of text that crosses the point where bidi reordering changes scan direction. By using unidirectional movement here we at least support the use case of measuring regions of text that have a uniformly R2L directionality, and regions that begin and end in text of the same directionality. */ it.bidi_p = false; int move_op = MOVE_TO_POS | MOVE_TO_Y; int to_x = -1; if (!NILP (x_limit)) { it.last_visible_x = max_x; /* Actually, we never want move_it_to stop at to_x. But to make sure that move_it_in_display_line_to always moves far enough, we set to_x to INT_MAX and specify MOVE_TO_X. */ move_op |= MOVE_TO_X; to_x = INT_MAX; } void *it2data = NULL; struct it it2; SAVE_IT (it2, it, it2data); x = move_it_to (&it, end, to_x, max_y, -1, move_op); /* We could have a display property at END, in which case asking move_it_to to stop at END will overshoot and stop at position after END. So we try again, stopping before END, and account for the width of the last buffer position manually. */ if (IT_CHARPOS (it) > end) { end--; RESTORE_IT (&it, &it2, it2data); x = move_it_to (&it, end, to_x, max_y, -1, move_op); /* Add the width of the thing at TO, but only if we didn't overshoot it; if we did, it is already accounted for. Also, account for the height of the thing at TO. */ if (IT_CHARPOS (it) == end) { x += it.pixel_width; it.max_ascent = max (it.max_ascent, it.ascent); it.max_descent = max (it.max_descent, it.descent); } } if (!NILP (x_limit)) { /* Don't return more than X-LIMIT. */ if (x > max_x) x = max_x; } /* Subtract height of header-line which was counted automatically by start_display. */ y = it.current_y + it.max_ascent + it.max_descent - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w); /* Don't return more than Y-LIMIT. */ if (y > max_y) y = max_y; if (EQ (mode_and_header_line, Qtab_line) || EQ (mode_and_header_line, Qt)) { Lisp_Object window_tab_line_format = window_parameter (w, Qtab_line_format); if (!EQ (window_tab_line_format, Qnone) && (!NILP (window_tab_line_format) || !NILP (BVAR (current_buffer, tab_line_format)))) /* Re-add height of tab-line as requested. */ y = y + display_mode_line (w, TAB_LINE_FACE_ID, NILP (window_tab_line_format) ? BVAR (current_buffer, tab_line_format) : window_tab_line_format); } if (EQ (mode_and_header_line, Qheader_line) || EQ (mode_and_header_line, Qt)) { Lisp_Object window_header_line_format = window_parameter (w, Qheader_line_format); if (!EQ (window_header_line_format, Qnone) && (!NILP (window_header_line_format) || !NILP (BVAR (current_buffer, header_line_format)))) /* Re-add height of header-line as requested. */ y = y + display_mode_line (w, HEADER_LINE_FACE_ID, NILP (window_header_line_format) ? BVAR (current_buffer, header_line_format) : window_header_line_format); } if (EQ (mode_and_header_line, Qmode_line) || EQ (mode_and_header_line, Qt)) { Lisp_Object window_mode_line_format = window_parameter (w, Qmode_line_format); if (!EQ (window_mode_line_format, Qnone) && (!NILP (window_mode_line_format) || !NILP (BVAR (current_buffer, mode_line_format)))) /* Add height of mode-line as requested. */ y = y + display_mode_line (w, CURRENT_MODE_LINE_FACE_ID (w), NILP (window_mode_line_format) ? BVAR (current_buffer, mode_line_format) : window_mode_line_format); } bidi_unshelve_cache (itdata, false); if (old_b) set_buffer_internal (old_b); return Fcons (make_fixnum (x), make_fixnum (y)); } This takes any face change into consideration and works. But it obviously disregards inhibit_free_realized_faces and so I'm not sure whether it's TRT (rather, I'm quite confident that it is not TRT). At the very least I probably should set windows_or_buffers_changed. There is this comment in redisplay_internal /* If face_change, init_iterator will free all realized faces, which includes the faces referenced from current matrices. So, we can't reuse current matrices in this case. */ if (face_change) windows_or_buffers_changed = 47; which hints at that (without considering that f->face_change is not handled there). > Thanks (and sorry for spreading misinformation: I was somehow > confident that it was myself who added the setting of > inhibit_free_realized_faces in PRODUCE_GLYPHS, but the truth is that > it was Gerd, long ago). Nevertheless, the fact that inhibit_free_realized_faces is true when entering the iterator after a face change is IMO a bug. We apparently always run the iterator with the old faces first. Only after we have (incidentally?) detected some change that forces us to "retry", we have redisplay set inhibit_free_realized_faces to false itself and the face change will get applied in the next iteration. If so, the outcome of the previous iterations gets lost if a face changed there. Does such a strategy give us any gains? Again, the question I tried to ask earlier was: Does a current matrix in between two redisplays rely on the old realized faces? If so, the answer is that inhibit_free_realized_faces should be always true but for the small window within retrying in redisplay_internal. And maybe somewhere at the beginning of redisplay when a face change occurred for the window's frame and we clear all matrices therefore and avoid any redisplay optimizations. In either case, it strikes me as a strange idea that redisplay_internal saves and restores the value of a variable which is apparently always true when it is entered (I suppose redisplay_internal is not re-entrant given /* I don't think this happens but let's be paranoid. */ if (redisplaying_p) return; ) but maybe there are exceptions I don't know. martin