gapid icon indicating copy to clipboard operation
gapid copied to clipboard

Vulkan Command Tree Grouping Returns Wrong Command paths.

Open pmuetschard opened this issue 8 years ago • 10 comments

Given a trace with the following tree:

- Frame 1 (0-363)
- Frame 2 (364-372)
  - Draw 1 (364-371)
    - 364: vkWaitForFences
    - ...
    - 371: vkQueueSubmit
      - [371 0]
        - [371 0 0]
          - RenderPass
            - 371.0.0.0: vkCmdBeginRenderPass
            - ...
            - 371.0.0.6: vkCmdEndRenderPass
  - vkQueuePresnetKHR

When fetching the nodes from the server the following values are returned:

The node at [1, 0, 7] (vkQueueSubmit) is for command: [371] - Correct. The node at [1, 0, 7, 0] is for command: [371, 0, 0] - Should be [371, 0]. The node at [1, 0, 7, 0, 0] is for command [371, 0, 0, 0] - Should be [371, 0, 0]. The node at [1, 0, 7, 0, 0, 0] (RenderPass) is for commands [0]-[6]- Should be [371, 0, 0]. The nodes at [1, 0, 7, 0, 0, 0, x] are for commands [371, 0, 0, x], which is correct again.

This results in strange behavior, especially when selecting the RenderPass node.

pmuetschard avatar Oct 17 '17 23:10 pmuetschard

It seems like this is closed automatically. Hopefully this is fixed now @pmuetschard

qining avatar Oct 18 '17 18:10 qining

It fixed most cases, however, the RenderPass group still is broken: The node at [1, 0, 7, 0, 0, 0] (RenderPass) is for commands [0]-[6]- Should be [371, 0, 0]

pmuetschard avatar Oct 20 '17 18:10 pmuetschard

@pmuetschard, this is because renderpass group is actually an api.CmdIDGroup, not a api.Command or api.SubCmdRoot or api.SubCmdIdx.

Do you think [371, 0, 0, 0]-[371, 0, 0, 6] is a valid range for this group? I'm thinking of making the Commands field of the returned service.CommandTreeNode something like cmdTree.path.Capture.SubCommandRange(....). Currently, it is cmdTree.path.Capture.CommandRange(...).

qining avatar Oct 20 '17 19:10 qining

@pmuetschard, and in terms of framebuffer request, the subcommand for [1, 0, 7, 0, 0, 0] (RenderPass) should be [371, 0, 0, 6]

qining avatar Oct 20 '17 19:10 qining

I am making these observations from the perspective of the API the server exposes, and not from the implementation:

  • The currently returned value of [0]-[6] is certainly incorrect, as it incorrectly says that the group consists of the first 7 commands of the trace.
  • [371, 0, 0, 0]-[371, 0, 0, 6] would certainly be valid, and just differ in where the new layer starts from what I was pointing out above:
- 364: Frame 2                          layer 0
  - 364: Draw 1                         layer 0
    - 364: vkWaitForFences              layer 0
    - ...
    - 371: vkQueueSubmit                layer 0
      - 371.0: [371 0]                  layer 1
        - 371.0.0: [371 0 0]            layer 2
          - 371.0.0.0: RenderPass       layer 3   -   vs layer 2, in my example
            - 371.0.0.0: vkCmd..        layer 3

pmuetschard avatar Oct 20 '17 19:10 pmuetschard

in terms of framebuffer request, the subcommand for [1, 0, 7, 0, 0, 0] (RenderPass) should be [371, 0, 0, 6]

This will, of course, all depend on how we resolve #1134.

pmuetschard avatar Oct 20 '17 19:10 pmuetschard

Hmm, I will put up an CL with minimized changes to make the CommandRange a SubCommandRange. Probably not align with how we will fix #1134, but at least makes sense for now.

qining avatar Oct 20 '17 19:10 qining

Sorry to be spammy, I want to make sure we also consider multiple render passes and render pass parents:

Yours:

...
- 371: vkQueueSubmit
  - 371.0: [371 0]
    - 371.0.0: [371 0 0] (range 371.0.0)
      - 371.0.0.0: RenderPass (range: 371.0.0.0 - 371.0.0.6)
      - 371.0.0.7: RenderPass (range: 371.0.0.7 - 371.0.0.13)
    - 371.0.1: [371 0 1] (range 371.0.1)
      - 371.0.1.0: RenderPass (range: 371.0.1.0 - 371.0.1.6)
      - 371.0.1.7: RenderPass (range: 371.0.1.7 - 371.0.1.13)

vs mine:

...
- 371: vkQueueSubmit
  - 371.0: [371 0]
    - 371.0.0: [371 0 0] (range 371.0.0 - 371.0.1)
      - 371.0.0: RenderPass (range: 371.0.0.0 - 371.0.0.6)
      - 371.0.1: RenderPass (range: 371.0.1.0 - 371.0.1.6)
    - 371.0.2: [371 0 2] (range 371.0.2 - 371.0.3)
      - 371.0.2: RenderPass (range: 371.0.2.0 - 371.0.2.6)
      - 371.0.3: RenderPass (range: 371.0.3.0 - 371.0.3.6)

The choice is really yours and depends on what you think makes the most sense in numbering things from the Vulkan perspective. I like mine slightly better, because the render pass grouping makes sense to me, however, I'm not as familiar with the API as you, so maybe the grouping of the render pass's parent is more important.

pmuetschard avatar Oct 20 '17 19:10 pmuetschard

Here is a situation why I think mine is better:

- 371: vkQueueSubmit
  - 371.0: [371 0]                                                   [indicate the SubmitInfo index]
    - 371.0.0: [371 0 0] (range 371.0.0)                 [indicate the Commandbuffer index in the SubmitInfo]
      - 371.0.0.0 vkCmdCopyImage
      - 371.0.0.1 vkCmdPipelineBarrier
      - (some other subcommands)
      - 371.0.0.10: RenderPass (range: 371.0.0.10 - 371.0.0.16)
       - 371.0.0.10: vkCmdBeginRenderpass
         - 371.0.0.11: Draw object A (range: 371.0.0.11-371.0.0.13)         [This is a debug marker group]
          - 371.0.0.11: vkCmdBeginMarkerGroup
          - 371.0.0.12: vkCmdDraw...
          - 371.0.0.13: vkCmdEndMarkerGroup
       - 371.0.0.14: vkCmdBind... 
       - 371.0.0.16: vkCmdEndRenderpass
      - 371.0.0.7: RenderPass (range: 371.0.0.7 - 371.0.0.13)
    - 371.0.1: [371 0 1] (range 371.0.1)             [second commandbuffer in the same SubmitInfo]
      - 371.0.1.0: RenderPass (range: 371.0.1.0 - 371.0.1.6)
      - 371.0.1.7: RenderPass (range: 371.0.1.7 - 371.0.1.13)

So, generally, each layer corresponds to a concrete thing in Vulkan API, in total there are: submit info index, command buffer index, command index, secondary command buffer index, secondary command index. Adding another layer of index does not quite match with the concepts in Vulkan API.

So, in the example above, the commands: [371.0.0.1] and [371.0.0.11] are at the same level, adding, showing them are in the same submit info, same command buffer. But adding another layer for the renderpass grouping introduces a new index and breaks the same level thing.

Besides, users are allowed to create as many as 'debug marker groups' as they want. So in extreme cases, it is possible to have tens of layers of debug marker groups.

qining avatar Oct 20 '17 20:10 qining

SGTM. Thanks for the clarification. I agree, it makes more sense to prioritize the grouping at the buffers.

pmuetschard avatar Oct 20 '17 20:10 pmuetschard