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 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, 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; Dave. _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
