diff options
| author | Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> | 2026-11-02 11:38:11 -0500 |
|---|---|---|
| committer | Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> | 2026-11-02 14:53:47 -0500 |
| commit | 13fc5fda26e6b5a9af67f5a654e831187bfa1962 (patch) | |
| tree | c1666bbbbd10107c4cd6a9041e765135821526f9 /src | |
| parent | 256b1dc346e19bc60c2884d4af875fc53ef0a99a (diff) | |
vulkan: repack 69-byte vertices only when needed
SetVertexRepackLayout returned old_mask | new_mask, which stays non-zero
on every draw that uses a 69-byte binding. PrepareDraw then re-dirtied the
vertex buffers and vertex input each time, so the repack compute pass ran
and the render pass ended once per draw, which is costly on tiled GPUs.
Return old_mask ^ new_mask so only a layout change re-dirties the
bindings. The repacked copy only stays valid while the source data is
unchanged, so the buffer cache now flags the vertex buffers overlapping a
CPU write, inline or DMA write, or GPU write, and re-dirties the repacked
ones before binding. Command buffer submission already invalidates all
vertex buffer state, which keeps the staging copy within its tick.
Also pass the source buffer size to the repack pass and clamp the
word-rounded storage descriptor range to it; it could run up to three
bytes past the end of the buffer. A binding that does not fit is skipped
like one over the device limits.
Follow-up to e38b738b84.
Assisted-by: Claude Sonnet 5.5 (implementation and commit message)
Diffstat (limited to 'src')
| -rw-r--r-- | src/tests/video_core/vertex_repack.cpp | 26 | ||||
| -rw-r--r-- | src/video_core/buffer_cache/buffer_cache.h | 37 | ||||
| -rw-r--r-- | src/video_core/buffer_cache/buffer_cache_base.h | 8 | ||||
| -rw-r--r-- | src/video_core/renderer_vulkan/vertex_repack.h | 14 | ||||
| -rw-r--r-- | src/video_core/renderer_vulkan/vk_buffer_cache.cpp | 8 | ||||
| -rw-r--r-- | src/video_core/renderer_vulkan/vk_buffer_cache.h | 1 | ||||
| -rw-r--r-- | src/video_core/renderer_vulkan/vk_compute_pass.cpp | 5 | ||||
| -rw-r--r-- | src/video_core/renderer_vulkan/vk_compute_pass.h | 7 |
8 files changed, 86 insertions, 20 deletions
diff --git a/src/tests/video_core/vertex_repack.cpp b/src/tests/video_core/vertex_repack.cpp index 6409be3ca4..3705a97926 100644 --- a/src/tests/video_core/vertex_repack.cpp +++ b/src/tests/video_core/vertex_repack.cpp @@ -85,7 +85,8 @@ TEST_CASE("Static vertex repacking requires explicit experimental opt-in", "[ver TEST_CASE("Vertex repack planning keeps descriptor offsets aligned", "[vertex_repack]") { for (const u32 alignment : {4U, 16U, 64U, 256U, 4096U}) { for (const u32 offset : {0U, 1U, 3U, 255U, 4099U}) { - const auto plan = Vulkan::MakeVertexRepackParameters(offset, 70, alignment, 1U << 20, 65535); + const auto plan = Vulkan::MakeVertexRepackParameters(offset, 70, alignment, 1U << 20, + 65535, 1U << 20); REQUIRE(plan.has_value()); REQUIRE(plan->source_base % alignment == 0); REQUIRE(plan->source_base + plan->source_bias == offset); @@ -99,13 +100,22 @@ TEST_CASE("Vertex repack planning keeps descriptor offsets aligned", "[vertex_re } TEST_CASE("Vertex repack planning rejects overflow and device-limit violations", "[vertex_repack]") { - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 0, 256, 1U << 20, 65535)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 0, 1U << 20, 65535)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 71, 65535)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(255, 69, 256, 323, 65535)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 1U << 20, 0)); - REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 0xffffffffU, 256, ~u64{0}, 0xffffffffU)); - const auto one = Vulkan::MakeVertexRepackParameters(0, 69, 256, 72, 1); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 0, 256, 1U << 20, 65535, 1U << 20)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 0, 1U << 20, 65535, 1U << 20)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 71, 65535, 1U << 20)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(255, 69, 256, 323, 65535, 1U << 20)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 1U << 20, 0, 1U << 20)); + REQUIRE_FALSE( + Vulkan::MakeVertexRepackParameters(0, 0xffffffffU, 256, ~u64{0}, 0xffffffffU, ~u64{0})); + const auto one = Vulkan::MakeVertexRepackParameters(0, 69, 256, 72, 1, 72); REQUIRE(one.has_value()); REQUIRE(one->source_range == 72); } + +TEST_CASE("Vertex repack planning keeps the source range inside the buffer", "[vertex_repack]") { + const auto clamped = Vulkan::MakeVertexRepackParameters(0, 69, 256, 1U << 20, 65535, 69); + REQUIRE(clamped.has_value()); + REQUIRE(clamped->source_range == 69); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(0, 69, 256, 1U << 20, 65535, 68)); + REQUIRE_FALSE(Vulkan::MakeVertexRepackParameters(300, 69, 256, 1U << 20, 65535, 256)); +} diff --git a/src/video_core/buffer_cache/buffer_cache.h b/src/video_core/buffer_cache/buffer_cache.h index cd176dec27..6878780866 100644 --- a/src/video_core/buffer_cache/buffer_cache.h +++ b/src/video_core/buffer_cache/buffer_cache.h @@ -10,6 +10,7 @@ #include <memory> #include <numeric> #include <stdexcept> +#include <utility> #include <vector> #include <fmt/ranges.h> @@ -180,6 +181,7 @@ void BufferCache<P>::WriteMemory(DAddr device_addr, u64 size) { gpu_modified_ranges.Subtract(device_addr, size); } memory_tracker.MarkRegionAsCpuModified(device_addr, size); + MarkVertexBuffersStale(device_addr, size); } template <class P> @@ -265,6 +267,7 @@ bool BufferCache<P>::DMACopy(GPUVAddr src_address, GPUVAddr dest_address, u64 am } ClearDownload(*cpu_dest_address, amount); + MarkVertexBuffersStale(*cpu_dest_address, amount); BufferId buffer_a; BufferId buffer_b; @@ -893,7 +896,39 @@ void BufferCache<P>::UpdateVertexBufferSlot(u32 index, const Binding& binding) { } template <class P> +void BufferCache<P>::MarkVertexBuffersStale(DAddr device_addr, u64 size) { + if constexpr (requires { runtime.IsVertexRepacked(u32{}); }) { + u32 mask = enabled_vertex_buffers_mask; + while (mask != 0) { + const u32 index = std::countr_zero(mask); + mask &= mask - 1; + const Binding& binding = v_buffer[index]; + if (device_addr < binding.device_addr + binding.size && + binding.device_addr < device_addr + size) { + stale_vertex_buffers_mask |= 1U << index; + } + } + } +} + +template <class P> +void BufferCache<P>::RefreshRepackedVertexBuffers() { + if constexpr (requires { runtime.IsVertexRepacked(u32{}); }) { + u32 mask = std::exchange(stale_vertex_buffers_mask, 0) & enabled_vertex_buffers_mask; + auto& flags = maxwell3d->dirty.flags; + while (mask != 0) { + const u32 index = std::countr_zero(mask); + mask &= mask - 1; + if (runtime.IsVertexRepacked(index)) { + flags[Dirty::VertexBuffer0 + index] = true; + } + } + } +} + +template <class P> void BufferCache<P>::BindHostVertexBuffers() { + RefreshRepackedVertexBuffers(); #ifdef __ANDROID__ const bool use_optimized_vertex_buffers = Settings::values.use_optimized_vertex_buffers.GetValue(); @@ -1655,6 +1690,7 @@ void BufferCache<P>::MarkWrittenBuffer(BufferId buffer_id, DAddr device_addr, u3 } } memory_tracker.MarkRegionAsGpuModified(device_addr, size); + MarkVertexBuffersStale(device_addr, size); gpu_modified_ranges.Add(device_addr, size); if (DownloadsAtFence(device_addr, size)) { uncommitted_gpu_modified_ranges.Add(device_addr, size); @@ -2205,6 +2241,7 @@ void BufferCache<P>::InlineMemoryImplementation(DAddr dest_address, size_t copy_ std::span<const u8> inlined_buffer) { ClearDownload(dest_address, copy_size); gpu_modified_ranges.Subtract(dest_address, copy_size); + MarkVertexBuffersStale(dest_address, copy_size); BufferId buffer_id = FindBuffer(dest_address, static_cast<u32>(copy_size), false); auto& buffer = slot_buffers[buffer_id]; diff --git a/src/video_core/buffer_cache/buffer_cache_base.h b/src/video_core/buffer_cache/buffer_cache_base.h index 1f0c6cf10f..59be418030 100644 --- a/src/video_core/buffer_cache/buffer_cache_base.h +++ b/src/video_core/buffer_cache/buffer_cache_base.h @@ -525,6 +525,13 @@ private: void UpdateVertexBufferSlot(u32 index, const Binding& binding); + /// Flags the vertex buffers overlapping a written range so host copies derived from them are + /// rebuilt. + void MarkVertexBuffersStale(DAddr device_addr, u64 size); + + /// Re-dirties repacked vertex buffers whose source data was written since they were bound. + void RefreshRepackedVertexBuffers(); + void ClearDownload(DAddr base_addr, u64 size); void InlineMemoryImplementation(DAddr dest_address, size_t copy_size, @@ -545,6 +552,7 @@ private: u32 last_index_count = 0; u32 enabled_vertex_buffers_mask = 0; + u32 stale_vertex_buffers_mask = 0; u64 vertex_buffers_serial = 0; std::array<Binding, 32> v_buffer{}; diff --git a/src/video_core/renderer_vulkan/vertex_repack.h b/src/video_core/renderer_vulkan/vertex_repack.h index b0849c25c7..456d1c6f20 100644 --- a/src/video_core/renderer_vulkan/vertex_repack.h +++ b/src/video_core/renderer_vulkan/vertex_repack.h @@ -2,6 +2,7 @@ #pragma once +#include <algorithm> #include <cstdlib> #include <cstring> #include <limits> @@ -31,8 +32,11 @@ struct VertexRepackParameters { u32 dispatch_groups; }; -inline std::optional<VertexRepackParameters> MakeVertexRepackParameters( - u32 offset, u32 size, u64 alignment, u64 max_storage_range, u32 max_dispatch_groups) { +inline std::optional<VertexRepackParameters> MakeVertexRepackParameters(u32 offset, u32 size, + u64 alignment, + u64 max_storage_range, + u32 max_dispatch_groups, + u64 buffer_size) { if (size == 0 || alignment == 0) { return std::nullopt; } @@ -40,9 +44,11 @@ inline std::optional<VertexRepackParameters> MakeVertexRepackParameters( const u64 output_size = vertices * 72; const u64 base = u64{offset} / alignment * alignment; const u64 bias = offset - base; - const u64 range = (bias + size + 3) / 4 * 4; + // The word-rounded range must not run past the end of the source buffer + const u64 available = buffer_size > base ? buffer_size - base : 0; + const u64 range = (std::min)((bias + size + 3) / 4 * 4, available); const u64 groups = (vertices * 18 + 127) / 128; - if (output_size > std::numeric_limits<u32>::max() || + if (range < bias + size || output_size > std::numeric_limits<u32>::max() || range > std::numeric_limits<u32>::max() || output_size > max_storage_range || range > max_storage_range || groups > max_dispatch_groups) { return std::nullopt; diff --git a/src/video_core/renderer_vulkan/vk_buffer_cache.cpp b/src/video_core/renderer_vulkan/vk_buffer_cache.cpp index 2449265509..4d5ccb6520 100644 --- a/src/video_core/renderer_vulkan/vk_buffer_cache.cpp +++ b/src/video_core/renderer_vulkan/vk_buffer_cache.cpp @@ -394,7 +394,7 @@ u32 BufferCacheRuntime::SetVertexRepackLayout(const Tegra::Engines::Maxwell3D::R LOG_DEBUG(Render_Vulkan, "Vertex repack selection: mask={:#x}, dynamic_input={}", vertex_repack_mask, dynamic_input); } - return old_mask | vertex_repack_mask; + return old_mask ^ vertex_repack_mask; } StagingBufferRef BufferCacheRuntime::UploadStagingBuffer(size_t size) { @@ -657,7 +657,8 @@ void BufferCacheRuntime::BindVertexBuffer(u32 index, VkBuffer buffer, u32 offset return; } if (buffer != VK_NULL_HANDLE && size != 0 && IsVertexRepacked(index)) { - const auto repacked = vertex_repack_pass->Assemble(buffer, offset, size); + const auto repacked = + vertex_repack_pass->Assemble(buffer, offset, size, u64{offset} + size); if (!repacked) { return; } @@ -702,7 +703,8 @@ void BufferCacheRuntime::BindVertexBuffers(VideoCommon::HostBindings<Buffer>& bi if (handle != VK_NULL_HANDLE && IsVertexRepacked(binding) && bindings.strides[i] == 69 && bindings.sizes[i] > 0) { const auto repacked = vertex_repack_pass->Assemble( - handle, static_cast<u32>(bindings.offsets[i]), static_cast<u32>(bindings.sizes[i])); + handle, static_cast<u32>(bindings.offsets[i]), static_cast<u32>(bindings.sizes[i]), + bindings.buffers[i]->SizeBytes()); if (!repacked) { continue; } diff --git a/src/video_core/renderer_vulkan/vk_buffer_cache.h b/src/video_core/renderer_vulkan/vk_buffer_cache.h index d3295ec6ed..a8d0274016 100644 --- a/src/video_core/renderer_vulkan/vk_buffer_cache.h +++ b/src/video_core/renderer_vulkan/vk_buffer_cache.h @@ -110,6 +110,7 @@ public: void TickFrame(Common::SlotVector<Buffer>& slot_buffers) noexcept; + /// Returns the mask of bindings whose repacked state changed. u32 SetVertexRepackLayout(const Tegra::Engines::Maxwell3D::Regs& regs, bool dynamic_input); bool IsVertexRepacked(u32 binding) const { diff --git a/src/video_core/renderer_vulkan/vk_compute_pass.cpp b/src/video_core/renderer_vulkan/vk_compute_pass.cpp index cb8dcde42f..5902b7d8f5 100644 --- a/src/video_core/renderer_vulkan/vk_compute_pass.cpp +++ b/src/video_core/renderer_vulkan/vk_compute_pass.cpp @@ -335,10 +335,11 @@ VertexRepackPass::VertexRepackPass(const Device& device_, Scheduler& scheduler_, scheduler{scheduler_}, staging_pool{staging_pool_}, descriptor_queue{descriptor_queue_} {} std::optional<std::pair<VkBuffer, VkDeviceSize>> VertexRepackPass::Assemble(VkBuffer source, - u32 offset, u32 size) { + u32 offset, u32 size, + u64 buffer_size) { const auto parameters = MakeVertexRepackParameters( offset, size, device.GetStorageBufferAlignment(), device.GetMaxStorageBufferRange(), - device.GetMaxComputeWorkGroupCount()[0]); + device.GetMaxComputeWorkGroupCount()[0], buffer_size); if (!parameters) { LOG_ERROR(Render_Vulkan, "Vertex repack exceeds device limits: offset={}, size={}", offset, size); return std::nullopt; diff --git a/src/video_core/renderer_vulkan/vk_compute_pass.h b/src/video_core/renderer_vulkan/vk_compute_pass.h index 27fffbbbb7..9687bfff91 100644 --- a/src/video_core/renderer_vulkan/vk_compute_pass.h +++ b/src/video_core/renderer_vulkan/vk_compute_pass.h @@ -60,9 +60,10 @@ class VertexRepackPass final : public ComputePass { public: VertexRepackPass(const Device& device, Scheduler& scheduler, DescriptorPool& descriptor_pool, StagingBufferPool& staging_pool, ComputePassDescriptorQueue& descriptor_queue); - /// Returns nothing when the range exceeds the device limits; the binding is then left as is. - std::optional<std::pair<VkBuffer, VkDeviceSize>> Assemble(VkBuffer source, u32 offset, - u32 size); + /// Returns nothing when the range exceeds the device limits or the source buffer; the binding + /// is then left as is. + std::optional<std::pair<VkBuffer, VkDeviceSize>> Assemble(VkBuffer source, u32 offset, u32 size, + u64 buffer_size); private: Scheduler& scheduler; |
