[Mesa-dev] [PATCH 01/18] anv: Add a short style guide

[Ian Romanick]

On 04/18/2016 05:10 PM, Jason Ekstrand wrote:
> ---
>  src/intel/vulkan/STYLE | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 src/intel/vulkan/STYLE
> 
> diff --git a/src/intel/vulkan/STYLE b/src/intel/vulkan/STYLE
> new file mode 100644
> index 0000000..4eb8f79
> --- /dev/null
> +++ b/src/intel/vulkan/STYLE
> @@ -0,0 +1,67 @@
> +The Intel Vulkan driver typically follows the mesa coding style with a few
> +exceptions.  First is that structs declared in anv_private.h should be
> +written as follows:
> +
> +struct anv_foo {
> +   int                  short_type;
> +   struct anv_long_type long_type;
> +   void *               ptr;
> +};
> +
> +Where the * for pointers goes one space after the type and the names are
> +vertically aligned.  The names should be tabbed over the minimum amount
> +(still a multiple of 3 spaces) such that they can all be aligned.
> +
> +When the anv_batch_emit function is used, it should look as follows:
> +
> +anv_batch_emit(&cmd_buffer->batch, GENX(3DPRIMITIVE),
> +   .VertexAccessType       = SEQUENTIAL,
> +   .PrimitiveTopologyType  = pipeline->topology,
> +   .VertexCountPerInstance = vertexCount,
> +   .StartVertexLocation    = firstVertex,
> +   .InstanceCount          = instanceCount,
> +   .StartInstanceLocation  = firstInstance,
> +   .BaseVertexLocation     = 0);
> +
> +The batch and struct name parameters should go on the same line with the
> +anv_batch_emit call and each named parameter on its own line.  The
> +alignment rules are the same as for structs where all of the "=" are
> +vertically aligned at the minimum tabstop required to do so.
> +
> +Eventually, we would like to move to a block-based packing mechanism.  In
> +this case, the above packing macro would look like this:
> +
> +anv_batch_emit(&cmd_buffer->batch, GENX(3DPRIMITIVE), prim) {
> +   prim.VertexAccessType         = SEQUENTIAL;
> +   prim.PrimitiveTopologyType    = pipeline->topology;
> +   prim.VertexCountPerInstance   = vertexCount;
> +   prim.StartVertexLocation      = firstVertex;
> +   prim.InstanceCount            = instanceCount;
> +   prim.StartInstanceLocation    = firstInstance;
> +   prim.BaseVertexLocation       = 0;
> +}

When I first read the style guide, I thought, "Man, that looks nice."
Then I read all the patches, and I thought, "Man, that looks irritating
to type."  I don't know a way to make any text editor (or indent) do
this for me automatically, so it seems really irritating to write.

Are we sure we want to commit to doing this?  It would be easy enough to
change now using sed on the patches, but changing it later will be much
more painful.

Also... it seems like patch 18 should update this. :)

> +
> +With this new block mechansim, you may end up mixing code and declarations.
> +In this case, it's up to the discression of the programmer exactly how to
> +tab things but trying to keep with the above rules is recommended.
> +
> +In meta code, we use a fair number of compound initializers for more easily
> +calling Vulkan functions that require struct arguments.  These should be
> +declared as follows:
> +
> +anv_CreateFramebuffer(anv_device_to_handle(device),
> +   &(VkFramebufferCreateInfo) {
> +      .sType = VK_STRUCTURE_TYPE_FRAMEBUFFER_CREATE_INFO,
> +      .attachmentCount = 1,
> +      .pAttachments = (VkImageView[]) {
> +         anv_image_view_to_handle(dest_iview),
> +      },
> +      .width = dest_iview->extent.width,
> +      .height = dest_iview->extent.height,
> +      .layers = 1
> +   }, &cmd_buffer->pool->alloc, &fb);
> +
> +The initial arguments go on the same line as the call and the primary
> +struct being passed in is declared on its own line tabbed over 1 tab with
> +it's contents on the following lines tabbed over an additional tab.
> +Substructures get tabbed over by additional tabs as needed.
>