Add Wayland decorations strategy #3454

tarek-y-ismail · 2024-07-01T13:01:40Z

Fixes the misunderstandings of the doomed #3453.

codecov · 2024-07-01T13:24:07Z

Codecov Report

Attention: Patch coverage is 19.23077% with 63 lines in your changes missing coverage. Please review.

Project coverage is 77.36%. Comparing base (6f465ad) to head (fc2d435).
Report is 359 commits behind head on main.

Files with missing lines	Patch %	Lines
...er/frontend_wayland/xdg_decoration_unstable_v1.cpp	12.82%	34 Missing ⚠️
src/miral/decorations.cpp	0.00%	23 Missing ⚠️
src/server/default_server_configuration.cpp	50.00%	4 Missing ⚠️
src/server/server.cpp	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3454      +/-   ##
==========================================
- Coverage   77.41%   77.36%   -0.06%     
==========================================
  Files        1074     1076       +2     
  Lines       68854    68911      +57     
==========================================
+ Hits        53304    53311       +7     
- Misses      15550    15600      +50

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

AlanGriffiths

A lot of tweaks, but this is the right idea

src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp

src/server/symbols.map

src/include/server/mir/decoration_strategy.h

src/core/symbols.map

src/include/server/mir/decoration_strategy.h

Co-authored-by: Alan Griffiths <[email protected]>

values

`DecorationStrategy`

src/server/default_server_configuration.cpp

src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp

Co-authored-by: Alan Griffiths <[email protected]>

…able copying. Co-authored-by: Alan Griffiths <[email protected]>

Also fixed a sneaky bug caused by checking if one toplevel was registered instead of zero. toplevels should be already unregistered by their decorations by the time they're destroyed, otherwise it's a protocol error.

orphaning

…re_decoration_throws_orphaned`

…de-decorations-strategy

tarek-y-ismail · 2024-07-02T14:32:26Z

I've got some questions:

Any suggestions for better naming?
Declaring Self: Is it to not have to recompile the header and all files that depend on it when changes are made?
Is to location of decoration_strategy.h as of the latest commit correct? (in the core module, not the server module. I couldn't include it from there)

AlanGriffiths

Some tidying up needed, but looking good

include/core/mir/decoration_strategy.h

include/miral/miral/decorations.h

src/core/symbols.map

src/miral/decorations.cpp

src/miral/symbols.map

src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp

src/server/symbols.map

AlanGriffiths · 2024-07-03T09:24:20Z

This branch has conflicts that must be resolved

Probably easiest to rebase on main

Also, changed `DecorationStrategy::Self` to inherit from `DecorationStrategy` and cleaned up the code a bit.

…earer Co-authored-by: Alan Griffiths <[email protected]>

AlanGriffiths

A little more tidy-up

include/miral/miral/decorations.h

src/include/server/mir/decoration_strategy.h

src/server/symbols.map

AlanGriffiths · 2024-07-03T11:32:53Z

There's a bunch of build failures: I think you need to run cmake --build build --target regenerate-miral-debian-symbols

AlanGriffiths

I am suggesting a little more documentation of miral::Decorations, but looks good

include/miral/miral/decorations.h

mattkae

One teeny suggestion about leaking in a switch (technically, although it hardly matters)

src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp

Co-authored-by: Alan Griffiths <[email protected]>

Co-authored-by: Matthew Kosarek <[email protected]>

mattkae

I am happy with this 😄

tarek-y-ismail self-assigned this Jul 1, 2024

tarek-y-ismail changed the base branch from main to wayland-server-side-decorations July 1, 2024 13:01

Add mir server side decoration strategy

7d848b0

tarek-y-ismail force-pushed the wayland-server-side-decorations-strategy branch from e95d038 to 7d848b0 Compare July 1, 2024 13:07

AlanGriffiths requested changes Jul 1, 2024

View reviewed changes

tarek-y-ismail and others added 8 commits July 2, 2024 09:36

Update naming of protocol mode and decorations type conversion functions

51f5fb3

Co-authored-by: Alan Griffiths <[email protected]>

Fix naming for function definitions

6025dfc

Make decoration_strategy const and move more instead of copying.

fe33489

Remove unnecessary default case.

bd287f8

Make to_decorations_type a method and improve logging on invalid

f30374f

values

Drop the base for mir::DecorationStrategy::DecorationsType

6469f70

Specify default constructor, disable copy constructor/operator in

9225da0

`DecorationStrategy`

Revert unnecessary changes done to src/core/symbols.map

c695222

AlanGriffiths requested changes Jul 2, 2024

View reviewed changes

src/server/default_server_configuration.cpp Outdated Show resolved Hide resolved

src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp Outdated Show resolved Hide resolved

Remove incorrect use of std::move in get_toplevel_decoration

3a75b0f

tarek-y-ismail force-pushed the wayland-server-side-decorations-strategy branch from bd17e18 to 3a75b0f Compare July 2, 2024 11:51

tarek-y-ismail and others added 10 commits July 2, 2024 15:01

Fix incorrect curly brace placement in the_decoration_strategy

10a2773

Co-authored-by: Alan Griffiths <[email protected]>

Implement some decoration strategies on the miral side.

d1a8a68

Fix lifetime issues related to toplevels_with_decorations.

f5e92a1

Fix naming convention of {register,unregister}Toplevel.

aa97067

Remove redundant ToplevelsWithDecorations construction code and dis…

2e60ad6

…able copying. Co-authored-by: Alan Griffiths <[email protected]>

Always unregister toplevel on destruction.

2bcf711

Also fixed a sneaky bug caused by checking if one toplevel was registered instead of zero. toplevels should be already unregistered by their decorations by the time they're destroyed, otherwise it's a protocol error.

Improve semantics of unregister_toplevel and explain decoration

148715d

orphaning

Rename destroy_toplevel_before_decoration to `destroy_toplevel_befo…

