Vulkan: Fix out of bound access in begin rendering

There is an implementation flaw in the render graph where local pointers
cannot be updated, but the data it refers to can be reallocated to
another location.

The cause of this is that the nodes use an union, which can only contain
simple constructed structs (eg memcpy). this union is stored in a vector
and can relocate the union. Any local pointers can (and will) become
invalid.

This PR is a quick fix by updating the pointers just before sending
them to the command buffer. In future a better fox needs to be done
as part of #121649.

Pull Request: https://projects.blender.org/blender/blender/pulls/121723

source/blender/gpu/vulkan/render_graph/nodes/vk_begin_rendering_node.hh

@@ -71,19 +71,9 @@ class VKBeginRenderingNode : public VKNodeInfo<VKNodeType::BEGIN_RENDERING,
                    "When create_info.node_data.vk_rendering_info.pStencilAttachment points to "
                    "something, it should point to create_info.node_data.stencil_attachment.");
     node.begin_rendering = create_info.node_data;
-    /* Localize pointers when set.*/
-    if (node.begin_rendering.vk_rendering_info.pColorAttachments) {
-      node.begin_rendering.vk_rendering_info.pColorAttachments =
-          node.begin_rendering.color_attachments;
-    }
-    if (node.begin_rendering.vk_rendering_info.pDepthAttachment) {
-      node.begin_rendering.vk_rendering_info.pDepthAttachment =
-          &node.begin_rendering.depth_attachment;
-    }
-    if (node.begin_rendering.vk_rendering_info.pStencilAttachment) {
-      node.begin_rendering.vk_rendering_info.pStencilAttachment =
-          &node.begin_rendering.stencil_attachment;
-    }
+    /* NOTE: pointers in vk_rendering_info will be set to the correct location just before sending
+     * to the command buffer. In the meantime these pointers are invalid.
+     * VKRenderingAttachmentInfo's should be used instead.*/
   }
 
   /**
@@ -100,9 +90,21 @@ class VKBeginRenderingNode : public VKNodeInfo<VKNodeType::BEGIN_RENDERING,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
+    /* Localize pointers just before sending to the command buffer. Pointer can (and will) change
+     * as they are stored in a union which is stored in a vector. When the vector reallocates, the
+     * pointers will become invalid. */
+    if (data.vk_rendering_info.pColorAttachments) {
+      data.vk_rendering_info.pColorAttachments = data.color_attachments;
+    }
+    if (data.vk_rendering_info.pDepthAttachment) {
+      data.vk_rendering_info.pDepthAttachment = &data.depth_attachment;
+    }
+    if (data.vk_rendering_info.pStencilAttachment) {
+      data.vk_rendering_info.pStencilAttachment = &data.stencil_attachment;
+    }
     command_buffer.begin_rendering(&data.vk_rendering_info);
   }
 };

source/blender/gpu/vulkan/render_graph/nodes/vk_blit_image_node.hh

