video_core: Use sampler IDs instead pointers in the pipeline config

The previous approach of storing pointers returned by `GetGraphicsSampler`/`GetComputeSampler` caused UB, as these functions can cause reallocation of the sampler slot vector and therefore invalidate the pointers

8 files changed, 68 insertions, 23 deletions

diff --git a/src/video_core/renderer_opengl/gl_compute_pipeline.cpp b/src/video_core/renderer_opengl/gl_compute_pipeline.cpp
index 1a0cea9b7..3151c0db8 100644
--- a/src/video_core/renderer_opengl/gl_compute_pipeline.cpp
+++ b/src/video_core/renderer_opengl/gl_compute_pipeline.cpp
@@ -87,7 +87,8 @@ void ComputePipeline::Configure() {
     texture_cache.SynchronizeComputeDescriptors();
 
     boost::container::static_vector<VideoCommon::ImageViewInOut, MAX_TEXTURES + MAX_IMAGES> views;
-    std::array<GLuint, MAX_TEXTURES> samplers;
+    boost::container::static_vector<VideoCommon::SamplerId, MAX_TEXTURES> samplers;
+    std::array<GLuint, MAX_TEXTURES> gl_samplers;
     std::array<GLuint, MAX_TEXTURES> textures;
     std::array<GLuint, MAX_IMAGES> images;
     GLsizei sampler_binding{};
@@ -131,7 +132,6 @@ void ComputePipeline::Configure() {
         for (u32 index = 0; index < desc.count; ++index) {
             const auto handle{read_handle(desc, index)};
             views.push_back({handle.first});
-            samplers[sampler_binding++] = 0;
         }
     }
     for (const auto& desc : info.image_buffer_descriptors) {
@@ -142,8 +142,8 @@ void ComputePipeline::Configure() {
             const auto handle{read_handle(desc, index)};
             views.push_back({handle.first});
 
-            Sampler* const sampler = texture_cache.GetComputeSampler(handle.second);
-            samplers[sampler_binding++] = sampler->Handle();
+            VideoCommon::SamplerId sampler = texture_cache.GetComputeSamplerId(handle.second);
+            samplers.push_back(sampler);
         }
     }
     for (const auto& desc : info.image_descriptors) {
@@ -186,10 +186,17 @@ void ComputePipeline::Configure() {
 
     const VideoCommon::ImageViewInOut* views_it{views.data() + num_texture_buffers +
                                                 num_image_buffers};
+    const VideoCommon::SamplerId* samplers_it{samplers.data()};
     texture_binding += num_texture_buffers;
     image_binding += num_image_buffers;
 
     u32 texture_scaling_mask{};
+
+    for (const auto& desc : info.texture_buffer_descriptors) {
+        for (u32 index = 0; index < desc.count; ++index) {
+            gl_samplers[sampler_binding++] = 0;
+        }
+    }
     for (const auto& desc : info.texture_descriptors) {
         for (u32 index = 0; index < desc.count; ++index) {
             ImageView& image_view{texture_cache.GetImageView((views_it++)->id)};
@@ -198,6 +205,12 @@ void ComputePipeline::Configure() {
                 texture_scaling_mask |= 1u << texture_binding;
             }
             ++texture_binding;
+
+            const Sampler& sampler{texture_cache.GetSampler(*(samplers_it++))};
+            const bool use_fallback_sampler{sampler.HasAddedAnisotropy() &&
+                                            !image_view.SupportsAnisotropy()};
+            gl_samplers[sampler_binding++] =
+                use_fallback_sampler ? sampler.HandleWithDefaultAnisotropy() : sampler.Handle();
         }
     }
     u32 image_scaling_mask{};
@@ -228,7 +241,7 @@ void ComputePipeline::Configure() {
     if (texture_binding != 0) {
         ASSERT(texture_binding == sampler_binding);
         glBindTextures(0, texture_binding, textures.data());
-        glBindSamplers(0, sampler_binding, samplers.data());
+        glBindSamplers(0, sampler_binding, gl_samplers.data());
     }
     if (image_binding != 0) {
         glBindImageTextures(0, image_binding, images.data());
diff --git a/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp b/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp
index 58e4e1919..c58f760b8 100644
--- a/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp
+++ b/src/video_core/renderer_opengl/gl_graphics_pipeline.cpp
@@ -275,7 +275,7 @@ GraphicsPipeline::GraphicsPipeline(const Device& device, TextureCache& texture_c
 template <typename Spec>
 void GraphicsPipeline::ConfigureImpl(bool is_indexed) {
     std::array<VideoCommon::ImageViewInOut, MAX_TEXTURES + MAX_IMAGES> views;
-    std::array<const Sampler*, MAX_TEXTURES> samplers;
+    std::array<VideoCommon::SamplerId, MAX_TEXTURES> samplers;
     size_t views_index{};
     size_t samplers_index{};
 
@@ -350,7 +350,7 @@ void GraphicsPipeline::ConfigureImpl(bool is_indexed) {
                 const auto handle{read_handle(desc, index)};
                 views[views_index++] = {handle.first};
 
-                Sampler* const sampler{texture_cache.GetGraphicsSampler(handle.second)};
+                VideoCommon::SamplerId sampler{texture_cache.GetGraphicsSamplerId(handle.second)};
                 samplers[samplers_index++] = sampler;
             }
         }
@@ -444,7 +444,7 @@ void GraphicsPipeline::ConfigureImpl(bool is_indexed) {
         program_manager.BindSourcePrograms(source_programs);
     }
     const VideoCommon::ImageViewInOut* views_it{views.data()};
-    const Sampler** samplers_it{samplers.data()};
+    const VideoCommon::SamplerId* samplers_it{samplers.data()};
     GLsizei texture_binding = 0;
     GLsizei image_binding = 0;
     GLsizei sampler_binding{};
@@ -484,7 +484,7 @@ void GraphicsPipeline::ConfigureImpl(bool is_indexed) {
                 ++texture_binding;
                 ++stage_texture_binding;
 
-                const Sampler& sampler{**(samplers_it++)};
+                const Sampler& sampler{texture_cache.GetSampler(*(samplers_it++))};
                 const bool use_fallback_sampler{sampler.HasAddedAnisotropy() &&
                                                 !image_view.SupportsAnisotropy()};
                 gl_samplers[sampler_binding++] =
diff --git a/src/video_core/renderer_vulkan/pipeline_helper.h b/src/video_core/renderer_vulkan/pipeline_helper.h
index 0a9dce937..71c783709 100644
--- a/src/video_core/renderer_vulkan/pipeline_helper.h
+++ b/src/video_core/renderer_vulkan/pipeline_helper.h
@@ -178,7 +178,7 @@ public:
 inline void PushImageDescriptors(TextureCache& texture_cache,
                                  GuestDescriptorQueue& guest_descriptor_queue,
                                  const Shader::Info& info, RescalingPushConstant& rescaling,
-                                 const Sampler**& samplers,
+                                 const VideoCommon::SamplerId*& samplers,
                                  const VideoCommon::ImageViewInOut*& views) {
     const u32 num_texture_buffers = Shader::NumDescriptors(info.texture_buffer_descriptors);
     const u32 num_image_buffers = Shader::NumDescriptors(info.image_buffer_descriptors);
@@ -187,9 +187,10 @@ inline void PushImageDescriptors(TextureCache& texture_cache,
     for (const auto& desc : info.texture_descriptors) {
         for (u32 index = 0; index < desc.count; ++index) {
             const VideoCommon::ImageViewId image_view_id{(views++)->id};
+            const VideoCommon::SamplerId sampler_id{*(samplers++)};
             ImageView& image_view{texture_cache.GetImageView(image_view_id)};
             const VkImageView vk_image_view{image_view.Handle(desc.type)};
-            const Sampler& sampler{**(samplers++)};
+            const Sampler& sampler{texture_cache.GetSampler(sampler_id)};
             const bool use_fallback_sampler{sampler.HasAddedAnisotropy() &&
                                             !image_view.SupportsAnisotropy()};
             const VkSampler vk_sampler{use_fallback_sampler ? sampler.HandleWithDefaultAnisotropy()
diff --git a/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp b/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp
index eb292dc34..73e585c2b 100644
--- a/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp
+++ b/src/video_core/renderer_vulkan/vk_compute_pipeline.cpp
@@ -115,7 +115,7 @@ void ComputePipeline::Configure(Tegra::Engines::KeplerCompute& kepler_compute,
 
     static constexpr size_t max_elements = 64;
     boost::container::static_vector<VideoCommon::ImageViewInOut, max_elements> views;
-    boost::container::static_vector<const Sampler*, max_elements> samplers;
+    boost::container::static_vector<VideoCommon::SamplerId, max_elements> samplers;
 
     const auto& qmd{kepler_compute.launch_description};
     const auto& cbufs{qmd.const_buffer_config};
@@ -160,7 +160,7 @@ void ComputePipeline::Configure(Tegra::Engines::KeplerCompute& kepler_compute,
             const auto handle{read_handle(desc, index)};
             views.push_back({handle.first});
 
-            Sampler* const sampler = texture_cache.GetComputeSampler(handle.second);
+            VideoCommon::SamplerId sampler = texture_cache.GetComputeSamplerId(handle.second);
             samplers.push_back(sampler);
         }
     }
@@ -192,7 +192,7 @@ void ComputePipeline::Configure(Tegra::Engines::KeplerCompute& kepler_compute,
     buffer_cache.BindHostComputeBuffers();
 
     RescalingPushConstant rescaling;
-    const Sampler** samplers_it{samplers.data()};
+    const VideoCommon::SamplerId* samplers_it{samplers.data()};
     const VideoCommon::ImageViewInOut* views_it{views.data()};
     PushImageDescriptors(texture_cache, guest_descriptor_queue, info, rescaling, samplers_it,
                          views_it);
diff --git a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp
index e6e004cb6..c1595642e 100644
--- a/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp
+++ b/src/video_core/renderer_vulkan/vk_graphics_pipeline.cpp
@@ -298,7 +298,7 @@ void GraphicsPipeline::AddTransition(GraphicsPipeline* transition) {
 template <typename Spec>
 void GraphicsPipeline::ConfigureImpl(bool is_indexed) {
     std::array<VideoCommon::ImageViewInOut, MAX_IMAGE_ELEMENTS> views;
-    std::array<const Sampler*, MAX_IMAGE_ELEMENTS> samplers;
+    std::array<VideoCommon::SamplerId, MAX_IMAGE_ELEMENTS> samplers;
     size_t sampler_index{};
     size_t view_index{};
 
@@ -367,7 +367,7 @@ void GraphicsPipeline::ConfigureImpl(bool is_indexed) {
                 const auto handle{read_handle(desc, index)};
                 views[view_index++] = {handle.first};
 
-                Sampler* const sampler{texture_cache.GetGraphicsSampler(handle.second)};
+                VideoCommon::SamplerId sampler{texture_cache.GetGraphicsSamplerId(handle.second)};
                 samplers[sampler_index++] = sampler;
             }
         }
@@ -453,7 +453,7 @@ void GraphicsPipeline::ConfigureImpl(bool is_indexed) {
 
     RescalingPushConstant rescaling;
     RenderAreaPushConstant render_area;
-    const Sampler** samplers_it{samplers.data()};
+    const VideoCommon::SamplerId* samplers_it{samplers.data()};
     const VideoCommon::ImageViewInOut* views_it{views.data()};
     const auto prepare_stage{[&](size_t stage) LAMBDA_FORCEINLINE {
         buffer_cache.BindHostStageBuffers(stage);
diff --git a/src/video_core/texture_cache/image_view_base.cpp b/src/video_core/texture_cache/image_view_base.cpp
index 5ebe9b9bb..000eec869 100644
--- a/src/video_core/texture_cache/image_view_base.cpp
+++ b/src/video_core/texture_cache/image_view_base.cpp
@@ -46,7 +46,6 @@ ImageViewBase::ImageViewBase(const ImageInfo& info, const ImageViewInfo& view_in
 ImageViewBase::ImageViewBase(const NullImageViewParams&) : image_id{NULL_IMAGE_ID} {}
 
 bool ImageViewBase::SupportsAnisotropy() const noexcept {
-    using namespace VideoCommon;
     switch (format) {
     case PixelFormat::R8_UNORM:
     case PixelFormat::R8_SNORM:
diff --git a/src/video_core/texture_cache/texture_cache.h b/src/video_core/texture_cache/texture_cache.h
index c7f7448e9..4027d860b 100644
--- a/src/video_core/texture_cache/texture_cache.h
+++ b/src/video_core/texture_cache/texture_cache.h
@@ -222,30 +222,50 @@ void TextureCache<P>::CheckFeedbackLoop(std::span<const ImageViewInOut> views) {
 
 template <class P>
 typename P::Sampler* TextureCache<P>::GetGraphicsSampler(u32 index) {
+    return &slot_samplers[GetGraphicsSamplerId(index)];
+}
+
+template <class P>
+typename P::Sampler* TextureCache<P>::GetComputeSampler(u32 index) {
+    return &slot_samplers[GetComputeSamplerId(index)];
+}
+
+template <class P>
+SamplerId TextureCache<P>::GetGraphicsSamplerId(u32 index) {
     if (index > channel_state->graphics_sampler_table.Limit()) {
         LOG_DEBUG(HW_GPU, "Invalid sampler index={}", index);
-        return &slot_samplers[NULL_SAMPLER_ID];
+        return NULL_SAMPLER_ID;
     }
     const auto [descriptor, is_new] = channel_state->graphics_sampler_table.Read(index);
     SamplerId& id = channel_state->graphics_sampler_ids[index];
     if (is_new) {
         id = FindSampler(descriptor);
     }
-    return &slot_samplers[id];
+    return id;
 }
 
 template <class P>
-typename P::Sampler* TextureCache<P>::GetComputeSampler(u32 index) {
+SamplerId TextureCache<P>::GetComputeSamplerId(u32 index) {
     if (index > channel_state->compute_sampler_table.Limit()) {
         LOG_DEBUG(HW_GPU, "Invalid sampler index={}", index);
-        return &slot_samplers[NULL_SAMPLER_ID];
+        return NULL_SAMPLER_ID;
     }
     const auto [descriptor, is_new] = channel_state->compute_sampler_table.Read(index);
     SamplerId& id = channel_state->compute_sampler_ids[index];
     if (is_new) {
         id = FindSampler(descriptor);
     }
-    return &slot_samplers[id];
+    return id;
+}
+
+template <class P>
+const typename P::Sampler& TextureCache<P>::GetSampler(SamplerId id) const noexcept {
+    return slot_samplers[id];
+}
+
+template <class P>
+typename P::Sampler& TextureCache<P>::GetSampler(SamplerId id) noexcept {
+    return slot_samplers[id];
 }
 
 template <class P>
diff --git a/src/video_core/texture_cache/texture_cache_base.h b/src/video_core/texture_cache/texture_cache_base.h
index 3bfa92154..d96ddea9d 100644
--- a/src/video_core/texture_cache/texture_cache_base.h
+++ b/src/video_core/texture_cache/texture_cache_base.h
@@ -159,6 +159,18 @@ public:
     /// Get the sampler from the compute descriptor table in the specified index
     Sampler* GetComputeSampler(u32 index);
 
+    /// Get the sampler id from the graphics descriptor table in the specified index
+    SamplerId GetGraphicsSamplerId(u32 index);
+
+    /// Get the sampler id from the compute descriptor table in the specified index
+    SamplerId GetComputeSamplerId(u32 index);
+
+    /// Return a constant reference to the given sampler id
+    [[nodiscard]] const Sampler& GetSampler(SamplerId id) const noexcept;
+
+    /// Return a reference to the given sampler id
+    [[nodiscard]] Sampler& GetSampler(SamplerId id) noexcept;
+
     /// Refresh the state for graphics image view and sampler descriptors
     void SynchronizeGraphicsDescriptors();