* bug#60210: 30.0.50; tab-bar height not recalculated when face changes @ 2022-12-19 23:04 Gabriel do Nascimento Ribeiro 2022-12-25 8:35 ` Juri Linkov 0 siblings, 1 reply; 16+ messages in thread From: Gabriel do Nascimento Ribeiro @ 2022-12-19 23:04 UTC (permalink / raw) To: 60210 Steps: 1) emacs -Q (master "ae91da52335aafaff5405a49c23460082dfb460d") 2) Enable tab-bar: (tab-bar-mode +1) 3) Change some face attribute of tab-bar to force increase the tab-bar height, e.g: (set-face-attribute 'tab-bar nil :box 4) (set-face-attribute 'tab-bar nil :height 1.2) Result: the new face is displayed and the tab-bar height is properly adjusted to reflect the new height. 4) Reset face attributes of tab-bar to force original height, e.g.: (set-face-attribute 'tab-bar nil :box nil) (set-face-attribute 'tab-bar nil :height 1.0) Result: the new face is displayed, but the tab-bar height is not properly adjusted to reflect the new height. --- Gabriel ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2022-12-19 23:04 bug#60210: 30.0.50; tab-bar height not recalculated when face changes Gabriel do Nascimento Ribeiro @ 2022-12-25 8:35 ` Juri Linkov 2022-12-25 9:29 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: Juri Linkov @ 2022-12-25 8:35 UTC (permalink / raw) To: Gabriel do Nascimento Ribeiro; +Cc: 60210 > 2) Enable tab-bar: > > (tab-bar-mode +1) > > 3) Change some face attribute of tab-bar to force increase the tab-bar > height, e.g: > > (set-face-attribute 'tab-bar nil :box 4) > (set-face-attribute 'tab-bar nil :height 1.2) > > Result: the new face is displayed and the tab-bar height is properly > adjusted to reflect the new height. > > 4) Reset face attributes of tab-bar to force original height, e.g.: > > (set-face-attribute 'tab-bar nil :box nil) > (set-face-attribute 'tab-bar nil :height 1.0) > > Result: the new face is displayed, but the tab-bar height is not > properly adjusted to reflect the new height. It seems this is related to recent bug reports about fonts such as bug#52493, bug#58912, bug#59271, bug#59285, etc. since there is no such problem in Emacs 28. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2022-12-25 8:35 ` Juri Linkov @ 2022-12-25 9:29 ` Eli Zaretskii 2022-12-26 0:24 ` Gregory Heytings 0 siblings, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2022-12-25 9:29 UTC (permalink / raw) To: Juri Linkov; +Cc: 60210, gabriel376 > Cc: 60210@debbugs.gnu.org > From: Juri Linkov <juri@linkov.net> > Date: Sun, 25 Dec 2022 10:35:10 +0200 > > > 2) Enable tab-bar: > > > > (tab-bar-mode +1) > > > > 3) Change some face attribute of tab-bar to force increase the tab-bar > > height, e.g: > > > > (set-face-attribute 'tab-bar nil :box 4) > > (set-face-attribute 'tab-bar nil :height 1.2) > > > > Result: the new face is displayed and the tab-bar height is properly > > adjusted to reflect the new height. > > > > 4) Reset face attributes of tab-bar to force original height, e.g.: > > > > (set-face-attribute 'tab-bar nil :box nil) > > (set-face-attribute 'tab-bar nil :height 1.0) > > > > Result: the new face is displayed, but the tab-bar height is not > > properly adjusted to reflect the new height. > > It seems this is related to recent bug reports about fonts > such as bug#52493, bug#58912, bug#59271, bug#59285, etc. > since there is no such problem in Emacs 28. I doubt the font-related changes are relevant, but bisecting will be welcome. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2022-12-25 9:29 ` Eli Zaretskii @ 2022-12-26 0:24 ` Gregory Heytings 2022-12-26 12:23 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: Gregory Heytings @ 2022-12-26 0:24 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 60210, gabriel376, Juri Linkov [-- Attachment #1: Type: text/plain, Size: 141 bytes --] > > I doubt the font-related changes are relevant, but bisecting will be > welcome. > Indeed, the culprit is 5db4ec20fe. Patch attached. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Improve-handling-of-tab-bar-height.patch --] [-- Type: text/x-diff; name=Improve-handling-of-tab-bar-height.patch, Size: 1430 bytes --] From f2081576513e8b6fba71e67f36622cebe915c4c4 Mon Sep 17 00:00:00 2001 From: Gregory Heytings <gregory@heytings.org> Date: Mon, 26 Dec 2022 00:20:59 +0000 Subject: [PATCH] Improve handling of tab-bar height. * src/xdisp.c (redisplay_tab_bar): When 'auto-resize-tab-bar' is not 'grow-only', also consider the case when the tab-bar height needs to shrink. Fixes bug#60210. --- src/xdisp.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/xdisp.c b/src/xdisp.c index ea2d11e8b4e..c9b3b187fe2 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -14271,12 +14271,14 @@ redisplay_tab_bar (struct frame *f) frame_default_tab_bar_height = new_height; } - /* If new_height or new_nrows indicate that we need to enlarge the - tab-bar window, we can return right away. */ + /* If new_height or new_nrows indicate that we need to enlarge or + shrink the tab-bar window, we can return right away. */ if (new_nrows > f->n_tab_bar_rows || (EQ (Vauto_resize_tab_bars, Qgrow_only) && !f->minimize_tab_bar_window_p - && new_height > WINDOW_PIXEL_HEIGHT (w))) + && new_height > WINDOW_PIXEL_HEIGHT (w)) + || (! EQ (Vauto_resize_tab_bars, Qgrow_only) + && new_height < WINDOW_PIXEL_HEIGHT (w))) { if (FRAME_TERMINAL (f)->change_tab_bar_height_hook) FRAME_TERMINAL (f)->change_tab_bar_height_hook (f, new_height); -- 2.35.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2022-12-26 0:24 ` Gregory Heytings @ 2022-12-26 12:23 ` Eli Zaretskii 2022-12-26 13:36 ` Gregory Heytings 0 siblings, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2022-12-26 12:23 UTC (permalink / raw) To: Gregory Heytings; +Cc: 60210, gabriel376, juri > Date: Mon, 26 Dec 2022 00:24:11 +0000 > From: Gregory Heytings <gregory@heytings.org> > cc: Juri Linkov <juri@linkov.net>, 60210@debbugs.gnu.org, > gabriel376@hotmail.com > > > I doubt the font-related changes are relevant, but bisecting will be > > welcome. > > > > Indeed, the culprit is 5db4ec20fe. > > Patch attached. Thanks, this LGTM. If you tested this with tab bars of more than a single line, including when tabs are removed so a single line is once again long enough, please install on the release branch. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2022-12-26 12:23 ` Eli Zaretskii @ 2022-12-26 13:36 ` Gregory Heytings 2022-12-26 17:29 ` Juri Linkov 0 siblings, 1 reply; 16+ messages in thread From: Gregory Heytings @ 2022-12-26 13:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 60210, gabriel376, juri >>> I doubt the font-related changes are relevant, but bisecting will be >>> welcome. >> >> Indeed, the culprit is 5db4ec20fe. >> >> Patch attached. > > Thanks, this LGTM. If you tested this with tab bars of more than a > single line, including when tabs are removed so a single line is once > again long enough, please install on the release branch. > I did test it, but I don't use tab bars, so I'm not 110% sure. Juri, could you please test the patch? ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2022-12-26 13:36 ` Gregory Heytings @ 2022-12-26 17:29 ` Juri Linkov 2022-12-26 17:40 ` Gregory Heytings 0 siblings, 1 reply; 16+ messages in thread From: Juri Linkov @ 2022-12-26 17:29 UTC (permalink / raw) To: Gregory Heytings; +Cc: 60210, gabriel376, Eli Zaretskii >>>> I doubt the font-related changes are relevant, but bisecting will be >>>> welcome. >>> >>> Indeed, the culprit is 5db4ec20fe. >>> >>> Patch attached. >> >> Thanks, this LGTM. If you tested this with tab bars of more than >> a single line, including when tabs are removed so a single line is once >> again long enough, please install on the release branch. > > I did test it, but I don't use tab bars, so I'm not 110% sure. Juri, could > you please test the patch? Thanks for the patch. I tested it in emacs-29 with tab-bar-lines > 1 and see no problems. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2022-12-26 17:29 ` Juri Linkov @ 2022-12-26 17:40 ` Gregory Heytings 2023-01-01 17:56 ` Gregory Heytings 0 siblings, 1 reply; 16+ messages in thread From: Gregory Heytings @ 2022-12-26 17:40 UTC (permalink / raw) To: Juri Linkov; +Cc: 60210-done, gabriel376, Eli Zaretskii >>> Thanks, this LGTM. If you tested this with tab bars of more than a >>> single line, including when tabs are removed so a single line is once >>> again long enough, please install on the release branch. >> >> I did test it, but I don't use tab bars, so I'm not 110% sure. Juri, >> could you please test the patch? > > Thanks for the patch. I tested it in emacs-29 with tab-bar-lines > 1 > and see no problems. > Thanks! Pushed (b14bbd108e), and closing. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2022-12-26 17:40 ` Gregory Heytings @ 2023-01-01 17:56 ` Gregory Heytings 2023-01-01 18:22 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: Gregory Heytings @ 2023-01-01 17:56 UTC (permalink / raw) To: Juri Linkov, Eli Zaretskii; +Cc: 60210 > > Thanks! Pushed (b14bbd108e), and closing. > I got (privately) an interesting bug report about this commit. The bug is that, because of this change, the tab-bar is not displayed anymore (or is displayed but stays empty) in certain circumstances. It's a font-related bug again, so it might be difficult to reproduce. On my system, (set-face-attribute 'default nil :font "JetBrains Mono") (tab-bar-mode 1) triggers the bug. The bug is that, in redisplay_tab_bar, WINDOW_PIXEL_HEIGHT (w) uses the height of the default face, which is 39 pixels, whereas new_height, which is computed with tab_bar_height, uses the font of the tab-bar face (variable-pitch in emacs -Q). On my system, new_height is (with a single *scratch* tab) 36 pixels. Therefore new_height < WINDOW_PIXEL_HEIGHT (w), when in fact according to the logic of the code we should have new_height == WINDOW_PIXEL_HEIGHT (w). ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2023-01-01 17:56 ` Gregory Heytings @ 2023-01-01 18:22 ` Eli Zaretskii 2023-01-01 21:50 ` Gregory Heytings 0 siblings, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2023-01-01 18:22 UTC (permalink / raw) To: Gregory Heytings; +Cc: 60210, juri > Date: Sun, 01 Jan 2023 17:56:36 +0000 > From: Gregory Heytings <gregory@heytings.org> > cc: 60210@debbugs.gnu.org > > The bug is that, in redisplay_tab_bar, WINDOW_PIXEL_HEIGHT (w) uses the > height of the default face, which is 39 pixels, whereas new_height, which > is computed with tab_bar_height, uses the font of the tab-bar face > (variable-pitch in emacs -Q). On my system, new_height is (with a single > *scratch* tab) 36 pixels. Therefore new_height < WINDOW_PIXEL_HEIGHT (w), > when in fact according to the logic of the code we should have new_height > == WINDOW_PIXEL_HEIGHT (w). I'm not sure I understand how the above causes the tab bar not to be displayed, or become empty. AFAIU, it just means the frame's change_tab_bar_height_hook will be called. What did I miss? ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2023-01-01 18:22 ` Eli Zaretskii @ 2023-01-01 21:50 ` Gregory Heytings 2023-01-02 3:29 ` Eli Zaretskii 2023-01-02 15:05 ` Eli Zaretskii 0 siblings, 2 replies; 16+ messages in thread From: Gregory Heytings @ 2023-01-01 21:50 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 60210, juri >> The bug is that, in redisplay_tab_bar, WINDOW_PIXEL_HEIGHT (w) uses the >> height of the default face, which is 39 pixels, whereas new_height, >> which is computed with tab_bar_height, uses the font of the tab-bar >> face (variable-pitch in emacs -Q). On my system, new_height is (with a >> single *scratch* tab) 36 pixels. Therefore new_height < >> WINDOW_PIXEL_HEIGHT (w), when in fact according to the logic of the >> code we should have new_height == WINDOW_PIXEL_HEIGHT (w). > > I'm not sure I understand how the above causes the tab bar not to be > displayed, or become empty. AFAIU, it just means the frame's > change_tab_bar_height_hook will be called. What did I miss? > I do not fully understand it either yet. A simpler recipe, which does not involve changing fonts: (set-face-attribute 'tab-bar nil :height 0.5) (tab-bar-mode 1) This should display a tiny tab-bar, it displays a white bar instead. In redisplay_tab_bar, new_height is set to half the height of a canonical line (17 and 34 on my system). Therefore redisplay_tab_bar returns true. The next redisplay cycle finds that WINDOW_TOTAL_LINES (w) == 0, and does nothing. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2023-01-01 21:50 ` Gregory Heytings @ 2023-01-02 3:29 ` Eli Zaretskii 2023-01-02 15:05 ` Eli Zaretskii 1 sibling, 0 replies; 16+ messages in thread From: Eli Zaretskii @ 2023-01-02 3:29 UTC (permalink / raw) To: Gregory Heytings; +Cc: 60210, juri > Date: Sun, 01 Jan 2023 21:50:56 +0000 > From: Gregory Heytings <gregory@heytings.org> > cc: 60210@debbugs.gnu.org, juri@linkov.net > > > >> The bug is that, in redisplay_tab_bar, WINDOW_PIXEL_HEIGHT (w) uses the > >> height of the default face, which is 39 pixels, whereas new_height, > >> which is computed with tab_bar_height, uses the font of the tab-bar > >> face (variable-pitch in emacs -Q). On my system, new_height is (with a > >> single *scratch* tab) 36 pixels. Therefore new_height < > >> WINDOW_PIXEL_HEIGHT (w), when in fact according to the logic of the > >> code we should have new_height == WINDOW_PIXEL_HEIGHT (w). > > > > I'm not sure I understand how the above causes the tab bar not to be > > displayed, or become empty. AFAIU, it just means the frame's > > change_tab_bar_height_hook will be called. What did I miss? > > > > I do not fully understand it either yet. A simpler recipe, which does not > involve changing fonts: > > (set-face-attribute 'tab-bar nil :height 0.5) > (tab-bar-mode 1) > > This should display a tiny tab-bar, it displays a white bar instead. > > In redisplay_tab_bar, new_height is set to half the height of a canonical > line (17 and 34 on my system). Therefore redisplay_tab_bar returns true. > The next redisplay cycle finds that WINDOW_TOTAL_LINES (w) == 0, and does > nothing. Ah, if WINDOW_TOTAL_LINES becomes zero, that will indeed explain the problem. We should prevent that from happening as long as the tab bar is turned on. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2023-01-01 21:50 ` Gregory Heytings 2023-01-02 3:29 ` Eli Zaretskii @ 2023-01-02 15:05 ` Eli Zaretskii 2023-01-02 15:28 ` Gregory Heytings 1 sibling, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2023-01-02 15:05 UTC (permalink / raw) To: Gregory Heytings; +Cc: 60210, juri > Date: Sun, 01 Jan 2023 21:50:56 +0000 > From: Gregory Heytings <gregory@heytings.org> > cc: 60210@debbugs.gnu.org, juri@linkov.net > > > I'm not sure I understand how the above causes the tab bar not to be > > displayed, or become empty. AFAIU, it just means the frame's > > change_tab_bar_height_hook will be called. What did I miss? > > > > I do not fully understand it either yet. A simpler recipe, which does not > involve changing fonts: > > (set-face-attribute 'tab-bar nil :height 0.5) > (tab-bar-mode 1) > > This should display a tiny tab-bar, it displays a white bar instead. > > In redisplay_tab_bar, new_height is set to half the height of a canonical > line (17 and 34 on my system). Therefore redisplay_tab_bar returns true. > The next redisplay cycle finds that WINDOW_TOTAL_LINES (w) == 0, and does > nothing. Thanks, I hope I fixed this now on emacs-29, please re-test. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2023-01-02 15:05 ` Eli Zaretskii @ 2023-01-02 15:28 ` Gregory Heytings 2023-01-02 16:53 ` Eli Zaretskii 0 siblings, 1 reply; 16+ messages in thread From: Gregory Heytings @ 2023-01-02 15:28 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 60210, juri > > Thanks, I hope I fixed this now on emacs-29, please re-test. > Thanks, it works now! My guess that is was a rounding problem was right. But I'm curious: in *_change_tab_bar_height we set f->tab_bar_lines, and in redisplay_tab_bar we check w->total_lines. Where is the former used to set the latter? ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2023-01-02 15:28 ` Gregory Heytings @ 2023-01-02 16:53 ` Eli Zaretskii 2023-01-04 13:53 ` Gregory Heytings 0 siblings, 1 reply; 16+ messages in thread From: Eli Zaretskii @ 2023-01-02 16:53 UTC (permalink / raw) To: Gregory Heytings; +Cc: 60210, juri > Date: Mon, 02 Jan 2023 15:28:33 +0000 > From: Gregory Heytings <gregory@heytings.org> > cc: 60210@debbugs.gnu.org, juri@linkov.net > > > Thanks, I hope I fixed this now on emacs-29, please re-test. > > Thanks, it works now! My guess that is was a rounding problem was right. > But I'm curious: in *_change_tab_bar_height we set f->tab_bar_lines, and > in redisplay_tab_bar we check w->total_lines. Where is the former used to > set the latter? In adjust_frame_size and its subroutines, I guess. ^ permalink raw reply [flat|nested] 16+ messages in thread
* bug#60210: 30.0.50; tab-bar height not recalculated when face changes 2023-01-02 16:53 ` Eli Zaretskii @ 2023-01-04 13:53 ` Gregory Heytings 0 siblings, 0 replies; 16+ messages in thread From: Gregory Heytings @ 2023-01-04 13:53 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 60210, juri >>> Thanks, I hope I fixed this now on emacs-29, please re-test. >> >> Thanks, it works now! My guess that is was a rounding problem was >> right. But I'm curious: in *_change_tab_bar_height we set >> f->tab_bar_lines, and in redisplay_tab_bar we check w->total_lines. >> Where is the former used to set the latter? > > In adjust_frame_size and its subroutines, I guess. > Thanks, I'll have a look. The person who sent me the bug report confirmed that the bug is also fixed with his configuration, so all is well. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-01-04 13:53 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-19 23:04 bug#60210: 30.0.50; tab-bar height not recalculated when face changes Gabriel do Nascimento Ribeiro 2022-12-25 8:35 ` Juri Linkov 2022-12-25 9:29 ` Eli Zaretskii 2022-12-26 0:24 ` Gregory Heytings 2022-12-26 12:23 ` Eli Zaretskii 2022-12-26 13:36 ` Gregory Heytings 2022-12-26 17:29 ` Juri Linkov 2022-12-26 17:40 ` Gregory Heytings 2023-01-01 17:56 ` Gregory Heytings 2023-01-01 18:22 ` Eli Zaretskii 2023-01-01 21:50 ` Gregory Heytings 2023-01-02 3:29 ` Eli Zaretskii 2023-01-02 15:05 ` Eli Zaretskii 2023-01-02 15:28 ` Gregory Heytings 2023-01-02 16:53 ` Eli Zaretskii 2023-01-04 13:53 ` Gregory Heytings
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).