Code review comment for lp:~mir-team/mir/touchspot-renderable

> needed?:
> 639 + auto mga_buffer = std::dynamic_pointer_cast<mga::Buffer>(buffer);
> 640 + if (!mga_buffer)
> 641 + BOOST_THROW_EXCEPTION(std::logic_error("Invalid buffer (did not
> originate from platform graphics driver)"));
>
> Yeah...kind of strange I guess, just using static cast.
>
> >> could there be stronger typing than void*?
> >> void mga::AndroidBufferWriter::write(std::shared_ptr<mg::Buffer> const&
> buffer, void const* data, >> size_t size)
>
> Using unsigned char now or did you mean something that encodes the pixel
> format?
>
> >> overlay is a bit overloaded already, maybe just add_cursor_renderable()?
> >> 2824 + void add_overlay(std::shared_ptr<graphics::Renderable> const&
> overlay);
> >> 2825 + void remove_overlay(std::weak_ptr<graphics::Renderable> const&
> overlay);
>
> How about "input_visualization"
>
>
> >> const&
> >> 643 + auto handle = buffer->native_buffer_handle();
>
> Fixed
>
> >> we shouldn't be adding this to Renderable without rejecting all 'decorated'
> renderables down in >> the hwc code. This function makes the RenderableList
> indeterminate in terms of what would appear >> on the screen. It says it
> should be decorated but doesn't say what this entails.
> >> 165 + virtual bool should_be_decorated() const = 0;
>
> I'm not sure the renderables should be / need to be rejected. If a shell wants
> to draw decorations then it is responsible for recognizing that passing the
> renderlist to the display buffer isn't appropriate right? So this method is
> really just for the shell compositor tor ecognize if the renderable is of a
> type which should be decorated. I agree it's strange...but I don't know how to
> do anything better without decorations expressed in the scene.

The problem is really that:
mg::DisplayBuffer::post_renderables_if_optimizable() has the contract of either rejecting the list as whole, or painting exactly what is contained in the list. The addition of should_be_decorated() throws a big wrench in that contract because the implementer doesn't know what to paint correctly if should_be_decorated() returns true.

This also drives at the conversation in lp:~kdub/mir/prep-for-1348330. The compositors wants to do something different if this is a 'input visualization'. So, a decent compromise to me seems to be:
bool is_a_input_visualization()
instead of
bool should_be_decorated().
That way, the RenderableList remains definite, and can leave it up to the compositors as to what they want to do with the visualization.