Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added 'disable_titlebar' option #8026

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

neuromagus
Copy link

@neuromagus neuromagus commented Mar 1, 2024

I fix my previous commit to master branch.

My patch fixed this issue #7409 (comment)

and you fixed the rendering so my patch is more trivial. Amazing work, guys.

@WhyNotHugo
Copy link
Contributor

WhyNotHugo commented Mar 1, 2024

I would like to see the ability to toggle this on a per-window basis (much like we can toggle floating per-window).

That said, I have absolutely no objection to this approach being merged.

@neuromagus
Copy link
Author

neuromagus commented Mar 1, 2024

@WhyNotHugo

I would like to see the ability to toggle this on a per-window basis (much like we can toggle floating per-window).

Hmm, why not? Particularly this commit focused on total disable any titlebars. IMO, need to add another option and (new commit) way for hotkey with toggle behavior switcher. If need this stuff, again, why not?

For example - press Mod+t - disable titlebar on focused window? Am I right?

P.S. My English is so terrible, sorry for that.

@y0nei
Copy link

y0nei commented Mar 1, 2024

We need this

@neuromagus
Copy link
Author

@y0nei watch this

@neuromagus
Copy link
Author

neuromagus commented Mar 4, 2024

Following feedback from @y0nei, this pull request need to be update to render the top border if the titlebar is not render visible. In my patch for 1.9 version this behavior fixed.

I think I'll fix it here later... Sway 1.9 came out a week or two ago xD and I prefer stable version of Wlroots, because Wlroots-dev did not like my mpv player...

@neuromagus
Copy link
Author

@Nefsen402. I need a help with this patch. In this commit 869baff u reorganize structure of Sway project (clear sway/desktop/ folder, and this is amazing (removed render behind scene)) and move render to the sway/desktop/transaction.c. Maybe I'm wrong and the render is located in a different place. So here it says a big tree with a rendering boundary (NONE, PIXEL, NORMAL), titlebar and stuff. I see, u are trying to fix removing or not rendering the Titlebar. Here in the master manpage:

*border* none|normal|csd|pixel [<n>]
    Set border style for focused window. _normal_ includes a border of
    thickness _n_ and a title bar. _pixel_ is a border without title bar _n_
    pixels thick. Default is _normal_ with border thickness 2. _csd_ is short
    for client-side-decorations, which allows the client to draw its own
    decorations.

This behavior did not work for me. For example i set in config default_border pixel 10 - Titlebar showed. With my hack (this pull request) for rendering "top border" need to be added (t/f my option) for rendering top border which will complicate the design of an already complex code, IMHO.

I mean, maybe this area (line 288 static void arrange_children() and line 381, static void arrange_container()) needs refactoring? Here is my diff for Sway1.9

Sorry, this is my first request and I don't know the rules or form for making such requests.

@Nefsen402
Copy link
Contributor

container_titlebar_height is only used when using the B_NORMAL mode for borders. B_PIXEL will use a size determined by container->current.border_top. I think your patch should have correct behavior if instead of trying to change container_titlebar_height in any way. This should instead work:

