From 87002299235987ace504133d8ed6570f3cb87bc2 Mon Sep 17 00:00:00 2001
From: alexis cruz-ayala <[email protected]>
Date: Mon, 19 Oct 2026 18:31:46 -0400
Subject: [PATCH] [skif] Update SkImageFilter_Base uniqueID to 64-bit

SkImageFilter_Base previously assigned each filter a 32-bit uniqueID
from an atomic counter that could wrap after 2^32 constructions and
cause cache collisions in SkImageFilterCache.

Update SkImageFilter_Base::fUniqueID and SkImageFilterCacheKey::fUniqueID to uint64_t, with an explicit zero-initialized padding field in SkImageFilterCacheKey to maintain tight packing for hashing.

Bug: b/568936530
Change-Id: If1b172c41e582bccb3c2323e54bbce43e0fd2671
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1398516
Reviewed-by: Julia Lavrova <[email protected]>
Commit-Queue: Alexis Cruz-Ayala <[email protected]>
Auto-Submit: Alexis Cruz-Ayala <[email protected]>
---
diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 35b9768..fc2cb43 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -135,10 +135,10 @@
 // SkImageFilter_Base
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
-static int32_t next_image_filter_unique_id() {
-    static std::atomic<int32_t> nextID{1};
+static uint64_t next_image_filter_unique_id() {
+    static std::atomic<uint64_t> nextID{1};
 
-    int32_t id;
+    uint64_t id;
     do {
         id = nextID.fetch_add(1, std::memory_order_relaxed);
     } while (id == 0);
diff --git a/src/core/SkImageFilterCache.h b/src/core/SkImageFilterCache.h
index a2909e0..92f40eb 100644
--- a/src/core/SkImageFilterCache.h
+++ b/src/core/SkImageFilterCache.h
@@ -21,7 +21,7 @@
 namespace skif { class FilterResult; }
 
 struct SkImageFilterCacheKey {
-    SkImageFilterCacheKey(const uint32_t uniqueID, const SkMatrix& matrix,
+    SkImageFilterCacheKey(const uint64_t uniqueID, const SkMatrix& matrix,
         const SkIRect& clipBounds, uint32_t srcGenID, const SkIRect& srcSubset)
         : fUniqueID(uniqueID)
         , fMatrix(matrix)
@@ -29,18 +29,20 @@
         , fSrcGenID(srcGenID)
         , fSrcSubset(srcSubset) {
         // Assert that Key is tightly-packed, since it is hashed.
-        static_assert(sizeof(SkImageFilterCacheKey) == sizeof(uint32_t) + sizeof(SkMatrix) +
-                                     sizeof(SkIRect) + sizeof(uint32_t) + 4 * sizeof(int32_t),
+        static_assert(sizeof(SkImageFilterCacheKey) == sizeof(uint64_t) + sizeof(SkMatrix) +
+                                     sizeof(SkIRect) + sizeof(uint32_t) + 4 * sizeof(int32_t) +
+                                     sizeof(uint32_t),
                                      "image_filter_key_tight_packing");
         fMatrix.getType();  // force initialization of type, so hashes match
         SkASSERT(fMatrix.isFinite());   // otherwise we can't rely on == self when comparing keys
     }
 
-    uint32_t fUniqueID;
+    uint64_t fUniqueID;
     SkMatrix fMatrix;
     SkIRect fClipBounds;
     uint32_t fSrcGenID;
     SkIRect fSrcSubset;
+    uint32_t fPad = 0;
 
     bool operator==(const SkImageFilterCacheKey& other) const {
         return fUniqueID == other.fUniqueID &&
diff --git a/src/core/SkImageFilter_Base.h b/src/core/SkImageFilter_Base.h
index 8beea3d..9b3d461 100644
--- a/src/core/SkImageFilter_Base.h
+++ b/src/core/SkImageFilter_Base.h
@@ -127,7 +127,7 @@
     using MatrixCapability = skif::MatrixCapability;
     MatrixCapability getCTMCapability() const;
 
-    uint32_t uniqueID() const { return fUniqueID; }
+    uint64_t uniqueID() const { return fUniqueID; }
 
     static SkFlattenable::Type GetFlattenableType() {
         return kSkImageFilter_Type;
@@ -302,7 +302,7 @@
     skia_private::AutoSTArray<2, sk_sp<SkImageFilter>> fInputs;
 
     bool fUsesSrcInput;
-    uint32_t fUniqueID; // Globally unique
+    uint64_t fUniqueID; // Globally unique
 
     using INHERITED = SkImageFilter;
 };
diff --git a/tests/ImageFilterCacheTest.cpp b/tests/ImageFilterCacheTest.cpp
index fa72b9c..3d7080f 100644
--- a/tests/ImageFilterCacheTest.cpp
+++ b/tests/ImageFilterCacheTest.cpp
@@ -47,6 +47,7 @@
 #endif
 
 #include <cstddef>
+#include <cstdint>
 #include <tuple>
 #include <utility>
 
@@ -109,10 +110,12 @@
     SkIRect clip2 = SkIRect::MakeWH(200, 200);
     SkImageFilterCacheKey key0(0, SkMatrix::I(), clip1, image->uniqueID(), image->subset());
     SkImageFilterCacheKey key1(1, SkMatrix::I(), clip1, image->uniqueID(), image->subset());
-    SkImageFilterCacheKey key2(0, SkMatrix::Translate(5, 5), clip1,
-                                   image->uniqueID(), image->subset());
-    SkImageFilterCacheKey key3(0, SkMatrix::I(), clip2, image->uniqueID(), image->subset());
-    SkImageFilterCacheKey key4(0, SkMatrix::I(), clip1, subset->uniqueID(), subset->subset());
+    SkImageFilterCacheKey key2(1ULL << 32, SkMatrix::I(), clip1,
+                               image->uniqueID(), image->subset());
+    SkImageFilterCacheKey key3(0, SkMatrix::Translate(5, 5), clip1,
+                               image->uniqueID(), image->subset());
+    SkImageFilterCacheKey key4(0, SkMatrix::I(), clip2, image->uniqueID(), image->subset());
+    SkImageFilterCacheKey key5(0, SkMatrix::I(), clip1, subset->uniqueID(), subset->subset());
 
     SkIPoint offset = SkIPoint::Make(3, 4);
     auto filter = make_filter();
@@ -123,6 +126,7 @@
     REPORTER_ASSERT(reporter, !cache->get(key2, &foundImage));
     REPORTER_ASSERT(reporter, !cache->get(key3, &foundImage));
     REPORTER_ASSERT(reporter, !cache->get(key4, &foundImage));
+    REPORTER_ASSERT(reporter, !cache->get(key5, &foundImage));
 }
 
 // Test purging when the max cache size is exceeded
@@ -362,3 +366,45 @@
         REPORTER_ASSERT(r, cache);
     }
 }
+
+DEF_TEST(ImageFilterCache_64BitUniqueID, reporter) {
+    static const size_t kCacheSize = 1000000;
+    sk_sp<SkImageFilterCache> cache(SkImageFilterCache::Create(kCacheSize));
+
+    SkBitmap bm = create_bm();
+    const SkIRect full = SkIRect::MakeWH(kFullSize, kFullSize);
+    sk_sp<SkSpecialImage> img1(SkSpecialImages::MakeFromRaster(full, bm, SkSurfaceProps()));
+    sk_sp<SkSpecialImage> img2(SkSpecialImages::MakeFromRaster(full, bm, SkSurfaceProps()));
+
+    constexpr uint64_t kBaseId = 42;
+    constexpr uint64_t kUpperBitDiffId = kBaseId + (1ULL << 32);
+
+    SkIRect clip = SkIRect::MakeWH(100, 100);
+    SkImageFilterCacheKey key1(kBaseId, SkMatrix::I(), clip, img1->uniqueID(), img1->subset());
+    SkImageFilterCacheKey key2(
+            kUpperBitDiffId, SkMatrix::I(), clip, img1->uniqueID(), img1->subset());
+
+    REPORTER_ASSERT(reporter, key1 == key1);
+    REPORTER_ASSERT(reporter, key2 == key2);
+    REPORTER_ASSERT(reporter, !(key1 == key2));
+
+    auto filter1 = make_filter();
+    auto filter2 = make_filter();
+    SkIPoint offset1 = SkIPoint::Make(1, 2);
+    SkIPoint offset2 = SkIPoint::Make(5, 6);
+
+    cache->set(key1, filter1.get(), skif::FilterResult(img1, skif::LayerSpace<SkIPoint>(offset1)));
+
+    skif::FilterResult found;
+    REPORTER_ASSERT(reporter, cache->get(key1, &found));
+    REPORTER_ASSERT(reporter, found.image() == img1.get());
+    REPORTER_ASSERT(reporter, !cache->get(key2, &found));
+
+    cache->set(key2, filter2.get(), skif::FilterResult(img2, skif::LayerSpace<SkIPoint>(offset2)));
+    SkDEBUGCODE(REPORTER_ASSERT(reporter, 2 == cache->count());)
+
+    REPORTER_ASSERT(reporter, cache->get(key1, &found));
+    REPORTER_ASSERT(reporter, found.image() == img1.get());
+    REPORTER_ASSERT(reporter, cache->get(key2, &found));
+    REPORTER_ASSERT(reporter, found.image() == img2.get());
+}