Revert "Reland "Use SkImage_Raster's unique ID to cache textures.""
This reverts commit 814c6db4c04de0c757816e1d5b2aff4e1ccada54.
Reason for revert: Pending investigatoin of what's going on with Chrome issue 1211203.
Original change's description:
> Reland "Use SkImage_Raster's unique ID to cache textures."
>
> This is a reland of 8005007e9879f2778189e6505d70c3b2ce83452c
>
> Original change's description:
> > Use SkImage_Raster's unique ID to cache textures.
> >
> > SkImages can share SkBitmaps and have different unique IDs/mipmap
> > status. Currently we cache the texture version using the bitmap's
> > unique ID. Instead use the image's ID so different images produce
> > different textures (e.g. mipped and nonmipped).
> >
> > Bug: skia:11983
> > Change-Id: Ic37564186f675277e5a9de1bcf36b40a19c3a3de
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407356
> > Commit-Queue: Brian Salomon <bsalomon@google.com>
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
>
> Bug: skia:11983
> Change-Id: I63e9d15ffdf6b6769c9b0b97d9aa30f353e1a3a3
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/409376
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:11983
Change-Id: I4fb2875c2a62d409c3f6ddd41344e83f4e68a375
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/411240
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/src/core/SkPictureImageGenerator.cpp b/src/core/SkPictureImageGenerator.cpp
index 845971e..04cc519 100644
--- a/src/core/SkPictureImageGenerator.cpp
+++ b/src/core/SkPictureImageGenerator.cpp
@@ -12,6 +12,7 @@
#include "include/core/SkPicture.h"
#include "include/core/SkSurface.h"
#include "src/core/SkTLazy.h"
+#include "src/gpu/SkGr.h"
#include "src/image/SkImage_Base.h"
class SkPictureImageGenerator : public SkImageGenerator {
@@ -92,7 +93,6 @@
#if SK_SUPPORT_GPU
#include "include/gpu/GrRecordingContext.h"
#include "src/gpu/GrRecordingContextPriv.h"
-#include "src/gpu/SkGr.h"
GrSurfaceProxyView SkPictureImageGenerator::onGenerateTexture(GrRecordingContext* ctx,
const SkImageInfo& info,
diff --git a/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp b/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp
index c90f743..3c14293 100644
--- a/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp
+++ b/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp
@@ -16,11 +16,11 @@
#include "src/core/SkReadBuffer.h"
#include "src/core/SkSpecialImage.h"
#include "src/core/SkWriteBuffer.h"
+#include "src/gpu/SkGr.h"
#if SK_SUPPORT_GPU
#include "src/gpu/GrRecordingContextPriv.h"
#include "src/gpu/GrTextureProxy.h"
-#include "src/gpu/SkGr.h"
#include "src/gpu/effects/GrMatrixConvolutionEffect.h"
#endif
diff --git a/src/gpu/GrTextureProxy.h b/src/gpu/GrTextureProxy.h
index 37ac1b1..a0ac99a 100644
--- a/src/gpu/GrTextureProxy.h
+++ b/src/gpu/GrTextureProxy.h
@@ -8,7 +8,6 @@
#ifndef GrTextureProxy_DEFINED
#define GrTextureProxy_DEFINED
-#include "include/gpu/GrBackendSurface.h"
#include "src/gpu/GrSamplerState.h"
#include "src/gpu/GrSurfaceProxy.h"
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index 41a2ede..7a593bb 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -163,14 +163,10 @@
return proxy;
}
-static std::tuple<GrSurfaceProxyView, GrColorType>
-make_cached_bitmap_view(GrRecordingContext* rContext,
- const SkBitmap& bitmap,
- GrMipmapped mipmapped,
- uint32_t cacheID,
- bool addListenerToBitmap) {
- SkASSERT(cacheID != SK_InvalidUniqueID);
-
+std::tuple<GrSurfaceProxyView, GrColorType>
+GrMakeCachedBitmapProxyView(GrRecordingContext* rContext,
+ const SkBitmap& bitmap,
+ GrMipmapped mipmapped) {
if (!bitmap.peekPixels(nullptr)) {
return {};
}
@@ -181,19 +177,14 @@
GrUniqueKey key;
SkIPoint origin = bitmap.pixelRefOrigin();
SkIRect subset = SkIRect::MakePtSize(origin, bitmap.dimensions());
- GrMakeKeyFromImageID(&key, cacheID, subset);
+ GrMakeKeyFromImageID(&key, bitmap.pixelRef()->getGenerationID(), subset);
mipmapped = adjust_mipmapped(mipmapped, bitmap, caps);
GrColorType ct = choose_bmp_texture_colortype(caps, bitmap);
auto installKey = [&](GrTextureProxy* proxy) {
- if (cacheID == bitmap.getGenerationID()) {
- if (addListenerToBitmap) {
- auto listener = GrMakeUniqueKeyInvalidationListener(&key,
- proxyProvider->contextID());
- bitmap.pixelRef()->addGenIDChangeListener(std::move(listener));
- }
- }
+ auto listener = GrMakeUniqueKeyInvalidationListener(&key, proxyProvider->contextID());
+ bitmap.pixelRef()->addGenIDChangeListener(std::move(listener));
proxyProvider->assignUniqueKeyToProxy(key, proxy);
};
@@ -239,22 +230,6 @@
}
std::tuple<GrSurfaceProxyView, GrColorType>
-GrMakeCachedBitmapProxyViewWithID(GrRecordingContext* rContext,
- const SkBitmap& bitmap,
- GrMipmapped mipmapped,
- uint32_t cacheID) {
- return make_cached_bitmap_view(rContext, bitmap, mipmapped, cacheID, /*add listener*/ false);
-}
-
-std::tuple<GrSurfaceProxyView, GrColorType>
-GrMakeCachedBitmapProxyView(GrRecordingContext* rContext,
- const SkBitmap& bitmap,
- GrMipmapped mipmapped) {
- uint32_t cacheID = bitmap.getGenerationID();
- return make_cached_bitmap_view(rContext, bitmap, mipmapped, cacheID, /*add listener*/ true);
-}
-
-std::tuple<GrSurfaceProxyView, GrColorType>
GrMakeUncachedBitmapProxyView(GrRecordingContext* rContext,
const SkBitmap& bitmap,
GrMipmapped mipmapped,
diff --git a/src/gpu/SkGr.h b/src/gpu/SkGr.h
index d14cfca..8dba066 100644
--- a/src/gpu/SkGr.h
+++ b/src/gpu/SkGr.h
@@ -199,19 +199,7 @@
* if mipmapping isn't supported or for a 1x1 bitmap. If GrMipmapped is kNo it indicates mipmaps
* aren't required but a previously created mipmapped texture may still be returned. A color type is
* returned as color type conversion may be performed if there isn't a texture format equivalent of
- * the bitmap's color type. cacheID is used in the cache key and is from the shared namespace of
- * SkBitmap generation IDs and SkImage unique IDs. It must be valid.
- */
-std::tuple<GrSurfaceProxyView, GrColorType>
-GrMakeCachedBitmapProxyViewWithID(GrRecordingContext*,
- const SkBitmap&,
- GrMipmapped,
- uint32_t cacheID);
-
-/**
- * Like GrMakeCachedBitmapProxyViewWithID but uses the bitmap's generation ID as the cache ID and
- * also may add a listener to the SkBitmap that will invalidate the cached texture if the bitmap
- * is deleted or its contents change.
+ * the bitmap's color type.
*/
std::tuple<GrSurfaceProxyView, GrColorType>
GrMakeCachedBitmapProxyView(GrRecordingContext*,
@@ -219,8 +207,8 @@
GrMipmapped = GrMipmapped::kNo);
/**
- * Like GrMakeCachedBitmapProxyView but always uploads the bitmap and never inserts into the cache.
- * Unlike GrMakeCachedBitmapProxyView, the texture may be approx or scratch and budgeted or not.
+ * Like above but always uploads the bitmap and never inserts into the cache. Unlike above, the
+ * texture may be approx or scratch and budgeted or not.
*/
std::tuple<GrSurfaceProxyView, GrColorType>
GrMakeUncachedBitmapProxyView(GrRecordingContext*,
diff --git a/src/image/SkImage_Raster.cpp b/src/image/SkImage_Raster.cpp
index 08b2545..52fe5ee 100644
--- a/src/image/SkImage_Raster.cpp
+++ b/src/image/SkImage_Raster.cpp
@@ -212,7 +212,6 @@
} else {
SkASSERT(fPinnedCount == 0);
SkASSERT(fPinnedUniqueID == 0);
- // It's important that we use the bitmap's gen ID here and not the image's unique ID.
std::tie(fPinnedView, fPinnedColorType) = GrMakeCachedBitmapProxyView(rContext,
fBitmap,
GrMipmapped::kNo);
@@ -429,7 +428,7 @@
return {fPinnedView, fPinnedColorType};
}
if (policy == GrImageTexGenPolicy::kDraw) {
- return GrMakeCachedBitmapProxyViewWithID(rContext, fBitmap, mipmapped, this->uniqueID());
+ return GrMakeCachedBitmapProxyView(rContext, fBitmap, mipmapped);
}
auto budgeted = (policy == GrImageTexGenPolicy::kNew_Uncached_Unbudgeted)
? SkBudgeted::kNo
diff --git a/tests/PinnedImageTest.cpp b/tests/PinnedImageTest.cpp
index de9e788..d29fdd1 100644
--- a/tests/PinnedImageTest.cpp
+++ b/tests/PinnedImageTest.cpp
@@ -51,9 +51,7 @@
sk_sp<SkSurface> gpuSurface = SkSurface::MakeRenderTarget(rContext, SkBudgeted::kYes, ii);
SkCanvas* canvas = gpuSurface->getCanvas();
- // w/o pinning - the gpu caches the contents of the image and assumes the bitmap is immutable.
- // This is actually undefined but we're assuming the GPU backend will cache the original and
- // that it won't be purged before this test ends.
+ // w/o pinning - the gpu draw always reflects the current state of the underlying bitmap
{
canvas->drawImage(img, 0, 0);
REPORTER_ASSERT(reporter, surface_is_expected_color(gpuSurface.get(), ii, SK_ColorRED));
@@ -61,29 +59,28 @@
bmCanvas.clear(SK_ColorGREEN);
canvas->drawImage(img, 0, 0);
- REPORTER_ASSERT(reporter, surface_is_expected_color(gpuSurface.get(), ii, SK_ColorRED));
+ REPORTER_ASSERT(reporter, surface_is_expected_color(gpuSurface.get(), ii, SK_ColorGREEN));
}
// w/ pinning - the gpu draw is stuck at the pinned state
{
- bmCanvas.clear(SK_ColorBLUE);
SkImage_pinAsTexture(img.get(), rContext); // pin at blue
canvas->drawImage(img, 0, 0);
- REPORTER_ASSERT(reporter, surface_is_expected_color(gpuSurface.get(), ii, SK_ColorBLUE));
+ REPORTER_ASSERT(reporter, surface_is_expected_color(gpuSurface.get(), ii, SK_ColorGREEN));
- bmCanvas.clear(SK_ColorGREEN);
+ bmCanvas.clear(SK_ColorBLUE);
canvas->drawImage(img, 0, 0);
- REPORTER_ASSERT(reporter, surface_is_expected_color(gpuSurface.get(), ii, SK_ColorBLUE));
+ REPORTER_ASSERT(reporter, surface_is_expected_color(gpuSurface.get(), ii, SK_ColorGREEN));
SkImage_unpinAsTexture(img.get(), rContext);
}
- // once unpinned revert to original cached texture
+ // once unpinned local changes will be picked up
{
canvas->drawImage(img, 0, 0);
- REPORTER_ASSERT(reporter, surface_is_expected_color(gpuSurface.get(), ii, SK_ColorRED));
+ REPORTER_ASSERT(reporter, surface_is_expected_color(gpuSurface.get(), ii, SK_ColorBLUE));
}
}