Code review comment for lp:~andreas-pokorny/mir/fix-1261647

(1) A simple conflict needs fixing:
Text conflict in src/server/scene/basic_surface.cpp
1 conflicts encountered.

(2) 3rd_party/*: I know we've agreed it's OK to change 3rd_party stuff. Now I'm wondering at what point does it not count as 3rd party any more? If we've modified and made it Mir-specific then it's more like a community contribution. So maybe such libraries need to move out of 3rd party, marking them as more "permanent".

(3) Deprecating xoffset/yoffset: Do we have to remove this stuff or can we just comment it as unused (much less code change)? In theory we could at some point try to diff our android-input with the original tarball and use that to patch the same changes to a newer android-input release. In that case, minimizing changes would be ideal.

(4) Warning: I believe Chris is working on reducing/eliminating our use of android-input (maybe). Please talk to him as you might be modifying code he will remove completely.

(5) Whole-screen transformations:
735 + // TODO Still missing further affine transformations that are applied to the
736 + // whole screen i.e. if something other than a orthographic matrix used.
I think we shouldn't even comment that as TODO. I think we should just accept that input coordinates are only correct for surfaces which get mapped to the regular flat screen (Z=0 in screen space). It would be complicated and have potentially serious performance impact to try and read back the screen transformations from OpenGL. And even after you do, you'd lag a frame behind. So I think that's a "TODO" enhancement we should never do.

(6) I've mentioned this a few times now; Surface transformations (animations) are so short-lived that they're not relevant to input calculations in reality. So we don't need to apply surface [inverse_]transformation() to input handling at all. In fact I've said several times and it's clearly documented in renderable.h that the transformation() method could and probably should go away completely. So where we have:
877 glm::mat4 transformation() const override;
878 + glm::mat4 inverse_transformation() const override;
We may well in future replace all occurrences of inverse_transformation() with an identity matrix (no-op).

(7) Even after fixing conflict (1) there are also compile-time conflicts preventing it from building:
/home/dan/bzr/mir/tmp.647/tests/unit-tests/scene/test_basic_surface.cpp: In member function ‘virtual void BasicSurfaceTest_input_region_is_moved_with_surface_Test::TestBody()’:
/home/dan/bzr/mir/tmp.647/tests/unit-tests/scene/test_basic_surface.cpp:379:15: error: no matching function for call to ‘mir::scene::BasicSurface::BasicSurface(<brace-enclosed initializer list>)’
         report};
               ^