Code review comment for lp://staging/~kdub/mir/rfc-vulkan-submission-modes

> 1. As you pointed out, we are in no obligation to support all Vulkan
> submission modes, so we should not list modes that we don't yet support in our
> headers. We may never support some of them. We should also have
> mir_present_mode_num_modes at the end.-

I think having all 4 modes in there at the start is good (I think we should support all the modes, we have had previous attempts to support something like IMMEDIATE (which was called nbuffers=1 at the time Daniel was experimenting with it)). Don't mind removing the unused ones, but would prefer to leave in.

>
> 2. mir_connection_present_mode_supported()
> Actually, the mode will depend on properties of the buffers in a swap chain.
> So this function should get this info from the platform (though I am not sure
> if we can assume that the app can always obtain the buffers with the exact
> properties it asks for, e.g. GBM_BO_USE_SCANOUT, GBM_BO_USE_LINEAR, etc).
>

Right, we'll probably need some further input from the platforms, but I think the signature will work for that as is? (MirConnection has access to platform-specific info)

> 3. I think the destroy function should not exist. We can fix the name
> collision with the create during Mir 1.0 break by adding a parameter to the
> get function. The destroy (which is only destroying the client side) can be
> implicit, i.e. get would be allowed to be called multiple times (with a
> different mode), and it would destroy the old PC and recreate the new one.
>
> Sigh, all that said our client API is still not very intuitive. We should
> think about getting rid of BSs and only have PCs (which should be wrapped in a
> nice RS api). I think things should be cleaner. But that's probably a
> discussion to be had face-to-face during a sprint.

Sure, although I think the new stuff by itself makes sense. BufferStream is legacy, and imo, that's where the confusion comes from. Its also been isolated (only SW MirBufferStreams can be created), and I'd be all for removing that support as well.

The current model, we have a chain or BS created from a RS, and that object has the same lifetime as the RS. We also cannot 'get' a chain/BS from a RS once a chain/BS has been made before. So this makes object creation and resource management a bit fiddly/confusing.

I think create/destroy gets rid of the re-creation and construction gotchas, but that can be addressed separately. (iirc, we decided not to address before 1.0 in the name of time)