#5972 LLUIImage based buffer cache #2

Simplify key a bit.

4 files changed, 63 insertions, 96 deletions

diff --git a/indra/llrender/lluiimage.cpp b/indra/llrender/lluiimage.cpp
index b1394a07bc..85754209a4 100644
--- a/indra/llrender/lluiimage.cpp
+++ b/indra/llrender/lluiimage.cpp
@@ -31,7 +31,6 @@
 
 // Project includes
 #include "lluiimage.h"
-#include <chrono>
 #include <algorithm>
 
 // Static member initialization
@@ -57,21 +56,10 @@ LLUIImage::~LLUIImage()
 {
     delete mImageLoaded;
 
-    if (!mDisplayLists.empty())
-    {
-        llassert(false);
-        // Unregister from global cleanup list (sanity check)
-        // But it's supposed to be cleared already, else we wouldn't
-        // be destructing this opbject.
-        auto it = std::find(sImageList.begin(), sImageList.end(), this);
-        if (it != sImageList.end())
-        {
-            // Swap with last element and pop (O(1) removal)
-            *it = sImageList.back();
-            sImageList.pop_back();
-        }
-    }
-
+    // Unregister from global cleanup list (sanity check)
+    // But it's supposed to be cleared already, else we wouldn't
+    // be destructing this object.
+    unregisterFromGlobalCleanup();
     mDisplayLists.clear();
 }
 
