Mir

Merge lp://staging/~alan-griffiths/mir/fix-1496849 into lp://staging/mir

  1. fix-1496849
  2. Merge into development-branch
Proposed by Alan Griffiths
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 2964
Proposed branch: lp://staging/~alan-griffiths/mir/fix-1496849
Merge into: lp://staging/mir
Diff against target: 119 lines (+38/-20)
3 files modified
src/server/graphics/nested/cursor.cpp (+4/-1)
src/server/graphics/nested/cursor.h (+1/-1)
src/server/graphics/nested/mir_client_host_connection.cpp (+33/-18)
To merge this branch: bzr merge lp://staging/~alan-griffiths/mir/fix-1496849
Reviewer Review Type Date Requested Status
Daniel van Vugt Needs Information
Alexandros Frantzis (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+271949@code.staging.launchpad.net

Commit message

graphics: when supplying the host Mir with a cursor don't discard the buffer stream that supports it

Description of the change

graphics: when supplying the host Mir with a cursor don't discard the buffer stream that supports it

NB This fixes the linked bug, but isn't the last problem in this area. In particular, testing with "animated_cursor" shows that when moving outside the client window the default cursor isn't restored.

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

@testing with "animated_cursor" - the problem is that the displayed cursor is a frame behind the nested server. Will tackle in a follow-up MP.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2953
http://jenkins.qa.ubuntu.com/job/mir-ci/4988/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4086
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/2993
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4028
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1138
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1138/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4029
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4029/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6727
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23570

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

review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

Nit:

+mgn::Cursor::~Cursor()
+{
+}

mgn::Cursor::~Cursor() = default

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

I haven't looked into how the cursor buffer stream stuff works yet, but based on your description alone I'm concerned we might have a bad client API design. The client should be able to set-and-forget a cursor. Don't we support that?

Revision history for this message
Daniel van Vugt (vanvugt) :
review: Needs Information
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> I haven't looked into how the cursor buffer stream stuff works yet, but based
> on your description alone I'm concerned we might have a bad client API design.
> The client should be able to set-and-forget a cursor. Don't we support that?

The original code didn't "forget" the buffer stream - it released it.

I'm not trying to defend the design (or implementation) of cursors. (As anyone hearing my recent complaints would realize.) I'm working to get the cursor code under some useful tests[1] as preparation for some rework.

[1] Note that the existing interaction between tests and implementation are "interesting". For example cursor_request_applied_from_buffer_stream configures the cursor before posting any buffers in this scenario BasicSurface::set_cursor_stream() creates an image from the stream before the stream has received a buffer.

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