Revert "Reland "Use SkImage_Raster's unique ID to cache textures.""

This reverts commit 814c6db4c0.

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 8005007e98
>
> 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>
This commit is contained in:
Brian Salomon 2021-05-21 14:57:03 +00:00 committed by Skia Commit-Bot
parent 465819d7c2
commit 54593e7424
7 changed files with 20 additions and 62 deletions

View File

@ -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 @@ bool SkPictureImageGenerator::onGetPixels(const SkImageInfo& info, void* pixels,
#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,

View File

@ -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

View File

@ -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"

View File

@ -163,14 +163,10 @@ static sk_sp<GrTextureProxy> make_bmp_proxy(GrProxyProvider* proxyProvider,
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 @@ make_cached_bitmap_view(GrRecordingContext* rContext,
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);
};
@ -238,22 +229,6 @@ make_cached_bitmap_view(GrRecordingContext* rContext,
return {{std::move(mippedProxy), kTopLeft_GrSurfaceOrigin, swizzle}, ct};
}
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,

View File

@ -199,19 +199,7 @@ GrSurfaceProxyView GrCopyBaseMipMapToView(GrRecordingContext*,
* 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 @@ GrMakeCachedBitmapProxyView(GrRecordingContext*,
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*,

View File

@ -212,7 +212,6 @@ bool SkImage_Raster::onPinAsTexture(GrRecordingContext* rContext) const {
} 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 @@ std::tuple<GrSurfaceProxyView, GrColorType> SkImage_Raster::onAsView(
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

View File

@ -51,9 +51,7 @@ static void basic_test(skiatest::Reporter* reporter, GrRecordingContext* rContex
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 @@ static void basic_test(skiatest::Reporter* reporter, GrRecordingContext* rContex
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));
}
}