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

[Jason Ekstrand]

On Mon, Apr 18, 2016 at 5:35 PM, Ian Romanick <idr at freedesktop.org> wrote:

> 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.
>

It's not that bad with block-edit in vim.  Sometimes it's nicer, sometimes
it's not.


> 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.
>

Mind saying what "this" is?


> 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.
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160418/9f9f9c10/attachment-0001.html>