Merge lp://qastaging/~alan-griffiths/miral/confine_pointer into lp://qastaging/miral

Proposed by Alan Griffiths
Status: Merged
Approved by: Daniel d'Andrada
Approved revision: 363
Merged at revision: 358
Proposed branch: lp://qastaging/~alan-griffiths/miral/confine_pointer
Merge into: lp://qastaging/miral
Diff against target: 537 lines (+230/-101)
11 files modified
debian/libmiral1.symbols (+6/-1)
include/miral/detail/mir_forward_compatibility.h (+130/-0)
include/miral/window_info.h (+3/-0)
include/miral/window_specification.h (+4/-98)
miral/CMakeLists.txt (+12/-1)
miral/basic_window_manager.cpp (+6/-0)
miral/mir_features.h.in (+28/-0)
miral/symbols.map (+8/-0)
miral/window_info.cpp (+11/-0)
miral/window_management_trace.cpp (+2/-1)
miral/window_specification.cpp (+20/-0)
To merge this branch: bzr merge lp://qastaging/~alan-griffiths/miral/confine_pointer
Reviewer Review Type Date Requested Status
Brandon Schaefer (community) Approve
Daniel d'Andrada (community) Approve
Alan Griffiths Pending
Review via email: mp+306635@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2016-09-21.

Commit message

Add pointer confinement

To post a comment you must log in.
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

Does not compile against Mir-0.20 - which is in 16.04 LTS, it needs similar compatibility hacks to MirPlacementGravity. (A PITA I know, but we should support the LTS.)

/home/alan/display_server/miral/include/miral/window_specification.h:196:57: error: ‘MirPointerConfinementState’ was not declared in this scope
     auto confine_pointer() const -> mir::optional_value<MirPointerConfinementState> const&;
                                                         ^

review: Needs Fixing
Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

On 22/09/2016 05:26, Alan Griffiths wrote:
> Review: Needs Fixing
>
> Does not compile against Mir-0.20 - which is in 16.04 LTS, it needs similar compatibility hacks to MirPlacementGravity. (A PITA I know, but we should support the LTS.)
>
> /home/alan/display_server/miral/include/miral/window_specification.h:196:57: error: ‘MirPointerConfinementState’ was not declared in this scope
> auto confine_pointer() const -> mir::optional_value<MirPointerConfinementState> const&;
> ^
>
>
MIR_CLIENT_VERSION hasn't changed between mir 0.23 and 0.24 (where that
symbol got released). What do you suggest?

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> MIR_CLIENT_VERSION hasn't changed between mir 0.23 and 0.24 (where that
> symbol got released). What do you suggest?

That's a Mir bug (not sure if I actually reported it).

I suggest detecting MIRCLIENT_VERSION (set by pkg_check_modules()) in CMakeLists.txt with:

    if (MIRTEST_VERSION VERSION_LESS 0.24)
    add_definitions(-DMIRAL_NO_POINTER_CONFINEMENT_IN_MIR)
    endif()

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> > MIR_CLIENT_VERSION hasn't changed between mir 0.23 and 0.24 (where that
> > symbol got released). What do you suggest?
>
> That's a Mir bug (not sure if I actually reported it).
>
> I suggest detecting MIRCLIENT_VERSION (set by pkg_check_modules()) in
> CMakeLists.txt with:
>
> if (MIRTEST_VERSION VERSION_LESS 0.24)
MIRCLIENT_VERSION
> add_definitions(-DMIRAL_NO_POINTER_CONFINEMENT_IN_MIR)
> endif()

Revision history for this message
Daniel d'Andrada (dandrader) wrote : Posted in a previous version of this proposal

On 22/09/2016 05:26, Alan Griffiths wrote:
> Review: Needs Fixing
>
> Does not compile against Mir-0.20 - which is in 16.04 LTS, it needs similar compatibility hacks to MirPlacementGravity. (A PITA I know, but we should support the LTS.)
>
> /home/alan/display_server/miral/include/miral/window_specification.h:196:57: error: ‘MirPointerConfinementState’ was not declared in this scope
> auto confine_pointer() const -> mir::optional_value<MirPointerConfinementState> const&;
> ^
>
>
Fixed.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

I'd like it fixed in a similar way to the window_placement_gravity() - i.e. on earlier versions of Mir the miral downstream sees the API (and ABI) but the feature is inactive.

That way the downstream never needs to care about the Mir version - they just write against the MirAL API.

review: Needs Fixing
362. By Alan Griffiths

Fix build on yakkety

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

Hmm, debuild doesn't seem to like my libmiral1.symbols file. I wish I understood it.

363. By Alan Griffiths

Add pointer confinement to WM trace

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

Looks good to me.
Thanks.

review: Approve
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

LGTM

review: Approve

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