f9b62e6

…re_decoration_throws_orphaned`

Add mir server side decoration strategy

4b65e2b

Merge branch 'wayland-server-side-decorations' into wayland-server-si…

98a011b

…de-decorations-strategy

tarek-y-ismail requested a review from AlanGriffiths July 2, 2024 15:58

Base automatically changed from wayland-server-side-decorations to main July 2, 2024 16:42

AlanGriffiths requested changes Jul 3, 2024

View reviewed changes

Merge branch 'main' into wayland-server-side-decorations-strategy

b79e5c3

tarek-y-ismail and others added 3 commits July 3, 2024 12:43

Dont expose DecorationStrategy in miral::Decorations's interface.

a1555bc

Also, changed `DecorationStrategy::Self` to inherit from `DecorationStrategy` and cleaned up the code a bit.

Make log message in XdgToplevelDecorationV1::to_decorations_type cl…

9c189ee

…earer Co-authored-by: Alan Griffiths <[email protected]>

Undo changes done to core/symbols.map erroneously

75526c4

tarek-y-ismail marked this pull request as ready for review July 3, 2024 09:55

tarek-y-ismail requested a review from a team as a code owner July 3, 2024 09:55

tarek-y-ismail requested a review from AlanGriffiths July 3, 2024 09:56

AlanGriffiths requested changes Jul 3, 2024

View reviewed changes

include/miral/miral/decorations.h Outdated Show resolved Hide resolved

src/include/server/mir/decoration_strategy.h Outdated Show resolved Hide resolved

src/server/symbols.map Show resolved Hide resolved

tarek-y-ismail added 2 commits July 3, 2024 13:21

Remove unnecessary DecorationStrategy forward declaration.

63c7287

Remove the base of DecorationStrategy::DecorationsType

2dc3ec6

tarek-y-ismail requested a review from AlanGriffiths July 3, 2024 10:22

Add debian symbols

cdfc15c

AlanGriffiths approved these changes Jul 3, 2024

View reviewed changes

include/miral/miral/decorations.h Show resolved Hide resolved

mattkae requested changes Jul 3, 2024

View reviewed changes

src/server/frontend_wayland/xdg_decoration_unstable_v1.cpp Outdated Show resolved Hide resolved

tarek-y-ismail and others added 2 commits July 3, 2024 17:34

Add class documentation comments to mir::Decorations

2aeb440

Co-authored-by: Alan Griffiths <[email protected]>

Properly scope pid inside to_decorations_type

fc2d435

Co-authored-by: Matthew Kosarek <[email protected]>

tarek-y-ismail requested a review from mattkae July 3, 2024 15:08

mattkae approved these changes Jul 3, 2024

View reviewed changes

AlanGriffiths enabled auto-merge July 3, 2024 15:15

AlanGriffiths added this pull request to the merge queue Jul 3, 2024

Merged via the queue into main with commit 468a852 Jul 3, 2024
32 of 36 checks passed

AlanGriffiths deleted the wayland-server-side-decorations-strategy branch July 3, 2024 15:50

AlanGriffiths mentioned this pull request Sep 9, 2024

Release 2.18.0 #3532

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Wayland decorations strategy #3454

Add Wayland decorations strategy #3454

tarek-y-ismail commented Jul 1, 2024 •

edited

Loading

codecov bot commented Jul 1, 2024 •

edited

Loading

AlanGriffiths left a comment

tarek-y-ismail commented Jul 2, 2024

AlanGriffiths left a comment

AlanGriffiths commented Jul 3, 2024

AlanGriffiths left a comment

AlanGriffiths commented Jul 3, 2024

AlanGriffiths left a comment

mattkae left a comment

mattkae left a comment

Add Wayland decorations strategy #3454

Add Wayland decorations strategy #3454

Conversation

tarek-y-ismail commented Jul 1, 2024 • edited Loading

codecov bot commented Jul 1, 2024 • edited Loading

Codecov Report

AlanGriffiths left a comment

Choose a reason for hiding this comment

tarek-y-ismail commented Jul 2, 2024

AlanGriffiths left a comment

Choose a reason for hiding this comment

AlanGriffiths commented Jul 3, 2024

AlanGriffiths left a comment

Choose a reason for hiding this comment

AlanGriffiths commented Jul 3, 2024

AlanGriffiths left a comment

Choose a reason for hiding this comment

mattkae left a comment

Choose a reason for hiding this comment

mattkae left a comment

Choose a reason for hiding this comment

tarek-y-ismail commented Jul 1, 2024 •

edited

Loading

codecov bot commented Jul 1, 2024 •

edited

Loading