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

Demonstrate using the window decoration strategy #3459

Merged
merged 2 commits into from
Jul 8, 2024

Conversation

AlanGriffiths
Copy link
Collaborator

No description provided.

@AlanGriffiths AlanGriffiths requested a review from a team as a code owner July 3, 2024 10:36
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.36%. Comparing base (3bed361) to head (965736d).
Report is 326 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3459   +/-   ##
=======================================
  Coverage   77.36%   77.36%           
=======================================
  Files        1076     1076           
  Lines       68911    68911           
=======================================
+ Hits        53311    53313    +2     
+ Misses      15600    15598    -2     

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

@tarek-y-ismail tarek-y-ismail force-pushed the demo-window-decorations-strategy branch from f3109d4 to 8b31e69 Compare July 3, 2024 11:45
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question!

examples/miral-shell/shell_main.cpp Show resolved Hide resolved
Copy link
Contributor

@mattkae mattkae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working well for me! I tried all of the different configurations and it seemed to work.

Comment on lines +44 to +53
miral::Decorations const decorations{[]
{
if (auto const strategy = getenv("MIRAL_SHELL_DECORATIONS"))
{
if (strcmp(strategy, "always-ssd") == 0) return miral::Decorations::always_ssd();
if (strcmp(strategy, "prefer-ssd") == 0) return miral::Decorations::prefer_ssd();
if (strcmp(strategy, "always-csd") == 0) return miral::Decorations::always_csd();
}
return miral::Decorations::prefer_csd();
}()};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite neat!
I would've done it like so:

ConfigureDecorations() 
    : decorations{init_decorations()}
{
}

// somewhere private
static auto init_decorations() -> miral::Decorations 
{
  // lambda code
}

I'll definitely add this to my arsenal though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, immediately-executed lambdas are a way of turning C++ into an expression language. It can be particularly useful for const initialisation.

Comment on lines +322 to +323
mw::MirShellV1::interface_name,
mw::XdgDecorationManagerV1::interface_name};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I forgot this one in my original PR, thanks for fixing it :)

Base automatically changed from wayland-server-side-decorations-strategy to main July 3, 2024 15:50
@AlanGriffiths AlanGriffiths enabled auto-merge July 3, 2024 16:13
@RAOF
Copy link
Contributor

RAOF commented Jul 8, 2024

Needs symbols fixes

@AlanGriffiths
Copy link
Collaborator Author

Needs symbols fixes

I think main has that. Will rebase when back at keyboard

@AlanGriffiths AlanGriffiths force-pushed the demo-window-decorations-strategy branch from 8b31e69 to 965736d Compare July 8, 2024 17:08
@AlanGriffiths AlanGriffiths added this pull request to the merge queue Jul 8, 2024
Merged via the queue into main with commit 96738e0 Jul 8, 2024
24 of 25 checks passed
@AlanGriffiths AlanGriffiths deleted the demo-window-decorations-strategy branch July 8, 2024 22:58
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 this pull request may close these issues.

4 participants