Mir

Code review comment for lp:~mir-team/mir/cursor-spike-phase-1-resubmit

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

(3) render_surfaces now segfaults on the first movement of the cursor:
(gdb) bt
#0 0x00007f0a4d0703de in mir::DefaultServerConfiguration::DefaultCursorListener::cursor_moved_to (this=0x96f9f8, abs_x=6, abs_y=0)
    at /home/dan/bzr/mir/tmp.cursor/src/server/default_server_configuration.cpp:101
#1 0x00007f0a4d117292 in mir::input::android::PointerController::notify_listener (this=0x11a3150)
    at /home/dan/bzr/mir/tmp.cursor/src/server/input/android/android_pointer_controller.cpp:56
#2 0x00007f0a4d117597 in mir::input::android::PointerController::setPosition (
    this=0x11a3150, new_x=6, new_y=-5)
    at /home/dan/bzr/mir/tmp.cursor/src/server/input/android/android_pointer_controller.cpp:112

(4) I agree mir::graphics::Buffer half fits what we need for CursorImage. Perhaps we need to keep the commonality in Buffer and move the surface-only stuff into a "SurfaceBuffer" or "ClientBuffer". Then CursorImage becomes a simple implementation of Buffer. Although such a change could come any time later...

(5) CursorRepository is a bad abstraction and the confusing word "Repository" is the reason. A repository of cursors is not a familiar concept even to a graphics programmer. I think the cursor theme should be the main object there. So instead of:
    img = repository->lookup_cursor(theme_name, cursor_name, size);
a better design would be:
    img = theme->get_image(cursor_name, size);

Remember to link the relevant bugs when ready:
https://bugs.launchpad.net/mir/+bugs?field.tag=cursor

review: Needs Fixing

« Back to merge proposal