From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [RFC] per-frame fonts_changed and cursor_type_changed flags Date: Fri, 16 Nov 2012 15:30:38 +0200 Message-ID: <83ip95smip.fsf@gnu.org> References: <50A61FC8.6050408@yandex.ru> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1353072678 10787 80.91.229.3 (16 Nov 2012 13:31:18 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 16 Nov 2012 13:31:18 +0000 (UTC) Cc: emacs-devel@gnu.org To: Dmitry Antipov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Nov 16 14:31:29 2012 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1TZM12-0002bH-So for ged-emacs-devel@m.gmane.org; Fri, 16 Nov 2012 14:31:29 +0100 Original-Received: from localhost ([::1]:47598 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TZM0s-0001RX-SV for ged-emacs-devel@m.gmane.org; Fri, 16 Nov 2012 08:31:18 -0500 Original-Received: from eggs.gnu.org ([208.118.235.92]:53460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TZM0o-0001Qf-1X for emacs-devel@gnu.org; Fri, 16 Nov 2012 08:31:17 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TZM0k-0006Dw-VD for emacs-devel@gnu.org; Fri, 16 Nov 2012 08:31:13 -0500 Original-Received: from mtaout21.012.net.il ([80.179.55.169]:61246) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TZM0k-0006Cq-NR for emacs-devel@gnu.org; Fri, 16 Nov 2012 08:31:10 -0500 Original-Received: from conversion-daemon.a-mtaout21.012.net.il by a-mtaout21.012.net.il (HyperSendmail v2007.08) id <0MDL003001H0FX00@a-mtaout21.012.net.il> for emacs-devel@gnu.org; Fri, 16 Nov 2012 15:31:09 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout21.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MDL003QY1JW8270@a-mtaout21.012.net.il>; Fri, 16 Nov 2012 15:31:09 +0200 (IST) In-reply-to: <50A61FC8.6050408@yandex.ru> X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 X-Received-From: 80.179.55.169 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:154878 Archived-At: > Date: Fri, 16 Nov 2012 15:13:12 +0400 > From: Dmitry Antipov > > This patch tries to offload redisplay from some work, assuming that > changing frame font and per-frame cursor type should invalidate only > the affected frame. Comments are definitely welcome. Some comments below. But ultimately, the only practical way of knowing if the patch is correct is to test it in variety of situations and in different builds (with and without X, with and without toolkits, with and without GTK, etc.). The display engine is not written to any formal specification, so its use of flags is anything but regular and disciplined. > @@ -446,7 +435,7 @@ > || right != matrix->right_margin_glyphs); > > if (!marginal_areas_changed_p > - && !fonts_changed_p > + && !XFRAME (w->frame)->fonts_changed Did that work for you? You cannot safely dereference w here, because it can be NULL (on TTY frames, see the commentary before the function). E.g., adjust_frame_glyphs_for_frame_redisplay calls this function that way. > @@ -13044,12 +13048,7 @@ > > /* If new fonts have been loaded that make a glyph matrix adjustment > necessary, do it. */ > - if (fonts_changed_p) > - { > - adjust_glyphs (NULL); > - ++windows_or_buffers_changed; > - fonts_changed_p = 0; > - } > + frame_fonts_changed (fr); 'fr' is the frame of the selected window on entry to redisplay_internal. Why do you assume that only that frame's fonts are involved here? E.g., the call to select_frame_for_redisplay, just above this hunk, could have changed the selected frame for redisplay. > @@ -13176,7 +13177,7 @@ > if (!display_last_displayed_message_p) > message_cleared_p = 0; > > - if (fonts_changed_p) > + if (fr->fonts_changed) > goto retry; Similarly here: I think you need to test the fonts_changed flags of all the frames, because not necessarily redisplaying frame 'fr'. > We can return without actually redisplaying the window if > - fonts_changed_p. In that case, redisplay_internal will > - retry. */ > + fonts of it's frame was changed. In that case, redisplay_internal > + will retry. */ "its", not "it's" (the latter is a short for "it is").