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

(3) NullCursor is not a realistic way to hide the cursor. We should either display a simple custom cursor or not modify render_surfaces at all in this proposal. NullCursor is not a useful example or manual test at all. Because showing nothing means you can't really tell it's working properly. And in reality when you do want "no cursor" you will probably toggle a flag, but not set a NullCursor.

(7) "CursorImages" vs "theme": In my mind cursor images come from a theme, but you're selecting a theme from CursorImages. That sounds backwards and will be hard to understand/extend/use:

304 +class CursorImages
305 +{
306 +public:
307 + /// Looks up the image for a named cursor in a given cursor theme. Cursor names
308 + /// follow the XCursor naming conventions.
309 + virtual std::shared_ptr<CursorImage> lookup_cursor(std::string const& theme_name,
310 + std::string const& cursor_name,
311 + geometry::Size const& size) = 0;
312 +

The most realistic (IMHO) interpretation of "cursor images" is a set of cursor images provided by a theme. In that way the cursor images concept is a subset of the theme, not the superset. Maybe there does exist a class name where this design makes sense, but not "CursorImages".