Mir

Merge lp://qastaging/~alan-griffiths/mir/discussion-migrate-demo-shell into lp://qastaging/mir

Proposed by Alan Griffiths
Status: Rejected
Rejected by: Alan Griffiths
Proposed branch: lp://qastaging/~alan-griffiths/mir/discussion-migrate-demo-shell
Merge into: lp://qastaging/mir
Prerequisite: lp://qastaging/~alan-griffiths/mir/migrate-render_surfaces
Diff against target: 559 lines (+97/-244)
15 files modified
CMakeLists.txt (+0/-1)
examples/CMakeLists.txt (+2/-0)
examples/demo-shell/CMakeLists.txt (+4/-1)
examples/demo-shell/demo_shell.cpp (+53/-90)
include/server/mir/default_server_configuration.h (+2/-1)
include/server/mir/server.h (+26/-3)
playground/CMakeLists.txt (+0/-16)
playground/README (+0/-6)
playground/server_configuration.cpp (+0/-72)
playground/server_configuration.h (+0/-50)
server-ABI-sha1sums (+2/-2)
src/server/server.cpp (+3/-0)
src/server/symbols.map (+3/-0)
tests/unit-tests/examples/test_demo_compositor.cpp (+1/-1)
tests/unit-tests/examples/test_demo_renderer.cpp (+1/-1)
To merge this branch: bzr merge lp://qastaging/~alan-griffiths/mir/discussion-migrate-demo-shell
Reviewer Review Type Date Requested Status
Daniel van Vugt Needs Fixing
Alan Griffiths Needs Information
PS Jenkins bot (community) continuous-integration Needs Fixing
Kevin DuBois (community) Disapprove
Review via email: mp+244152@code.qastaging.launchpad.net

Commit message

examples: update the demo-shell example to use the mir::Server API

Description of the change

examples: update the demo-shell example to use the mir::Server API

This involves exposing stuff that had been "privatized" and this might not be the best way to expose the functionality needed. Hence I'd like some discussion about whether this is justified.

To post a comment you must log in.
Revision history for this message
Kevin DuBois (kdub) wrote :

Our downstreams have been able to get along without our utility classes (
gl_program, gl_texture, gl_primitive, gl_renderer, etc) to this point, and re-exposing them exposes a lot of ABI that has been hidden and is not used. These classes are more utility classes of the way we've used GLES internally than a recommendation/suggestion to others as to how they should use GLES.

I think its possible to transition the demo shell from playground to examples with a bit of more in-depth "surgery" to have it target reimplementing our already-exposed /interfaces/ instead of overriding different components of utility classes.

review: Disapprove
2139. By Alan Griffiths

merge lp:mir

2140. By Alan Griffiths

merge lp:~alan-griffiths/mir/migrate-render_surfaces

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I'm pretty sure playground/ can access a superset of what examples/ can. That's why it exists.

So there's no reason to republish all those headers just yet, is there?

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Quoting the README:

    The Playground

    These are mir demos that excercise private in-flux mir functionality.
    As such functionality matures, related headers become public and the playground
    code may become an example of how to use such feature.

The reason for this MP is to drive a discussion of what is preventing users of our supported API doing what we do in the mir_demo_server_shell example.

As I see it the only real problem highlighted by this MP is that the DemoRenderer class that has been implemented using some of our private gl tools. That is I don't see that access to the_compositor_report() or the_input_scene() is a big deal.

So the question is: how do we serve our current and potential users better?

1. Provide an example of shell decorations etc that doesn't use Mir internals
2. Design a way for users to implement shell decorations that we're happy to publish
3. Ignore the issue

Using our private gl tools isn't the only implementation option and (as Kevin says) our existing downstreams haven't found a need for this approach. It would be good to have an example that is more like what we expect users to do.

review: Needs Information
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Definitely this migration is something that will have to happen in future. I was leaning toward keeping things as private as possible for as long as possible. That means I expected to keep much of this private (so we can freely change interfaces) until such time as some new shell developer asks us to expose parts. Putting things in the public API prematurely and then having to change them is a painful issue we keep encountering. So I'd prefer to keep as much as possible private for as long as possible to minimize the problem. We all share the goal of wanting releases in future that don't bump the server ABI, so letting it stabilize privately still feels like a good idea to me...

On a more practical note:
(1) Needs Fixing: mir_demo_server_shell now crashes immediately on startup.

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

OK, I didn't really think this would land in this form - I was more interested in highlighting the gaps between what we our supported API allows and what we believe reasonable Mir users would want to do.

Unmerged revisions

2140. By Alan Griffiths

merge lp:~alan-griffiths/mir/migrate-render_surfaces

2139. By Alan Griffiths

merge lp:mir

2138. By Alan Griffiths

merge lp:~alan-griffiths/mir/migrate-render_surfaces/

2137. By Alan Griffiths

Move demo-shell to examples

2136. By Alan Griffiths

Publish headers referenced by demo shell

2135. By Alan Griffiths

Tidy up demo code

2134. By Alan Griffiths

Delete dead code

2133. By Alan Griffiths

BFI migration of demo shell away from legacy API

2132. By Alan Griffiths

merge lp:~alan-griffiths/mir/migrate-render_surfaces

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches