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

Rework the miral-kiosk splash screen to use Wayland #666

Merged
merged 7 commits into from Dec 13, 2018

Conversation

Projects
None yet
3 participants
@AlanGriffiths
Copy link
Member

AlanGriffiths commented Dec 11, 2018

This is the first of the "internal clients" to be migrated from the mirclient API to Wayland.

(Needed to function on Nvidia as mirclient API doesn't work on Nvidia.)

@wmww
Copy link
Contributor

wmww left a comment

Tested and it seems to work. I'll make some changes.

int32_t height;
};

using Outputs = std::vector<OutputInfo>;

This comment has been minimized.

@wmww

wmww Dec 11, 2018

Contributor

Any reason not to use a std::map<struct wl_output *, OutputInfo> here? Might make the lookups further down a bit cleaner.

int width = 0;
int height = 0;
int width = 400;
int height = 400;

This comment has been minimized.

@wmww

wmww Dec 11, 2018

Contributor

Magic numbers in seemingly generic code. Whats so special about 400? don't like.

Show resolved Hide resolved examples/example-server-lib/sw_splash.cpp
@@ -17,4 +17,4 @@ target_include_directories(example-shell-lib
PRIVATE
${FREETYPE_INCLUDE_DIRS}
)
target_link_libraries(example-shell-lib PUBLIC miral mirclient PRIVATE ${FREETYPE_LIBRARIES})
target_link_libraries(example-shell-lib PUBLIC miral mirclient PRIVATE ${WAYLAND_CLIENT_LIBRARIES} ${FREETYPE_LIBRARIES})

This comment has been minimized.

@wmww

wmww Dec 11, 2018

Contributor

My tests show that mirclient can be safely dropped.

This comment has been minimized.

@wmww

wmww Dec 11, 2018

Contributor

Done

return {owned, deleter};
}

struct Globals

This comment has been minimized.

@wmww

wmww Dec 11, 2018

Contributor

Non blocking, we can always do it later: This struct seems generic enough to be potentially useful to other clients in the project. There might be a better place to put it.

@wmww

wmww approved these changes Dec 11, 2018

bors bot added a commit that referenced this pull request Dec 13, 2018

Merge #667
667: Strip out legacy titlebar logic r=RAOF a=AlanGriffiths

This simplifies the DecorationProvider code. (The mirclient API based "titlebar" logic ignores Wayland clients anyway).

We need to follow up with something similar to #666: This is the final "internal clients" needing to be migrated from the mirclient API to Wayland.

(Needed to function on Nvidia as mirclient API doesn't work on Nvidia.)

Co-authored-by: Alan Griffiths <alan@octopull.co.uk>
@RAOF

RAOF approved these changes Dec 13, 2018

Copy link
Member

RAOF left a comment

A couple of nits, but nothing that can't be fixed in post!

bors r+

{
Outputs* outputs = static_cast<decltype(outputs)>(data);

auto const& output = outputs->find(wl_output);

This comment has been minimized.

@RAOF

RAOF Dec 13, 2018

Member

I'm not a huge fan of taking references to temporaries.

Yes, I know it works, but it continues to be weird, particularly here where it serves no purpose.

x = 3*x/4;
}

std::this_thread::sleep_for(std::chrono::milliseconds(200));

This comment has been minimized.

@RAOF

RAOF Dec 13, 2018

Member

Hm. Why are we sleeping here?
I guess it's easier than a poll(), but wl_display_dispatch() will block until there are events available, so it's going to be a bit hit and miss.

Transitioning to poll() driven usage can be a followup.

bors bot added a commit that referenced this pull request Dec 13, 2018

Merge #666
666: Rework the miral-kiosk splash screen to use Wayland r=RAOF a=AlanGriffiths

This is the first of the "internal clients" to be migrated from the mirclient API to Wayland.

(Needed to function on Nvidia as mirclient API doesn't work on Nvidia.)

Co-authored-by: Alan Griffiths <alan@octopull.co.uk>
Co-authored-by: William Wold <wm@wmww.sh>
@bors

This comment has been minimized.

Copy link
Contributor

bors bot commented Dec 13, 2018

@bors bors bot merged commit 6f3094a into master Dec 13, 2018

2 checks passed

bors Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bors bors bot deleted the internal-clients-use-wayland branch Dec 13, 2018

AlanGriffiths added a commit that referenced this pull request Dec 14, 2018

Merge #667
667: Strip out legacy titlebar logic r=RAOF a=AlanGriffiths

This simplifies the DecorationProvider code. (The mirclient API based "titlebar" logic ignores Wayland clients anyway).

We need to follow up with something similar to #666: This is the final "internal clients" needing to be migrated from the mirclient API to Wayland.

(Needed to function on Nvidia as mirclient API doesn't work on Nvidia.)

Co-authored-by: Alan Griffiths <alan@octopull.co.uk>

AlanGriffiths added a commit that referenced this pull request Dec 14, 2018

Merge #666
666: Rework the miral-kiosk splash screen to use Wayland r=RAOF a=AlanGriffiths

This is the first of the "internal clients" to be migrated from the mirclient API to Wayland.

(Needed to function on Nvidia as mirclient API doesn't work on Nvidia.)

Co-authored-by: Alan Griffiths <alan@octopull.co.uk>
Co-authored-by: William Wold <wm@wmww.sh>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment