From 17b4bab488449ed4aaca0a406bc950dcc88fa357 Mon Sep 17 00:00:00 2001
From: Thomas Smith <[email protected]>
Date: Tue, 19 May 2026 15:33:06 -0400
Subject: [PATCH] [graphite] correctly advance index in drawEdgeAAImageSet

Bug: b/513836996
Change-Id: Iff633e0d99443cfc431352cea05051ff567b8f64
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1238179
Reviewed-by: Kaylee Lubick <[email protected]>
Reviewed-by: Michael Ludwig <[email protected]>
---

diff --git a/gn/tests.gni b/gn/tests.gni
index f4a4605..3430e2d 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -336,6 +336,7 @@
   "$_tests/graphite/UpdateBackendTextureTest.cpp",
   "$_tests/graphite/UploadBufferManagerTest.cpp",
   "$_tests/graphite/VerticesPaddingTest.cpp",
+  "$_tests/graphite/crbug_513836996.cpp",
 ]
 
 precompile_tests_sources = [
diff --git a/src/gpu/graphite/Device.cpp b/src/gpu/graphite/Device.cpp
index 218c763..c75b096 100644
--- a/src/gpu/graphite/Device.cpp
+++ b/src/gpu/graphite/Device.cpp
@@ -1309,32 +1309,31 @@
                 dstToDraw.setEmpty();
             }
         }
-        if (dstToDraw.isEmpty()) {
-            continue; // Nothing to draw for this set entry
+        if (!dstToDraw.isEmpty()) {
+            PaintParams::SimpleImage imageShader{
+                    set[i].fImage.get(),
+                    &localMatrix,
+                    constraint == SkCanvas::kStrict_SrcRectConstraint ? subset : imageBounds,
+                    sampling};
+
+            // NOTE: See drawEdgeAAQuad for details, we do not snap non-AA quads.
+            SkEnumBitMask<EdgeAAQuad::Flags> flags =
+                    static_cast<EdgeAAQuad::Flags>(set[i].fAAFlags);
+            EdgeAAQuad quad = set[i].fHasClip ? EdgeAAQuad(dstClips + dstClipIndex, flags)
+                                              : EdgeAAQuad(dstToDraw, flags);
+
+            // TODO: Calling drawGeometry() for each entry re-evaluates the clip stack every time,
+            // which is consistent with Ganesh's behavior. It also matches the behavior if edge-AA
+            // images were submitted one at a time by SkiaRenderer (a nice client simplification).
+            // However, we should explore the performance trade off with doing one bulk evaluation
+            // for the whole set
+            const SkMatrix* xtraXform =
+                    set[i].fMatrixIndex < 0 ? nullptr : &preViewMatrices[set[i].fMatrixIndex];
+            this->drawGeometry(xtraXform ? localToDevice.concat(SkM44(*xtraXform)) : localToDevice,
+                               Geometry(quad),
+                               PaintParams(paint, imageShader, set[i].fAlpha),
+                               DefaultFillStyle());
         }
-
-        PaintParams::SimpleImage imageShader{set[i].fImage.get(),
-                                             &localMatrix,
-                                             constraint == SkCanvas::kStrict_SrcRectConstraint ?
-                                                    subset : imageBounds,
-                                             sampling};
-
-        // NOTE: See drawEdgeAAQuad for details, we do not snap non-AA quads.
-        SkEnumBitMask<EdgeAAQuad::Flags> flags = static_cast<EdgeAAQuad::Flags>(set[i].fAAFlags);
-        EdgeAAQuad quad = set[i].fHasClip ? EdgeAAQuad(dstClips + dstClipIndex, flags)
-                                          : EdgeAAQuad(dstToDraw, flags);
-
-        // TODO: Calling drawGeometry() for each entry re-evaluates the clip stack every time, which
-        // is consistent with Ganesh's behavior. It also matches the behavior if edge-AA images were
-        // submitted one at a time by SkiaRenderer (a nice client simplification). However, we
-        // should explore the performance trade off with doing one bulk evaluation for the whole set
-        const SkMatrix* xtraXform = set[i].fMatrixIndex < 0 ? nullptr
-                                                            : &preViewMatrices[set[i].fMatrixIndex];
-        this->drawGeometry(xtraXform ?  localToDevice.concat(SkM44(*xtraXform)) : localToDevice,
-                           Geometry(quad),
-                           PaintParams(paint, imageShader, set[i].fAlpha),
-                           DefaultFillStyle());
-
         dstClipIndex += 4 * set[i].fHasClip;
     }
 }
