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

Cleaner approach to decorations #1728

Closed
xi opened this issue Apr 19, 2024 · 4 comments · Fixed by #1733
Closed

Cleaner approach to decorations #1728

xi opened this issue Apr 19, 2024 · 4 comments · Fixed by #1733

Comments

@xi
Copy link
Contributor

xi commented Apr 19, 2024

I believe the current approach to decorations is a bit messy and hard to extend. I tried to propose extensions in #1724 and #1725 with mixed success. If we want to avoid breaking changes in the future, I think it is better to think hard about the design first before merging any changes.

I am probably overthinking this. But now that the overthinking has already happened, I might as well share it.

Why is this complicated?

We cannot directly port concepts from openbox because Wayland distinguishes between client side decorations (CSD) and server side decorations (SSD).

The current situation

As of

  • There are 3 modes of decoration: 'none', 'borders', and 'full'.
  • The ToggleDecoration action is implemented, Decorate and Undecorate are not.
  • CSD negotiation is implemented for both xdg-shell and xwayland.
    • The results can be overwritten using <windowRule serverDecoration="">
    • A default can be provided using <core><decoration>
    • CSD windows get initialized with 'none' decoration, SSD windows get initialized with 'full' decoration
  • <theme><keepBorder> is implement, but behaves differently from openbox (and also from its documentation) (see Add keepBorder <theme> option and enable it by default #876)
    • If it is disabled, ToggleDecorations toggles between 'none' and 'full'
    • If it is enabled, Toggledecorations cycles between 'border', 'none', and 'full'
    • It has no effect on the initial decoration of CSD windows (as claimed by the documentation)
  • The implementation doesn't use the 3 modes. Instead it uses the following:
    • view->ssd_enabled and view->ssd_titlebar_hidden to track whether ssd and titlebar are enabled
    • view->ssd and view->ssd->titlebar.tree->node->enabled to track whether ssd and titlebar actually exist. For example, full screen windows do not have decoration, even if it is enabled.
    • has_ssd(view) stores the result of the CSD/SSD negotiation

What do we want?

As a start for discussion:

  • We want to keep backwards compatibility as much as possible. Among other things, this means that we have to stick with the the 3-state ToggleDecorations and keepBorder.
  • For CSD windows, undecorated means 'none'
  • For SSD windows, the meaning of undecorated depends on keepBorder
    • if keepBorder=yes, undecorated means 'borders'
    • if keepBorder=yes, undecorated means 'none'
  • Decorated always means 'full'
    • I am not 100% sure about this. Another approach would be to allow users to disable titlebars globally. In that case, decorated would mean 'border' and undecorated would mean 'none'. On the other hand, this could also be achieved by using something like <windowRule serverDecoration="server"> <action name="Undecorate"/> (rule) or ToggleDecorations (interactive).
    • You could also argue that for CSD windows, decorated should mean 'none' because they have their own decoration.
  • Users should be able to switch to any of the 3 decoration modes manually, but not necessarily in a single step (this is what ToggleDecorations provides).

Settings / Rules / Actions

Actions are more flexible than rules (because they can be used both in <windowRole> and <keybind>/<mousebind>) and rules are more flexible than settings (because different rules can apply to different windows). So if we add anything, I think we should prefer actions over rules and settings.

Use cases

Which of these do we want to support?

disable CSD

<windowRule identifier="">
  <windowRule serverDecoration="server">
</windowRule>

manually toggle decorations

<keybind key="">
  <action name="ToggleDecorations"/>
</keybind>

manually toggle between 'full' and 'none'

Only possible if keepBorder is disabled globally.

manually toggle between 'full' and 'border'

Not currently possible

manually toggle between 'border' and 'none'

Not currently possible

manually switch to 'full'

Not currently possible

manually switch to 'border'

Not currently possible

manually switch to 'none'

Not currently possible

toggle between 'decorated' and 'undecorated' (whatever that means based on CSD/SSD negotation and keepBorder)

Not currently possible

manually switch to 'decorated' (whatever that means based on CSD/SSD negotation and keepBorder)

Not currently possible

manually switch to 'undecorated' (whatever that means based on CSD/SSD negotation and keepBorder)

Not currently possible

disable titlebars

Not currently possible. It could look something like this:

<windowRule identifier="">
  <action name="Undecorate"/>
</windowRule>

hide titlebars on maximize

Not currently possible (except that borders are removed automatically). It could look something like this:

<keybind key="W-Up">
  <action name="Undecorate"/>
  <action name="Maximize" direction="both"/>
</keybind>
<keybind key="W-Down">
  <action name="Decorate"/>
  <action name="Unmaximize"/>
</keybind>
<keybind key="W-Left">
  <action name="Undecorate"/>
  <action name="SnapToEdge" direction="left"/>
</keybind>
<keybind key="W-Right">
  <action name="Undecorate"/>
  <action name="SnapToEdge" direction="right"/>
</keybind>
<mousebind button="W-Left" action="Drag">
  <action name="Undecorate"/>
  <action name="Move"/>
</mousebind>

Proposals

  • Initially, ToggleDecorations toggled between 'none' and 'full'. This does not cover any usecases involving 'border', but it is straight forward and Decorate/Undecorate could easily be added.
  • The current version was implemented in Add keepBorder <theme> option and enable it by default #876. It adds basic support for 'border', but does not cover all use cases. it is also no longer clear what Decorate/Undecorate should do.
  • The initial version of Add keepBorder <theme> option and enable it by default #876 did not have the 3-state ToggleDecorations. Instead, it would toggle between 'full' and 'none' if keepBorder=no and between 'full' and 'border' otherwise. This was changed because it would add non-removable borders to CSD windows.
  • In Distinguish between "should it be decorated" and "should it use SSD" #1724 I proposed to mirror the structure of the code: ToggleDecorations/Decorate/Undecorate would manipulate view->ssd_enabled, and ToggleTitlebar/ShowTitlebar/HideTitlebar would manipulate view->ssd_titlebar_hidden. This would cover most use cases, but breaks backwards compatibility and was considered too low-level for end users.
  • Implement Decorate/Undecorate #1725 is based on a proposal by @ahesford. The main idea was to add keepBorder attributes to some actions to overwrite the global setting. It covers many use cases without breaking backwards compatibility, but has some weird edge cases.
  • Decorate/Undecorate could use rules based on has_ssd(view) and keepBorder to decide what is the right thing to do. That would be an opinionated approach that intentionally does not cover all use cases.

Some additional ideas that only cover specific use cases:

  • There was also the proposal to add <action name="SetServerSideDecorationMode" mode="none|border|all"/>.
    • We could add value ranges, e.g. 'border-or-more' and 'border-or-less'. This would allow for example to remove titlebars from SSD windows without affecting CSD windows.
  • The original use case why I got into all of this was disabling titlebars. That could be done with a configurable default value for view->ssd_titlebar_hidden. However, I would prefer to find a more generic solution.
  • <windowRule> could gain an decoration=server|client attribute so that rules can be restricted to SSD or CSD windows.
@Consolatis
Copy link
Member

Consolatis commented Apr 19, 2024

Nice write-up of the current situation, thanks for that.

I think one use-case missing in the above list is people that want to *always* have full SSD decorations around the windows, even if they additionally use client side ones.

Just throwing another random idea around: We do have the If action, we could add a query argument for the current SSD state, e.g. CSD|borders|full|none. That in combination with a SetDecorations action which directly addresses the different states should allow users to use it for any scenario they want.

Something like:

<!-- Use borders by default for non-csd views -->
<windowRule identifier="*">
	<action name="If">
		<query decorations="csd" />
		<else>
			<action name="SetDecorations" decorations="borders" />
		</else>
	</action>
</windowRule>

or

<!-- Use full decorations by default, even for CSD views -->
<windowRule identifier="*">
	<action name="SetDecorations" decorations="full" />
</windowRule>

@Vladimir-csp
Copy link

Please add an option to preserve window size for SetDecorations and ToggleDecorations.

@Consolatis
Copy link
Member

The window size (the underlying client generated surface) should be kept (unless the window is snapped to some edge / region or maximized), so I assume you are referring to modifying the window size when changing the SSD decoration state?

@Vladimir-csp
Copy link

Yes. Preserve window's external dimensions.

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

Successfully merging a pull request may close this issue.

3 participants