AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Remove GPU read-back logic in SkSpecialImage

Browse Source 
All image filters are now implemented entirely on the GPU, so we never
need to read back the contents of a texture-backed special image.

Bug: skia:10202
Change-Id: I9e814d4bccde1e638f7bfc27b140e010ddcbcdb9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298138
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This commit is contained in:
Brian Osman 2020-06-22 14:51:22 -04:00 committed by Skia Commit-Bot
parent f105d38d10
commit e7ad8c0d3b
3 changed files with 35 additions and 98 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/core/SkSpecialImage.cpp
View File

@ -10,11 +10,9 @@
#include "include/core/SkBitmap.h"
#include "include/core/SkCanvas.h"
#include "include/core/SkImage.h"
#include "src/core/SkBitmapCache.h"
#include "src/core/SkSpecialSurface.h"
#include "src/core/SkSurfacePriv.h"
#include "src/image/SkImage_Base.h"
#include <atomic>
#if SK_SUPPORT_GPU
#include "include/gpu/GrContext.h"
@ -23,7 +21,6 @@
#include "src/gpu/GrImageInfo.h"
#include "src/gpu/GrProxyProvider.h"
#include "src/gpu/GrRecordingContextPriv.h"
#include "src/gpu/GrSurfaceContext.h"
#include "src/gpu/GrTextureProxy.h"
#include "src/image/SkImage_Gpu.h"
#endif
@ -98,10 +95,6 @@ sk_sp<SkSpecialImage> SkSpecialImage::makeTextureImage(GrRecordingContext* conte
}
SkBitmap bmp;
// At this point, we are definitely not texture-backed, so we must be raster or generator
// backed. If we remove the special-wrapping-an-image subclass, we may be able to assert that
// we are strictly raster-backed (i.e. generator images become raster when they are specialized)
// in which case getROPixels could turn into peekPixels...
if (!this->getROPixels(&bmp)) {
return nullptr;
}
@ -385,14 +378,7 @@ public:
, fView(std::move(view))
, fColorType(ct)
, fAlphaType(at)
, fColorSpace(std::move(colorSpace))
, fAddedRasterVersionToCache(false) {
}
~SkSpecialImage_Gpu() override {
if (fAddedRasterVersionToCache.load()) {
SkNotifyBitmapGenIDIsStale(this->uniqueID());
}
, fColorSpace(std::move(colorSpace)) {
}
SkAlphaType alphaType() const override { return fAlphaType; }
@ -426,35 +412,10 @@ public:
GrSurfaceProxyView onView(GrRecordingContext* context) const override { return fView; }
bool onGetROPixels(SkBitmap* dst) const override {
const auto desc = SkBitmapCacheDesc::Make(this->uniqueID(), this->subset());
if (SkBitmapCache::Find(desc, dst)) {
SkASSERT(dst->getGenerationID() == this->uniqueID());
SkASSERT(dst->isImmutable());
SkASSERT(dst->getPixels());
return true;
}
SkPixmap pmap;
SkImageInfo info = SkImageInfo::MakeN32(this->width(), this->height(),
this->alphaType(), fColorSpace);
auto rec = SkBitmapCache::Alloc(desc, info, &pmap);
if (!rec) {
return false;
}
auto sContext = GrSurfaceContext::Make(fContext, fView, fColorType, this->alphaType(),
fColorSpace);
if (!sContext) {
return false;
}
if (!sContext->readPixels(info, pmap.writable_addr(), pmap.rowBytes(),
{this->subset().left(), this->subset().top()})) {
return false;
}
SkBitmapCache::Add(std::move(rec), dst);
fAddedRasterVersionToCache.store(true);
return true;
// This should never be called: All GPU image filters are implemented entirely on the GPU,
// so we never perform read-back.
SkASSERT(false);
return false;
}
SkColorSpace* onGetColorSpace() const override {
@ -531,7 +492,6 @@ private:
const GrColorType fColorType;
const SkAlphaType fAlphaType;
sk_sp<SkColorSpace> fColorSpace;
mutable std::atomic<bool> fAddedRasterVersionToCache;
typedef SkSpecialImage_Base INHERITED;
};

33
tests/ImageFilterTest.cpp
View File

@ -491,6 +491,19 @@ static void test_cropRects(skiatest::Reporter* reporter, GrContext* context) {
}
}
static bool special_image_to_bitmap(const SkSpecialImage* src, SkBitmap* dst) {
sk_sp<SkImage> img = src->asImage();
if (!img) {
return false;
}
if (!dst->tryAllocN32Pixels(src->width(), src->height())) {
return false;
}
return img->readPixels(dst->pixmap(), src->subset().fLeft, src->subset().fTop);
}
static void test_negative_blur_sigma(skiatest::Reporter* reporter, GrContext* context) {
// Check that SkBlurImageFilter will accept a negative sigma, either in
// the given arguments or after CTM application.
@ -536,10 +549,10 @@ static void test_negative_blur_sigma(skiatest::Reporter* reporter, GrContext* co
SkBitmap positiveResultBM1, positiveResultBM2;
SkBitmap negativeResultBM1, negativeResultBM2;
REPORTER_ASSERT(reporter, positiveResult1->getROPixels(&positiveResultBM1));
REPORTER_ASSERT(reporter, positiveResult2->getROPixels(&positiveResultBM2));
REPORTER_ASSERT(reporter, negativeResult1->getROPixels(&negativeResultBM1));
REPORTER_ASSERT(reporter, negativeResult2->getROPixels(&negativeResultBM2));
REPORTER_ASSERT(reporter, special_image_to_bitmap(positiveResult1.get(), &positiveResultBM1));
REPORTER_ASSERT(reporter, special_image_to_bitmap(positiveResult2.get(), &positiveResultBM2));
REPORTER_ASSERT(reporter, special_image_to_bitmap(negativeResult1.get(), &negativeResultBM1));
REPORTER_ASSERT(reporter, special_image_to_bitmap(negativeResult2.get(), &negativeResultBM2));
for (int y = 0; y < kHeight; y++) {
int diffs = memcmp(positiveResultBM1.getAddr32(0, y),
@ -627,9 +640,9 @@ static void test_morphology_radius_with_mirror_ctm(skiatest::Reporter* reporter,
SkBitmap normalResultBM, mirrorXResultBM, mirrorYResultBM;
REPORTER_ASSERT(reporter, normalResult->getROPixels(&normalResultBM));
REPORTER_ASSERT(reporter, mirrorXResult->getROPixels(&mirrorXResultBM));
REPORTER_ASSERT(reporter, mirrorYResult->getROPixels(&mirrorYResultBM));
REPORTER_ASSERT(reporter, special_image_to_bitmap(normalResult.get(), &normalResultBM));
REPORTER_ASSERT(reporter, special_image_to_bitmap(mirrorXResult.get(), &mirrorXResultBM));
REPORTER_ASSERT(reporter, special_image_to_bitmap(mirrorYResult.get(), &mirrorYResultBM));
for (int y = 0; y < kHeight; y++) {
int diffs = memcmp(normalResultBM.getAddr32(0, y),
@ -678,7 +691,7 @@ static void test_zero_blur_sigma(skiatest::Reporter* reporter, GrContext* contex
SkBitmap resultBM;
REPORTER_ASSERT(reporter, result->getROPixels(&resultBM));
REPORTER_ASSERT(reporter, special_image_to_bitmap(result.get(), &resultBM));
for (int y = 0; y < resultBM.height(); y++) {
for (int x = 0; x < resultBM.width(); x++) {
@ -716,7 +729,7 @@ static void test_fail_affects_transparent_black(skiatest::Reporter* reporter, Gr
REPORTER_ASSERT(reporter, nullptr != result.get());
if (result.get()) {
SkBitmap resultBM;
REPORTER_ASSERT(reporter, result->getROPixels(&resultBM));
REPORTER_ASSERT(reporter, special_image_to_bitmap(result.get(), &resultBM));
REPORTER_ASSERT(reporter, *resultBM.getAddr32(0, 0) == SK_ColorGREEN);
}
}
@ -1492,7 +1505,7 @@ static void test_composed_imagefilter_bounds(skiatest::Reporter* reporter, GrCon
REPORTER_ASSERT(reporter, result->subset().size() == SkISize::Make(100, 100));
SkBitmap resultBM;
REPORTER_ASSERT(reporter, result->getROPixels(&resultBM));
REPORTER_ASSERT(reporter, special_image_to_bitmap(result.get(), &resultBM));
REPORTER_ASSERT(reporter, resultBM.getColor(50, 50) == SK_ColorGREEN);
}

50
tests/SpecialImageTest.cpp
View File

@ -78,11 +78,13 @@ static void test_image(const sk_sp<SkSpecialImage>& img, skiatest::Reporter* rep
}
//--------------
// Test getROPixels - this should always succeed regardless of backing store
SkBitmap bitmap;
REPORTER_ASSERT(reporter, img->getROPixels(&bitmap));
REPORTER_ASSERT(reporter, kSmallerSize == bitmap.width());
REPORTER_ASSERT(reporter, kSmallerSize == bitmap.height());
// Test getROPixels - this only works for raster-backed special images
if (!img->isTextureBacked()) {
SkBitmap bitmap;
REPORTER_ASSERT(reporter, img->getROPixels(&bitmap));
REPORTER_ASSERT(reporter, kSmallerSize == bitmap.width());
REPORTER_ASSERT(reporter, kSmallerSize == bitmap.height());
}
//--------------
// Test that draw restricts itself to the subset
@ -286,41 +288,3 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SpecialImage_Gpu, reporter, ctxInfo) {
test_image(subSImg2, reporter, context, true);
}
}
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SpecialImage_ReadbackAndCachingSubsets_Gpu, reporter, ctxInfo) {
GrContext* context = ctxInfo.grContext();
SkImageInfo ii = SkImageInfo::Make(50, 50, kN32_SkColorType, kPremul_SkAlphaType);
auto surface = SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, ii);
// Fill out our surface:
// Green | Blue
// Red | Green
{
surface->getCanvas()->clear(SK_ColorGREEN);
SkPaint p;
p.setColor(SK_ColorRED);
surface->getCanvas()->drawRect(SkRect::MakeXYWH(0, 25, 25, 25), p);
p.setColor(SK_ColorBLUE);
surface->getCanvas()->drawRect(SkRect::MakeXYWH(25, 0, 25, 25), p);
}
auto image = surface->makeImageSnapshot();
auto redImg = SkSpecialImage::MakeFromImage(context, SkIRect::MakeXYWH(10, 30, 10, 10), image);
auto blueImg = SkSpecialImage::MakeFromImage(context, SkIRect::MakeXYWH(30, 10, 10, 10), image);
// This isn't necessary, but if it ever becomes false, then the cache collision bug that we're
// checking below is irrelevant.
REPORTER_ASSERT(reporter, redImg->uniqueID() == blueImg->uniqueID());
SkBitmap redBM, blueBM;
SkAssertResult(redImg->getROPixels(&redBM));
SkAssertResult(blueImg->getROPixels(&blueBM));
// Each image should read from the correct sub-rect. Past bugs (skbug.com/8448) have included:
// - Always reading back from (0, 0), producing green
// - Incorrectly hitting the cache on the 2nd read-back, causing blueBM to be red
REPORTER_ASSERT(reporter, redBM.getColor(0, 0) == SK_ColorRED,
"0x%08x != 0x%08x", redBM.getColor(0, 0), SK_ColorRED);
REPORTER_ASSERT(reporter, blueBM.getColor(0, 0) == SK_ColorBLUE,
"0x%08x != 0x%08x", blueBM.getColor(0, 0), SK_ColorBLUE);
}