diff --git a/tests/graphite/crbug_513836996.cpp b/tests/graphite/crbug_513836996.cpp
new file mode 100644
index 0000000..436468f
--- /dev/null
+++ b/tests/graphite/crbug_513836996.cpp
@@ -0,0 +1,84 @@
+/*
+ * Copyright 2026 Google LLC
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "include/core/SkBitmap.h"
+#include "include/core/SkCanvas.h"
+#include "include/core/SkColor.h"
+#include "include/core/SkImageInfo.h"
+#include "include/core/SkPoint.h"
+#include "include/core/SkRect.h"
+#include "include/core/SkRefCnt.h"
+#include "include/core/SkSamplingOptions.h"
+#include "include/core/SkSurface.h"
+#include "include/gpu/graphite/Context.h"
+#include "include/gpu/graphite/Image.h"
+#include "include/gpu/graphite/Recorder.h"
+#include "include/gpu/graphite/Recording.h"
+#include "include/gpu/graphite/Surface.h"
+#include "tests/Test.h"
+
+using namespace skgpu::graphite;
+
+DEF_GRAPHITE_TEST_FOR_ALL_CONTEXTS(crbug_513836996, reporter, context, CtsEnforcement::kNever) {
+    constexpr int kW = 200;
+    constexpr int kH = 100;
+    const SkImageInfo ii = SkImageInfo::Make(kW, kH, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+
+    std::unique_ptr<Recorder> recorder = context->makeRecorder();
+    sk_sp<SkSurface> surface = SkSurfaces::RenderTarget(recorder.get(), ii);
+    SkCanvas* canvas = surface->getCanvas();
+    canvas->clear(SK_ColorBLACK);
+
+    auto src_ii = SkImageInfo::Make(32, 32, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+    SkBitmap src_bm;
+    src_bm.allocPixels(src_ii);
+    src_bm.eraseColor(SK_ColorGREEN);
+
+    sk_sp<SkImage> rasterImage = SkImages::RasterFromBitmap(src_bm);
+    sk_sp<SkImage> green = SkImages::TextureFromImage(recorder.get(), rasterImage);
+
+    const SkPoint dstClips[8] = {
+        {10, 10}, {60, 10}, {60, 60}, {10, 60},     // LEFT
+        {110, 10}, {190, 10}, {190, 90}, {110, 90}, // RIGHT
+    };
+
+    SkCanvas::ImageSetEntry entries[2];
+    entries[0].fImage = green;
+    entries[0].fSrcRect = SkRect::MakeLTRB(-100, -100, -50, -50);
+    entries[0].fDstRect = SkRect::MakeWH(kW, kH);
+    entries[0].fMatrixIndex = -1;
+    entries[0].fAlpha = 1.f;
+    entries[0].fAAFlags = SkCanvas::kNone_QuadAAFlags;
+    entries[0].fHasClip = true;
+
+    entries[1].fImage = green;
+    entries[1].fSrcRect = SkRect::MakeWH(32, 32);
+    entries[1].fDstRect = SkRect::MakeWH(kW, kH);
+    entries[1].fMatrixIndex = -1;
+    entries[1].fAlpha = 1.f;
+    entries[1].fAAFlags = SkCanvas::kNone_QuadAAFlags;
+    entries[1].fHasClip = true;
+
+    canvas->experimental_DrawEdgeAAImageSet(
+        entries, 2, dstClips, nullptr, SkSamplingOptions(),
+        nullptr, SkCanvas::kFast_SrcRectConstraint);
+
+    std::unique_ptr<Recording> recording = recorder->snap();
+    context->insertRecording({recording.get()});
+
+    SkBitmap bm;
+    bm.allocPixels(ii);
+    REPORTER_ASSERT(reporter, surface->readPixels(bm, 0, 0), "Failed to read pixels");
+
+    SkColor left_px  = bm.getColor(35, 35);
+    SkColor right_px = bm.getColor(150, 50);
+
+    REPORTER_ASSERT(reporter, left_px == SK_ColorBLACK,
+        "Left pixel should be BLACK, got 0x%08x", left_px);
+    REPORTER_ASSERT(reporter, right_px == SK_ColorGREEN,
+        "Right pixel should be GREEN, got 0x%08x", right_px);
+}