@@ -91,12 +79,12 @@ buffer_data_list_t* LLUIImage::findDisplayList(S32 x, S32 y, S32 width, S32 heig
 {
     LLVector3 ui_translation = gGL.getUITranslation();
     LLVector3 ui_scale = gGL.getUIScale();
-    DisplayListKey key{ x, y, width, height, color, solid_color, ui_translation, ui_scale };
+
+    auto key = PackedKey::create(x, y, width, height, color, solid_color, ui_translation, ui_scale);
 
     auto it = mDisplayLists.find(key);
     if (it != mDisplayLists.end())
     {
-        // Found cached display list, update last used time
         it->second.last_used = std::chrono::steady_clock::now();
         return &it->second.list;
     }
@@ -108,7 +96,7 @@ buffer_data_list_t* LLUIImage::genDisplayList(S32 x, S32 y, S32 width, S32 heigh
     LL_PROFILE_ZONE_SCOPED;
     LLVector3 ui_translation = gGL.getUITranslation();
     LLVector3 ui_scale = gGL.getUIScale();
-    DisplayListKey key{ x, y, width, height, color, solid_color, ui_translation, ui_scale };
+    auto key = PackedKey::create(x, y, width, height, color, solid_color, ui_translation, ui_scale);
 
     CachedDisplayList cached;
     cached.last_used = std::chrono::steady_clock::now();
@@ -129,6 +117,7 @@ buffer_data_list_t* LLUIImage::genDisplayList(S32 x, S32 y, S32 width, S32 heigh
     gGL.endList();
 
     // Insert into cache
+    // emplace, since we only call genDisplayList if key was not found.
     auto result = mDisplayLists.emplace(key, std::move(cached));
 
     // Register for cleanup on first buffer creation
@@ -154,7 +143,18 @@ void LLUIImage::cleanupDisplayLists()
         llassert(false); //it shouldn't be in this list
         unregisterFromGlobalCleanup();
         // marks current position for a recheck. Increments after cleanupDisplayLists.
-        sCleanupIndex--;
+        if (sCleanupIndex > 0)
+        {
+            sCleanupIndex--;
+        }
+        else if (sImageList.empty())
+        {
+            sCleanupIndex = 0;
+        }
+        else
+        {
+            sCleanupIndex = sImageList.size() - 1;
+        }
         return;
     }
 
@@ -180,7 +180,18 @@ void LLUIImage::cleanupDisplayLists()
     {
         unregisterFromGlobalCleanup();
         // marks current position for a recheck. Increments after cleanupDisplayLists.
-        sCleanupIndex--;
+        if (sCleanupIndex > 0)
+        {
+            sCleanupIndex--;
+        }
+        else if (sImageList.empty())
+        {
+            sCleanupIndex = 0;
+        }
+        else
+        {
+            sCleanupIndex = sImageList.size() - 1;
+        }
     }
 }
 
diff --git a/indra/llrender/lluiimage.h b/indra/llrender/lluiimage.h
index 9e481b6615..6f33c31bfe 100644
--- a/indra/llrender/lluiimage.h
+++ b/indra/llrender/lluiimage.h
@@ -38,7 +38,11 @@
 
 #include <boost/signals2.hpp>
 
+#include <bit>
+#include <chrono>
 #include <type_traits>
+#include <unordered_map>
+#include <vector>
 
 extern const LLColor4 UI_VERTEX_COLOR;
 
@@ -120,46 +124,42 @@ public:
     static void enableDisplayListsCollection(bool enable) { sEnableDisplayListsCollection = enable; }
 
 protected:
-    // Key for identifying unique display list configurations
-    struct DisplayListKey
+    // Packed key for identifying unique display list configurations
+    struct PackedKey
     {
-        S32 x;
-        S32 y;
-        S32 width;
-        S32 height;
-        LLColor4 color;
-        bool solid_color;
-        LLVector3 translate;
-        LLVector3 scale;
+        uint64_t position;    // x and y coordinates
+        uint64_t color_flags; // RGBA color + solid_color flag
+        uint64_t dimensions;  // width and height
+        uint64_t translate;   // UI offset
+        uint64_t scale;       // UI scale
 
-        // Pack for hashing
-        struct PackedKey
+        constexpr bool operator==(const PackedKey& other) const
         {
-            uint64_t position;    // x and y coordinates
-            uint64_t color_flags; // RGBA color + solid_color flag
-            uint64_t dimensions;  // width and height
-            // todo: fix, translation, scale, position, shouldn't be needed
-            uint64_t translate;   // UI offset
-            uint64_t scale;      // UI scale
+            return position == other.position &&
+                color_flags == other.color_flags &&
+                dimensions == other.dimensions &&
+                translate == other.translate &&
+                scale == other.scale;
+        }
 
-            constexpr bool operator==(const PackedKey& other) const
+        struct Hash
+        {
+            std::size_t operator()(const PackedKey& key) const
             {
-                return position == other.position &&
-                    color_flags == other.color_flags &&
-                    dimensions == other.dimensions &&
-                    translate == other.translate &&
-                    scale == other.scale;
+                return static_cast<std::size_t>(key.position ^ key.color_flags ^
+                                               key.dimensions ^ key.translate ^ key.scale);
             }
         };
 
-        constexpr PackedKey pack() const
+        // Static factory function to create PackedKey from parameters
+        static constexpr PackedKey create(S32 x, S32 y, S32 width, S32 height,
+                                         const LLColor4& color, bool solid_color,
+                                         const LLVector3& translate, const LLVector3& scale)
         {
-            // Convert floats to 8-bit values for packing (0.0-1.0 -> 0-255)
             auto float_to_u8 = [](F32 f) -> uint8_t {
                 return static_cast<uint8_t>(llclamp(f * 255.0f, 0.0f, 255.0f));
             };
 
-            // Helper to convert float to uint32 preserving bit pattern
             auto float_to_bits = [](F32 f) -> uint32_t {
                 return std::bit_cast<uint32_t>(f);
             };
@@ -189,47 +189,6 @@ protected:
 
             return PackedKey{ pos, col, dim, trns, scl };
         }
-
-        constexpr bool operator==(const DisplayListKey& other) const
-        {
-            return pack() == other.pack();
-        }
-
-        struct Hash
-        {
-            using is_transparent = void;  // Enable transparent lookup
-
-            std::size_t operator()(const DisplayListKey& key) const
-            {
-                auto packed = key.pack();
-                return static_cast<std::size_t>(packed.position ^ packed.color_flags ^ packed.dimensions ^ packed.translate ^ packed.scale);
-            }
-
-            std::size_t operator()(const PackedKey& packed) const
-            {
-                return static_cast<std::size_t>(packed.position ^ packed.color_flags ^ packed.dimensions ^ packed.translate ^ packed.scale);
-            }
-        };
-
-        struct KeyEqual
-        {
-            using is_transparent = void;  // Enable transparent lookup
-
-            bool operator()(const DisplayListKey& lhs, const DisplayListKey& rhs) const
-            {
-                return lhs == rhs;
-            }
-
-            bool operator()(const DisplayListKey& lhs, const PackedKey& rhs) const
-            {
-                return lhs.pack() == rhs;
-            }
-
-            bool operator()(const PackedKey& lhs, const DisplayListKey& rhs) const
-            {
-                return lhs == rhs.pack();
-            }
-        };
     };
 
     // Cached display list for a specific configuration
@@ -261,11 +220,8 @@ protected:
     mutable S32             mCachedW;
     mutable S32             mCachedH;
 
-    // Display list cache
-    // const member functions promise not to modify the object's logical state, but
-    // cache does not modify logical state, mutabale to permit const correctness
-    // (standard C++ pattern for transparent caching).
-    mutable std::unordered_map<DisplayListKey, CachedDisplayList, DisplayListKey::Hash> mDisplayLists;
+    // Display list cache - now using PackedKey directly
+    mutable std::unordered_map<PackedKey, CachedDisplayList, PackedKey::Hash> mDisplayLists;
 
     // Track all LLUIImage cache instances for global cleanup
     static std::vector<LLPointer<LLUIImage> > sImageList;
diff --git a/indra/llrender/lluiimage.inl b/indra/llrender/lluiimage.inl
index 1915d0e7ce..d40835a0f4 100644
--- a/indra/llrender/lluiimage.inl
+++ b/indra/llrender/lluiimage.inl
@@ -39,7 +39,7 @@ void LLUIImage::draw(S32 x, S32 y, S32 width, S32 height, const LLColor4& color,
         if (display_list && !display_list->empty())
         {
             // Deliberately empty pending verts.
-            // They aren't related to the iamge, so don't register them under draw
+            // They aren't related to the image, so don't register them under draw zone
             gGL.flush();
             LL_PROFILE_ZONE_SCOPED;
             gGL.getTexUnit(0)->enable(LLTexUnit::TT_TEXTURE);
diff --git a/indra/newview/app_settings/settings.xml b/indra/newview/app_settings/settings.xml
index 43b3fdf1b0..75258cfa79 100644
--- a/indra/newview/app_settings/settings.xml
+++ b/indra/newview/app_settings/settings.xml
@@ -9896,7 +9896,7 @@
   <key>CollectUIImageVertexBuffers</key>
   <map>
     <key>Comment</key>
-    <string>When enabled images will cache buffers and reuse them. When disabled general cahce will be used with a significant overhead for hash, but it regenerates vertices each frame so it's always up to date.</string>
+    <string>When enabled, images will cache vertex buffers and reuse them. When disabled, the general cache will be used with significant hash-lookup overhead, but vertices are regenerated each frame so they are always up to date.</string>
     <key>Persist</key>
     <integer>0</integer>
     <key>Type</key>