<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 18, 2016 at 5:35 PM, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 04/18/2016 05:10 PM, Jason Ekstrand wrote:<br>
> ---<br>
> src/intel/vulkan/STYLE | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++<br>
> 1 file changed, 67 insertions(+)<br>
> create mode 100644 src/intel/vulkan/STYLE<br>
><br>
> diff --git a/src/intel/vulkan/STYLE b/src/intel/vulkan/STYLE<br>
> new file mode 100644<br>
> index 0000000..4eb8f79<br>
> --- /dev/null<br>
> +++ b/src/intel/vulkan/STYLE<br>
> @@ -0,0 +1,67 @@<br>
> +The Intel Vulkan driver typically follows the mesa coding style with a few<br>
> +exceptions. First is that structs declared in anv_private.h should be<br>
> +written as follows:<br>
> +<br>
> +struct anv_foo {<br>
> + int short_type;<br>
> + struct anv_long_type long_type;<br>
> + void * ptr;<br>
> +};<br>
> +<br>
> +Where the * for pointers goes one space after the type and the names are<br>
> +vertically aligned. The names should be tabbed over the minimum amount<br>
> +(still a multiple of 3 spaces) such that they can all be aligned.<br>
> +<br>
> +When the anv_batch_emit function is used, it should look as follows:<br>
> +<br>
> +anv_batch_emit(&cmd_buffer->batch, GENX(3DPRIMITIVE),<br>
> + .VertexAccessType = SEQUENTIAL,<br>
> + .PrimitiveTopologyType = pipeline->topology,<br>
> + .VertexCountPerInstance = vertexCount,<br>
> + .StartVertexLocation = firstVertex,<br>
> + .InstanceCount = instanceCount,<br>
> + .StartInstanceLocation = firstInstance,<br>
> + .BaseVertexLocation = 0);<br>
> +<br>
> +The batch and struct name parameters should go on the same line with the<br>
> +anv_batch_emit call and each named parameter on its own line. The<br>
> +alignment rules are the same as for structs where all of the "=" are<br>
> +vertically aligned at the minimum tabstop required to do so.<br>
> +<br>
> +Eventually, we would like to move to a block-based packing mechanism. In<br>
> +this case, the above packing macro would look like this:<br>
> +<br>
> +anv_batch_emit(&cmd_buffer->batch, GENX(3DPRIMITIVE), prim) {<br>
> + prim.VertexAccessType = SEQUENTIAL;<br>
> + prim.PrimitiveTopologyType = pipeline->topology;<br>
> + prim.VertexCountPerInstance = vertexCount;<br>
> + prim.StartVertexLocation = firstVertex;<br>
> + prim.InstanceCount = instanceCount;<br>
> + prim.StartInstanceLocation = firstInstance;<br>
> + prim.BaseVertexLocation = 0;<br>
> +}<br>
<br>
</div></div>When I first read the style guide, I thought, "Man, that looks nice."<br>
Then I read all the patches, and I thought, "Man, that looks irritating<br>
to type." I don't know a way to make any text editor (or indent) do<br>
this for me automatically, so it seems really irritating to write.<br></blockquote><div><br></div><div>It's not that bad with block-edit in vim. Sometimes it's nicer, sometimes it's not.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Are we sure we want to commit to doing this? It would be easy enough to<br>
change now using sed on the patches, but changing it later will be much<br>
more painful.<br></blockquote><div><br></div><div>Mind saying what "this" is?<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also... it seems like patch 18 should update this. :)<br>
<div class="HOEnZb"><div class="h5"><br>
> +<br>
> +With this new block mechansim, you may end up mixing code and declarations.<br>
> +In this case, it's up to the discression of the programmer exactly how to<br>
> +tab things but trying to keep with the above rules is recommended.<br>
> +<br>
> +In meta code, we use a fair number of compound initializers for more easily<br>
> +calling Vulkan functions that require struct arguments. These should be<br>
> +declared as follows:<br>
> +<br>
> +anv_CreateFramebuffer(anv_device_to_handle(device),<br>
> + &(VkFramebufferCreateInfo) {<br>
> + .sType = VK_STRUCTURE_TYPE_FRAMEBUFFER_CREATE_INFO,<br>
> + .attachmentCount = 1,<br>
> + .pAttachments = (VkImageView[]) {<br>
> + anv_image_view_to_handle(dest_iview),<br>
> + },<br>
> + .width = dest_iview->extent.width,<br>
> + .height = dest_iview->extent.height,<br>
> + .layers = 1<br>
> + }, &cmd_buffer->pool->alloc, &fb);<br>
> +<br>
> +The initial arguments go on the same line as the call and the primary<br>
> +struct being passed in is declared on its own line tabbed over 1 tab with<br>
> +it's contents on the following lines tabbed over an additional tab.<br>
> +Substructures get tabbed over by additional tabs as needed.<br>
><br>
<br>
</div></div></blockquote></div><br></div></div>