From 9e015f703c2e2e1340fbdbc353bea2e9b64782c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20Foucault?= <foucault.clem@gmail.com>
Date: Fri, 2 Feb 2024 19:04:45 +0100
Subject: [PATCH] Fix: EEVEE-Next: Shadow: Fix infinite loop in shadow
 rendering

The render shadow loop would always tag new casters to update
the tiles that were already rendered. This patch split the
caster tagging into it's own pass and move it out of the loop.

Also adds a needed `async_flush_to_host` to make sure the
statistic buffer is up to date.
---
 .../draw/engines/eevee_next/eevee_shadow.cc   | 52 +++++++++++--------
 .../draw/engines/eevee_next/eevee_shadow.hh   |  1 +
 2 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/source/blender/draw/engines/eevee_next/eevee_shadow.cc b/source/blender/draw/engines/eevee_next/eevee_shadow.cc
index 27559d0b9d3..444bb95c51d 100644
--- a/source/blender/draw/engines/eevee_next/eevee_shadow.cc
+++ b/source/blender/draw/engines/eevee_next/eevee_shadow.cc
@@ -1008,6 +1008,28 @@ void ShadowModule::end_sync()
   {
     Manager &manager = *inst_.manager;
 
+    {
+      /* Mark for update all shadow pages touching an updated shadow caster. */
+      PassSimple &pass = caster_update_ps_;
+      pass.init();
+      pass.shader_set(inst_.shaders.static_shader_get(SHADOW_TILEMAP_TAG_UPDATE));
+      pass.bind_ssbo("tilemaps_buf", tilemap_pool.tilemaps_data);
+      pass.bind_ssbo("tiles_buf", tilemap_pool.tiles_data);
+      /* Past caster transforms. */
+      if (past_casters_updated_.size() > 0) {
+        pass.bind_ssbo("bounds_buf", &manager.bounds_buf.previous());
+        pass.bind_ssbo("resource_ids_buf", past_casters_updated_);
+        pass.dispatch(int3(past_casters_updated_.size(), 1, tilemap_pool.tilemaps_data.size()));
+      }
+      /* Current caster transforms. */
+      if (curr_casters_updated_.size() > 0) {
+        pass.bind_ssbo("bounds_buf", &manager.bounds_buf.current());
+        pass.bind_ssbo("resource_ids_buf", curr_casters_updated_);
+        pass.dispatch(int3(curr_casters_updated_.size(), 1, tilemap_pool.tilemaps_data.size()));
+      }
+      pass.barrier(GPU_BARRIER_SHADER_STORAGE);
+    }
+
     {
       PassSimple &pass = tilemap_setup_ps_;
       pass.init();
@@ -1055,26 +1077,6 @@ void ShadowModule::end_sync()
         }
         sub.barrier(GPU_BARRIER_SHADER_STORAGE);
       }
-      {
-        /* Mark for update all shadow pages touching an updated shadow caster. */
-        PassSimple::Sub &sub = pass.sub("CasterUpdate");
-        sub.shader_set(inst_.shaders.static_shader_get(SHADOW_TILEMAP_TAG_UPDATE));
-        sub.bind_ssbo("tilemaps_buf", tilemap_pool.tilemaps_data);
-        sub.bind_ssbo("tiles_buf", tilemap_pool.tiles_data);
-        /* Past caster transforms. */
-        if (past_casters_updated_.size() > 0) {
-          sub.bind_ssbo("bounds_buf", &manager.bounds_buf.previous());
-          sub.bind_ssbo("resource_ids_buf", past_casters_updated_);
-          sub.dispatch(int3(past_casters_updated_.size(), 1, tilemap_pool.tilemaps_data.size()));
-        }
-        /* Current caster transforms. */
-        if (curr_casters_updated_.size() > 0) {
-          sub.bind_ssbo("bounds_buf", &manager.bounds_buf.current());
-          sub.bind_ssbo("resource_ids_buf", curr_casters_updated_);
-          sub.dispatch(int3(curr_casters_updated_.size(), 1, tilemap_pool.tilemaps_data.size()));
-        }
-        sub.barrier(GPU_BARRIER_SHADER_STORAGE);
-      }
     }
 
     /* Non volume usage tagging happens between these two steps.
@@ -1275,7 +1277,7 @@ bool ShadowModule::shadow_update_finished()
     return true;
   }
 
-  int max_updated_view_count = tilemap_pool.tilemaps_data.size();
+  int max_updated_view_count = tilemap_pool.tilemaps_data.size() * SHADOW_TILEMAP_LOD;
   if (max_updated_view_count <= SHADOW_VIEW_MAX) {
     /* There is enough shadow views to cover all tilemap updates.
      * No readback needed as it is guaranteed that all of them will be updated. */
@@ -1283,6 +1285,7 @@ bool ShadowModule::shadow_update_finished()
   }
 
   /* Read back and check if there is still tile-map to update. */
+  statistics_buf_.current().async_flush_to_host();
   statistics_buf_.current().read();
   ShadowStatistics stats = statistics_buf_.current();
   /* Rendering is finished if we rendered all the remaining pages. */
@@ -1335,6 +1338,13 @@ void ShadowModule::set_view(View &view, GPUTexture *depth_tx)
 
   inst_.hiz_buffer.update();
 
+  /* Run caster update once and before the update loop.
+   * This is valid even before the view update since only the static tilemaps
+   * are concerned about this tagging. */
+  /* TODO(fclem): There is an optimization opportunity here where we can
+   * test casters only against the static tilemaps instead of all of them. */
+  inst_.manager->submit(caster_update_ps_, view);
+
   do {
     DRW_stats_group_start("Shadow");
     {
diff --git a/source/blender/draw/engines/eevee_next/eevee_shadow.hh b/source/blender/draw/engines/eevee_next/eevee_shadow.hh
index 6574ce8e96d..2ffe6db1ed8 100644
--- a/source/blender/draw/engines/eevee_next/eevee_shadow.hh
+++ b/source/blender/draw/engines/eevee_next/eevee_shadow.hh
@@ -228,6 +228,7 @@ class ShadowModule {
 
   Framebuffer usage_tag_fb;
 
+  PassSimple caster_update_ps_ = {"CasterUpdate"};
   /** List of Resource IDs (to get bounds) for tagging passes. */
   StorageVectorBuffer<uint, 128> past_casters_updated_ = {"PastCastersUpdated"};
   StorageVectorBuffer<uint, 128> curr_casters_updated_ = {"CurrCastersUpdated"};