Merge into development-branch : add-client-cursor-conf-from-argb : Code : Mir

Status:	Work in progress
Proposed branch:	lp://qastaging/~mir-team/mir/add-client-cursor-conf-from-argb
Merge into:	lp://qastaging/mir
Diff against target:	433 lines (+274/-29) 8 files modified client-ABI-sha1sums (+1/-1) include/client/mir_toolkit/mir_cursor_configuration.h (+16/-0) src/client/cursor_configuration.h (+23/-3) src/client/mir_cursor_api.cpp (+70/-22) src/client/mir_surface.cpp (+20/-2) src/protobuf/mir_protobuf.proto (+9/-1) src/server/frontend/session_mediator.cpp (+56/-0) tests/acceptance-tests/test_client_cursor_api.cpp (+79/-0)
To merge this branch:	bzr merge lp://qastaging/~mir-team/mir/add-client-cursor-conf-from-argb
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration	2014-11-25	Approve on 2014-11-25
Robert Carr (community)			Abstain on 2014-11-25
Review via email: mp+242832@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2014-11-24.

Commit message

Add a method for clients to upload ARGB cursor images from pixel data.

Description of the change

Add a method for clients to upload ARGB cursor images from pixel data. Previously noted as required, just not added. Doing now to enable nested cursor API passthrough (user session would be responsible for loading cursor themes and pass images up to USC...), and complete Qt cursor support.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-25: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:2100
http://jenkins.qa.ubuntu.com/job/mir-ci/2240/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/321
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/321/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/295/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/229/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/295/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2240/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-25: Posted in a previous version of this proposal

#

FAILED: Continuous integration, rev:2101
http://jenkins.qa.ubuntu.com/job/mir-ci/2242/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/324
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/324/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/298/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/231/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/298/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2242/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-25: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2244/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-11-25: Posted in a previous version of this proposal

#

Hm. Could this instead be

MirSurface* create_cursor_surface();

MirCursorConfiguration* mir_cursor_configuration_from_surface(MirSurface, hotspot_x, hotspot_y)?

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-11-25:

#

Hey Chris, I started taking a stab at it this morning...the more I thought about it the less im convinced it's what we want though.

From the client perspective, the surface API is a little more difficult:
1. Create cursor-image-surface
2. Map cursor-image-surface and copy contents
3. Swap buffers
4. Attach cursor-imagesurface to cursor configuration and cursor configuration to surface

vs.

1. Create cursor conf with contents
2. Attach cursor conf to surface.

What does the client gain from the more difficult API though? my perspective is not much...besides perhaps pixel format negotiation.

Once I noticed this I started thinking about if it was really a surface? I think it bears some resemblance to a surface as it is a format of buffer upload to the server...but I think it omits the key aspects of a surface:

1. Presence in scene
2. Frame synchronization/buffer exchange ala frame-dropping is always on for cursor-image-surfaces.
3. Attribute/state-fullness, e.g. cursor upload is fire and forget

Maybe I am missing a benefit to having it represented as a surface. Maybe the alarming bit is that it isn't using MirPixelFormat, etc? Perhaps with a few tweaks it could be mir_cursor_configuration_from_graphics_region

Let me know what you think. Thanks :)

review: Approve

Reply

Revision history for this message

Robert Carr (robertcarr) on 2014-11-25:

#

review: Abstain

Reply

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-25:

#

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2260/rebuild

review: Approve (continuous-integration)

Reply

Unmerged revisions

2103. By Robert Carr on 2014-11-25: Fix pointer arithmetic
2102. By Robert Carr on 2014-11-25: Fix uninitialized memory
2101. By Robert Carr on 2014-11-24: Do not allow setting hotspot past bounds of cursor
2100. By Robert Carr on 2014-11-24: Only support default sized cursors
2099. By Robert Carr on 2014-11-24: Implement hotspot in raw image API
2098. By Robert Carr on 2014-11-24: First impl
2097. By Robert Carr on 2014-11-24: Test skeleton for image cursors
2096. By Robert Carr on 2014-11-24: Test skeleton for image cursors

Mir

Merge lp://qastaging/~mir-team/mir/add-client-cursor-conf-from-argb into lp://qastaging/mir

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers