On 04/18/2016 05:56 PM, Dave Airlie wrote: > On 19 April 2016 at 10:35, Ian Romanick <[email protected]> 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. >> >> 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. :) > > Really we should just pick kernel style or Mesa style. I'm not sure we > need to invest
Well... I tend to agree, but we're doing things here that we haven't previously done in Mesa, and I don't know that the kernel does (much?) of this either. > in a new style. If your style guide isn't an indent command line and > .emacs and .vim > snippets, then it isn't near as useful. Also as I said on irc, It definitely decreases the probability that people will get it consistently right... and increases the number of vN+1 patches. > separating the * from the name > is bad documentation practice. > > I can spot the bug in both lines below a lot easier in the second. The > * goes with the name > not with the type. > > uint32_t * ptr1, ptr2; > uint32_t *ptr1, ptr2; Right. That's part of the reason Mesa doesn't generally declare multiple variables in a single line like that. I have seen similar styles that do int foo; int *bar; int asdf; The names still line up, the but the * is someplace sensible. > Dave. _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
