Skip to content
Snippets Groups Projects

vkd3d: Fix handling of non-shader-visible descriptor heaps.

Merged Conor McCarthy requested to merge cmccarthy/vkd3d:visible_heaps into master
2 unresolved threads

Shader visibility is currently ignored, but we don't want to create Vulkan descriptor sets for CPU heaps.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
    • -            d3d12_desc_copy(&dst[dst_idx], &src[src_idx], device);
      +            d3d12_desc_copy(&dst[dst_idx], &src[src_idx], dst_heap, device);

      Do we really need to pass "device" around if we're already passing "dst_heap"? As you mention in the commit message, we don't end up accessing "device" in a lot of places any more, and it seems like we could use "dst_heap->device" when we do.

    • Author Developer

      This function is the main reason for much lower performance vs Windows in HZD at least. Not loading device from dst_heap when we can have it in a register is an advantage. I've added a comment to clarify.

    • Please register or sign in to reply
  • Conor McCarthy added 4 commits

    added 4 commits

    • 66633cbd - vkd3d: Enable Vulkan-backed heaps for each heap instead of per device.
    • 237d3f7c - tests: Test GetGPUDescriptorHandleForHeapStart() on a non-shader-visible heap.
    • cc28a2f2 - vkd3d: Return a null handle from GetGPUDescriptorHandleForHeapStart() for non-shader-visible heaps.
    • 0184a3b0 - vkd3d: Do not create Vulkan descriptor sets for non-shader-visible heaps.

    Compare with previous version

    • This function is the main reason for much lower performance vs Windows in HZD at least. Not loading device from dst_heap when we can have it in a register is an advantage. I've added a comment to clarify.

      In that case, should we be calling d3d12_desc_copy() in a loop from device.c at all? Or would it be better to e.g. introduce a "d3d12_descriptor_heap_copy()" function that takes care of that loop and inlines d3d12_desc_copy() (and quite possibly d3d12_desc_write_atomic())? We may also want to consider placing "device" in the same cacheline as "use_vk_heaps" inside struct d3d12_descriptor_heap in that case.

    • Author Developer

      Yes I think it will benefit from another patch series to optimise copying. It may also be worth storing texture views in their respective resources to eliminate refcounting of texture views.

    • Please register or sign in to reply
  • Conor McCarthy added 4 commits

    added 4 commits

    • 561ea5d2 - vkd3d: Enable Vulkan-backed heaps for each heap instead of per device.
    • 1089a0e4 - tests: Test GetGPUDescriptorHandleForHeapStart() on a non-shader-visible heap.
    • b5d05750 - vkd3d: Return a null handle from GetGPUDescriptorHandleForHeapStart() for non-shader-visible heaps.
    • 5a0fcfdd - vkd3d: Do not create Vulkan descriptor sets for non-shader-visible heaps.

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • Alexandre Julliard added 45 commits

    added 45 commits

    • 5a0fcfdd...d52eb859 - 41 commits from branch wine:master
    • 0a98d091 - vkd3d: Enable Vulkan-backed heaps for each heap instead of per device.
    • 981034e6 - tests: Test GetGPUDescriptorHandleForHeapStart() on a non-shader-visible heap.
    • 90141831 - vkd3d: Return a null handle from GetGPUDescriptorHandleForHeapStart() for non-shader-visible heaps.
    • 42d46d25 - vkd3d: Do not create Vulkan descriptor sets for non-shader-visible heaps.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

Please register or sign in to reply
Loading