diff --git a/sway/desktop/transaction.c b/sway/desktop/transaction.c
index 042141ab..4895add1 100644
--- a/sway/desktop/transaction.c
+++ b/sway/desktop/transaction.c
@@ -392,6 +392,10 @@ static void arrange_container(struct sway_container *con,
                int border_top = container_titlebar_height();
                int border_width = con->current.border_thickness;
 
+               if (wants to disable title bar) {
+                       title_bar = false;
+               }
+
                if (title_bar && con->current.border != B_NORMAL) {
                        wlr_scene_node_set_enabled(&con->title_bar.tree->node, false);
                        wlr_scene_node_set_enabled(&con->border.top->node, true);

However, it might be beneficial to keep the current behavior. The top border size can be manually set to 0 by the user if they truly don't want any top decorations of any kind.

@neuromagus
Copy link
Author

neuromagus commented Mar 7, 2024

Thanks for the quick and detailed answer. Not everything is so clear here. For your recommendation top border did not showed if we remove titlebar, because rendering happens below. titlebar = false doesn't allow me to use the if (title_bar... blah-blah) where rendering border_top.

So, I'm stuck, need copy render func in this construction

if (wants to disable title bar) {
    title_bar = false;
    border_top = blah-blah
    ...
    wlr_scene_node_set_enabled(&con->border.top->node, true);
}

or insert inside "if" more checking?

} else if (con->current.border != B_NORMAL && config->disable-titlebar) {
   //copy here code for initial size and render border_top
}

Again. Borders shows in NORMAL and PIXEL modes. If i disable titlebar in this modes showed empty space without color where we want see border_top

@neuromagus
Copy link
Author

@Nefsen402

However, it might be beneficial to keep the current behavior. The top border size can be manually set to 0 by the user if they truly don't want any top decorations of any kind.

After researching and testing TABBED mode with borders, I found this configuration to be unfortunate for this mode, the borders made it difficult to hit the scrollbar by mouse...

So maybe, but not necessary to be fixed manpages to recommend options with disable_titlebar (default_border none, default_floating_border none/pixel 2 etc) and that's it.

@tmpm697
Copy link

tmpm697 commented Mar 28, 2024

I'm looking forward for at least to disable title on tabbed window as that the currently use case for me.

@neuromagus
Copy link
Author

use working solution today, @tmpm697 xD - https://github.com/neuromagus/disable_titlebar_in_sway

@tmpm697
Copy link

tmpm697 commented Mar 28, 2024

tks @neuromagus , hope it merged soon, critical to one of my favourite workflow.

@quincyf467
Copy link

Thanks for patch! Works as intended

@tormath1
Copy link

Hey folks, I tested this patch against sway-1.9, it compiles and works fine. Great job! I still see a titlebar when the window opacity is set to < 1. Is that expected or? Here's the rendering + configuration: https://gist.github.com/tormath1/3188521f928f4584d2363ce5eb416858

@neuromagus
Copy link
Author

neuromagus commented Apr 18, 2024

@tormath1 yes, master and 1.9 - are different beasts. Use this version for normal work.

@tormath1
Copy link

@neuromagus thanks for sharing the 1.9 patch, it works as expected. Looking forward to see this merged and sorry for the noise 🙇

@emersion
Copy link
Member

emersion commented May 8, 2024

In terms of command design:

  • The term "titlebar" refers to the whole window decoration, including borders. Maybe using "title" instead would be more accurate? (This would be consistent with e.g. "title_format")
  • Do we want a new command, or do we want to special-case the empty string for "title_format"? It may be desirable for users to have a titlebar with no contents, however that could be achieved with a space character, for instance…
  • Do we want to extend "default_border" instead?

@neuromagus
Copy link
Author

neuromagus commented May 8, 2024

The term "titlebar" refers to the whole window decoration, including borders. Maybe using "title" instead would be more accurate? (This would be consistent with e.g. "title_format")

why not?

Do we want a new command, or do we want to special-case the empty string for "title_format"? It may be desirable for users to have a titlebar with no contents, however that could be achieved with a space character, for instance…

I think it's better to remove titlebars completely, no "empty strings", just remove (before we used a hack with font 0 and... better disable titlebar completely (don't allocate window space, don't call a function for format Titlebar et cetera), IMO) :} What do you think?

Do we want to extend "default_border" instead?

This is separate entity, IMHO. Someone want borders without titlebars...
Should I add all this to my patch or you could make the adjustments yourself in Sway?
I only want to use Sway with no "Titlebars" (on any layouts).

@neuromagus
Copy link
Author

neuromagus commented May 8, 2024

@emersion
something wrong with Wlroots in build farm (testing Archlinux)!

Cloning into 'wlroots'...
fatal: unable to access 'https://gitlab.freedesktop.org/wlroots/wlroots.git/': The requested URL returned error: 504
[#1215763] 2024/05/08 16:03:16 Failed to clone git repository. If this a private repository, make sure you've added a suitable SSH key.

@fossdd
Copy link

fossdd commented May 8, 2024

something wrong with Wlroots for testing (archlinux)!

thats a issue on your side, working perfectly with my internet connection.

@neuromagus
Copy link
Author

thats a issue on your side, working perfectly with my internet connection.

ok, how to restart build test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants