GNOME Bugzilla – Bug 336850
Title-bar maintains rounded corner when maximizing big windows
Last modified: 2020-11-06 20:06:52 UTC
That bug has been opened on https://launchpad.net/distros/ubuntu/+source/metacity/+bug/35519 "-Resize any window to cover all the screen. -Maximize the window. -The title bar didn't change as it should; it keeps the rounded corners."
Happening here as well, if the window is nearly maximized, only a few pixels spare (as close as you can reasonably get it) and maximize it, you get rounded maximized corners. 2.14.0, Ubuntu Dapper... Tried with a wide variety of apps as well, Firefox, QCad, KTorrent, Evolution...
*** Bug 338143 has been marked as a duplicate of this bug. ***
I can confirm this bug using GNOME 2.14.1 under Ubuntu Dapper.
Playing around and noticed this does not happen for me if I have a panel on the top of the screen. If I move all panels to the bottom then it starts again.
Scratch that, still happens... Sorry for the spam :)
I've been trying to debug this bug. The bug is present in both Metacity CVS and Metacity in Dapper Drake. It seems like when you maximize a window, meta_window_maximize() triggers some code which makes meta_window_move_resize_internal() execute. But if the window is big enough, then the new size of the frame will be equal to the old size of the frame and then need_resize_frame will be FALSE. Ultimately that leads to meta_ui_apply_frame_shape() not being called so meta_frames_apply_shapes() isn't called either which, I think, is the function that controls whether window corners are rounded or not. I could be very wrong, but it seems the core of the problem is that maximizing a window doesn't set window->frame->need_reapply_frame_shape to TRUE.
Created attachment 69189 [details] [review] Make it so maximized windows do not have rounded corners This patch should fix the problem. It does so by strategically inserting: window->frame->need_reapply_frame_shape = TRUE; in meta_window_maximize_internal()
(Can I mark bugs as accepted-commit_now, or should it be commented-on?) I've tested this and it works beautifully. The only suggestion I have is that "the clients size" in the comment should be "the client's size".
*** Bug 349358 has been marked as a duplicate of this bug. ***
When will this patch be in the Ubuntu binpackages?
Looks good, thanks.
re: #10 Jan, I imagine that'd be a question for the Ubuntu developers. This is only the Gnome bug tracker. See this: https://launchpad.net/distros/ubuntu/+source/metacity/+bug/35519 for details.
It does seem a bit like we could have similar bugs in other cases... an ideal fix might be to always reapply the frame shape if we change what corner rounding we choose from the theme. This fix seems like a bit of a hack to me since it's not setting that need_reapply flag exactly in the place in the code where we know whether we need to reapply - it'll be setting it more often than strictly required, and perhaps not setting it in some cases that are required, no?
(Modifying patch state as per Havoc's comments)
The hack is in meta_window_maximize_internal(). And if the window is maximized, I think we can be sure that the frame shape needs to be reapplied because the window can then change from rounded to non-rounded corners. meta_frame_sync_to_window(), which is called from meta_window_move_resize_internal() has an if-block that looks like this: /* set bg to none to avoid flicker */ if (need_resize) { meta_ui_unflicker_frame_bg (frame->window->screen->ui, frame->xwindow, frame->rect.width, frame->rect.height); /* we need new shape if we're resized */ frame->need_reapply_frame_shape = TRUE; } This code (I think) is supposed to reapply the frame shape each time the window changes size, which is much more often than each time the window is maximized. :) I believe you could get rid of the "frame->need_reapply_frame_shape = TRUE" statement here because the frame shape only needs to be reapplied when the frame's state (maximized, unmaximized, shaded, etc) changes.
Not saying the existing code in move_resize_internal is right; simply that if we start scattering need_reapply_frame_shape = TRUE into all the functions that maximize, unmaximize, shade, etc. the result is going to be a little messy and easy to get wrong. And if we put it in only one of those, it seems we're closing only one bug when there are likely a number of related bugs. I don't know if there's a very easy way to fix this more comprehensively or not... I'd look at where metacity decides which style in the theme to use, and see if there's a way to factor that out and check whether it changed ? It's possible trying to do this right will make a flakier/uglier mess than just putting need_reapply_frame_shape = true in all the state change functions, so we should maybe just do that - but should think about whether it's needed in any others besides maximize? The shape does always need to be reapplied on resize iirc - remember it's just a bitmask, of a fixed size. So if not reapplied it would not line up with the corners after resizing.
Yeah! I agree with that. If we assume that the existing code that sets frame->need_reapply_frame_shape = TRUE may not be correct, then the bug is easier to solve correctly. But what do you say about the patch that was applied a few days ago? Should it be reverted? In that case someone else has to do the reversion as my CVS-fu isn't strong enough. IMHO, it could work as an interim solution until a real solution is found. I'll try to look at the code which changes the frame style where probably the real fix should be.
I don't think it has to be reverted, I'd just be sure some bug was left open about a full fix and/or look into a full fix
The only code that seem to be always called whenever the frame state changes (maximize, unmaximize, shade, focus, unfocus etc.) is the set_net_wm_state() function in window.c. So in this patch I moved the fix there. But just setting frame->need_reapply_frame_shape = TRUE; wasn't enough, because the code that actually reapplies the shape isn't called. For example, if you have a window maximized, whose unmaximized size is equal to its maximized size, and then you unmaximize it, the border rounding isn't reapplied. You have to look carefully to notice it but the bug is there. So I added a new function to frame.c, meta_frame_reapply_shape() which reapplies the shape for the frame immidiately. I also found that it was possible to replace all "frame->need_reapply_frame_shape = TRUE;" lines with direct calls to meta_frame_reapply_shape(). So I did that because it made the code less complex.
Created attachment 70148 [details] [review] Makes it so the frame shape is always updated when the window changes state
Reapplying the shape is pretty expensive... I would tend to at least experimentally verify we aren't doing it way too often in this case... e.g. do we ever short-circuit set_net_wm_state or just always do it on recalc_window_features (which we call a lot) ... if you put debug spew in reapply_frame_shape then look at the verbose log, how many calls happen if e.g. you tab between windows or something One solution might be to keep need_reapply_frame_shape but check this flag in the _draw_ idle handler in addition to the resize handler? On a more nitpick side, here's a comment left but the code it describes deleted: /* Done before the window resize, because doing it before means * part of the window being resized becomes unshaped, which may * be sort of hard to see with bg = None. If we did it after * window resize, part of the window being resized would become * shaped, which might be more visible. */ - update_shape (frame); -
I'm certain that reapplying the frame shape in set_net_wm_state() isn't a problem. Here is how: In the meta_frames_apply_shape() function I added a 100 ms delay usleep(100 * 1000). This makes resizing dirt slow because meta_frames_apply_shape() is called many times per second when resizing. However, actions that involves set_net_wm_state() such as meta_window_maximize() only becomes slightly lagged. That is because they generally only call set_net_wm_state() once. Tabbing between windows doesn't seem to call meta_frames_apply_shape() at all, which is how I think it should be - none of the frame shapes are changed at all. Changing workspace, OTOH, seem to call set_net_wm_state() for each window in both workspaces. But it is pretty hard to measure the performance impact of that. But the performance loss should logically still be smaller than the performance loss from meta_window_move_resize_internal()'s call to meta_frames_apply_shape(). IMHO, since there doesn't seem to be a performance problem with meta_frames_apply_shape() there is no reason to try to minimize the nr of calls to it with a flag.
*** Bug 352390 has been marked as a duplicate of this bug. ***
I'm noticing the problem isn't specific to rounded corners, and the entire title-bar doesn't change. That includes the "minimize/maximize/close" buttons. Moving the mouse cursor a single pixel over those three buttons redraws the entire title-bar to the correct state.
This is still present in Gnome 2.26.1. It happens with different themes. Comment 24 is still correct. The redraw also occurs if the mouse goes over the top-right corner icon-button.
bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all old bug reports in Bugzilla which have not seen updates for many years. If you can still reproduce this issue in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/metacity/-/issues/ Thank you for reporting this issue and we are sorry it could not be fixed.