WayfireWM / wayfire

A modular and extensible wayland compositor
https://wayfire.org/
MIT License
2.4k stars 178 forks source link

The 'maximized is true' criteria is not working. #2043

Closed xiaohuirong closed 10 months ago

xiaohuirong commented 11 months ago

Describe the bug I set ignore_views = app_id is "xfce4-terminal" | maximized is true for decoration plugin, but the maximized is true criteria is not working and it will render the app_id is "xfce4-terminal" ineffective.

To Reproduce Steps to reproduce the behavior:

  1. enable ssd and decoration plugin.
  2. set ignore_views = app_id is "xfce4-terminal" | maximized is true for decoration plugin.
  3. The xfce4-terminal's ssd still exists, and it won't be hidden when the window is maximized.

Expected behavior The SSD can be hidden when the window is maximized.

Wayfire version wayfire 0.8.0.r52.g8f7787e1-1 built from latest 8f7787e1 and wayfire 0.7.5

ammen99 commented 11 months ago

The criteria here will be applied only once, when the window is mapped. The intention has never been to allow this to hide the decoration when the window is maximized and restore decorations when it is not ...

xiaohuirong commented 11 months ago

Is it possible to support this feature? It seems that apart from automatically hiding decorations, there doesn't seem to be any other place where the maximized is true criteria can be used. I tried adding this criteria to the blur plugin, but it seems to be ineffective as well.

ammen99 commented 11 months ago

The maximized criteria was originally intended for window-rules, but I agree it is not particularly interesting. The decoration plugin could, of course, handle this, but I am not sure what the best way to expose the functionality is - (ab)using the existing option seems less than ideal.

Maybe we want configurable decoration size for different states, so that you can set the size to 0 for maximized windows?

PS. For blur it won't work either, they all have a one-off effect: only when the view is mapped. The maximized criteria was meant for window-rules primarily.

xiaohuirong commented 11 months ago

Dynamic setting of window height also requires the decoration to change along with the window's state. Therefore, the key to solving this issue is to ensure that these one-off effects can refresh along with the window state changes. This design should be more reasonable; otherwise, the usability of criteria that change with window state is very limited.

ammen99 commented 11 months ago

In all honesty I believe that the best solution is custom IPC scripts, you can then embed any logic you want, provided that:

timgott commented 10 months ago

Also affects winshadows. "floating is true" does not work either. This used to work, so I consider this a bug. Probably the view_matcher is broken, but I did not investigate yet.

See https://github.com/timgott/wayfire-shadows/issues/19

Edit: I checked quickly, indeed the decoration plugin does not connect to the tiled signal so probably it cannot work for that reason. The shadows plugin does though.

Edit2: I was wrong, the view matcher does work (test: disable and enable title bar in firefox to trigger decoration state update). The tiled signal in the shadows plugin does not work anymore probably.

tldr: In the decoration plugin this could be easily fixed by updating state on view_tiled.

timgott commented 10 months ago

The criteria parser does not even accept "maximized is true" for me:

[src/core/matcher.cpp:27] Failed to parse condition maximized is true from option ignore_views
[src/core/matcher.cpp:28] Reason for the failure: Condition parser error. Unexpected symbol.

"tiled-top is true" does not work either:

[src/core/matcher.cpp:27] Failed to parse condition tiled-top is true from option ignore_views
[src/core/matcher.cpp:28] Reason for the failure: Symbol parser error. Identifier contains invalid character. text:tiled-top

"floating is true" does work.

ammen99 commented 10 months ago

@timgott See https://github.com/WayfireWM/wf-utils/pull/8

erikvanhamme commented 10 months ago

I have checked and merged the pull request.

ammen99 commented 10 months ago

I have checked and merged the pull request.

Cool, thanks :)

With #2062 this issue should be resolved.