@@ -70,7 +70,7 @@ class VKBlitImageNode : public VKNodeInfo<VKNodeType::BLIT_IMAGE,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.blit_image(data.src_image,

source/blender/gpu/vulkan/render_graph/nodes/vk_clear_attachments_node.hh

@@ -56,7 +56,7 @@ class VKClearAttachmentsNode : public VKNodeInfo<VKNodeType::CLEAR_ATTACHMENTS,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.clear_attachments(

source/blender/gpu/vulkan/render_graph/nodes/vk_clear_color_image_node.hh

@@ -57,7 +57,7 @@ class VKClearColorImageNode : public VKNodeInfo<VKNodeType::CLEAR_COLOR_IMAGE,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.clear_color_image(data.vk_image,

source/blender/gpu/vulkan/render_graph/nodes/vk_clear_depth_stencil_image_node.hh

@@ -57,7 +57,7 @@ class VKClearDepthStencilImageNode : public VKNodeInfo<VKNodeType::CLEAR_DEPTH_S
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.clear_depth_stencil_image(data.vk_image,

source/blender/gpu/vulkan/render_graph/nodes/vk_copy_buffer_node.hh

@@ -58,7 +58,7 @@ class VKCopyBufferNode : public VKNodeInfo<VKNodeType::COPY_BUFFER,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.copy_buffer(data.src_buffer, data.dst_buffer, 1, &data.region);

source/blender/gpu/vulkan/render_graph/nodes/vk_copy_buffer_to_image_node.hh

@@ -59,7 +59,7 @@ class VKCopyBufferToImageNode : public VKNodeInfo<VKNodeType::COPY_BUFFER_TO_IMA
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.copy_buffer_to_image(

source/blender/gpu/vulkan/render_graph/nodes/vk_copy_image_node.hh

@@ -65,7 +65,7 @@ class VKCopyImageNode : public VKNodeInfo<VKNodeType::COPY_IMAGE,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.copy_image(data.src_image,

source/blender/gpu/vulkan/render_graph/nodes/vk_copy_image_to_buffer_node.hh

@@ -64,7 +64,7 @@ class VKCopyImageToBufferNode : public VKNodeInfo<VKNodeType::COPY_IMAGE_TO_BUFF
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.copy_image_to_buffer(

source/blender/gpu/vulkan/render_graph/nodes/vk_dispatch_indirect_node.hh

@@ -76,7 +76,7 @@ class VKDispatchIndirectNode : public VKNodeInfo<VKNodeType::DISPATCH_INDIRECT,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines &r_bound_pipelines) override
   {
     vk_pipeline_data_build_commands(

source/blender/gpu/vulkan/render_graph/nodes/vk_dispatch_node.hh

@@ -73,7 +73,7 @@ class VKDispatchNode : public VKNodeInfo<VKNodeType::DISPATCH,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines &r_bound_pipelines) override
   {
     vk_pipeline_data_build_commands(

source/blender/gpu/vulkan/render_graph/nodes/vk_end_rendering_node.hh

@@ -55,7 +55,7 @@ class VKEndRenderingNode : public VKNodeInfo<VKNodeType::END_RENDERING,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data & /*data*/,
+                      Data & /*data*/,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.end_rendering();

source/blender/gpu/vulkan/render_graph/nodes/vk_fill_buffer_node.hh

@@ -53,7 +53,7 @@ class VKFillBufferNode : public VKNodeInfo<VKNodeType::FILL_BUFFER,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     command_buffer.fill_buffer(data.vk_buffer, 0, data.size, data.data);

source/blender/gpu/vulkan/render_graph/nodes/vk_node_info.hh

@@ -103,7 +103,7 @@ class VKNodeInfo : public NonCopyable {
    * cases. The test cases will validate the log to find out if the correct commands where added.
    */
   virtual void build_commands(VKCommandBufferInterface &command_buffer,
-                              const Data &data,
+                              Data &data,
                               VKBoundPipelines &r_bound_pipelines) = 0;
 };
 }  // namespace blender::gpu::render_graph

source/blender/gpu/vulkan/render_graph/nodes/vk_synchronization_node.hh

@@ -63,7 +63,7 @@ class VKSynchronizationNode : public VKNodeInfo<VKNodeType::SYNCHRONIZATION,
    * Build the commands and add them to the command_buffer.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      const Data &data,
+                      Data &data,
                       VKBoundPipelines & /*r_bound_pipelines*/) override
   {
     UNUSED_VARS(command_buffer, data);

source/blender/gpu/vulkan/render_graph/tests/vk_render_graph_test_render.cc

@@ -10,7 +10,6 @@ namespace blender::gpu::render_graph {
 
 TEST(vk_render_graph, begin_clear_attachments_end_read_back)
 {
-  GTEST_SKIP() << "Disabled as it doesn't succeed consistently on Windows";
   VkHandle<VkImage> image(1u);
   VkHandle<VkImageView> image_view(2u);
   VkHandle<VkBuffer> buffer(3u);

source/blender/gpu/vulkan/render_graph/vk_command_builder.cc

@@ -65,7 +65,7 @@ void VKCommandBuilder::build_nodes(VKRenderGraph &render_graph,
 void VKCommandBuilder::build_node(VKRenderGraph &render_graph,
                                   VKCommandBufferInterface &command_buffer,
                                   NodeHandle node_handle,
-                                  const VKRenderGraphNode &node)
+                                  VKRenderGraphNode &node)
 {
   build_pipeline_barriers(render_graph, command_buffer, node_handle, node.pipeline_stage_get());
   node.build_commands(command_buffer, state_.active_pipelines);

source/blender/gpu/vulkan/render_graph/vk_command_builder.hh

@@ -81,7 +81,7 @@ class VKCommandBuilder {
   void build_node(VKRenderGraph &render_graph,
                   VKCommandBufferInterface &command_buffer,
                   NodeHandle node_handle,
-                  const VKRenderGraphNode &node);
+                  VKRenderGraphNode &node);
 
   /**
    * Build the pipeline barriers that should be recorded before the given node handle.

source/blender/gpu/vulkan/render_graph/vk_render_graph_node.hh

@@ -140,7 +140,7 @@ struct VKRenderGraphNode {
    * `VKCommandBuilder::build_node` and `VKCommandBuilder::build_pipeline_barriers.
    */
   void build_commands(VKCommandBufferInterface &command_buffer,
-                      VKBoundPipelines &r_bound_pipelines) const
+                      VKBoundPipelines &r_bound_pipelines)
   {
     switch (type) {
       case VKNodeType::UNUSED: {