Remove use of MakeRenderTargetDirect from view system
Here is the CL that sent me down the SkGammaColorFilter path
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2178353005
Review-Url: https://codereview.chromium.org/2178353005
diff --git a/example/HelloWorld.cpp b/example/HelloWorld.cpp
index 32bee5d..5239311 100644
--- a/example/HelloWorld.cpp
+++ b/example/HelloWorld.cpp
@@ -29,7 +29,6 @@
HelloWorldWindow::HelloWorldWindow(void* hwnd)
: INHERITED(hwnd) {
fType = kGPU_DeviceType;
- fRenderTarget = NULL;
fRotationAngle = 0;
this->setTitle();
this->setUpBackend();
@@ -46,8 +45,7 @@
SkSafeUnref(fInterface);
fInterface = NULL;
- SkSafeUnref(fRenderTarget);
- fRenderTarget = NULL;
+ fGpuSurface = nullptr;
INHERITED::release();
}
@@ -70,19 +68,17 @@
}
fInterface = GrGLCreateNativeInterface();
-
SkASSERT(NULL != fInterface);
fContext = GrContext::Create(kOpenGL_GrBackend, (GrBackendContext)fInterface);
SkASSERT(NULL != fContext);
- this->setUpRenderTarget();
+ this->setUpGpuBackedSurface();
return true;
}
-void HelloWorldWindow::setUpRenderTarget() {
- SkSafeUnref(fRenderTarget);
- fRenderTarget = this->renderTarget(fAttachmentInfo, fInterface, fContext);
+void HelloWorldWindow::setUpGpuBackedSurface() {
+ fGpuSurface = this->makeGpuBackedSurface(fAttachmentInfo, fInterface, fContext);
}
void HelloWorldWindow::drawContents(SkCanvas* canvas) {
@@ -142,7 +138,7 @@
}
void HelloWorldWindow::draw(SkCanvas* canvas) {
- drawContents(canvas);
+ this->drawContents(canvas);
// in case we have queued drawing calls
fContext->flush();
// Invalidate the window to force a redraw. Poor man's animation mechanism.
@@ -150,21 +146,22 @@
if (kRaster_DeviceType == fType) {
// need to send the raster bits to the (gpu) window
- sk_sp<SkImage> snap = fSurface->makeImageSnapshot();
+ sk_sp<SkImage> snap = fRasterSurface->makeImageSnapshot();
SkPixmap pmap;
if (snap->peekPixels(&pmap)) {
const SkImageInfo& info = pmap.info();
- fRenderTarget->writePixels(0, 0, snap->width(), snap->height(),
- SkImageInfo2GrPixelConfig(info, *fContext->caps()),
- pmap.addr(), pmap.rowBytes(),
- GrContext::kFlushWrites_PixelOp);
+
+ SkCanvas* canvas = fGpuSurface->getCanvas();
+
+ canvas->writePixels(info, pmap.addr(), pmap.rowBytes(), 0, 0);
+ canvas->flush();
}
}
INHERITED::present();
}
void HelloWorldWindow::onSizeChange() {
- setUpRenderTarget();
+ this->setUpGpuBackedSurface();
}
bool HelloWorldWindow::onHandleChar(SkUnichar unichar) {
diff --git a/example/HelloWorld.h b/example/HelloWorld.h
index deb56ba..d3fc7cf 100644
--- a/example/HelloWorld.h
+++ b/example/HelloWorld.h
@@ -33,14 +33,15 @@
DeviceType getDeviceType() const { return fType; }
protected:
- SkSurface* createSurface() override {
+ sk_sp<SkSurface> makeSurface() override {
SkSurfaceProps props(INHERITED::getSurfaceProps());
if (kGPU_DeviceType == fType) {
- return SkSurface::MakeRenderTargetDirect(fRenderTarget, nullptr, &props).release();
+ return fGpuSurface;
}
- static const SkImageInfo info = SkImageInfo::MakeN32Premul(
- SkScalarRoundToInt(this->width()), SkScalarRoundToInt(this->height()));
- return fSurface = SkSurface::MakeRaster(info, &props).release();
+ const SkImageInfo info = SkImageInfo::MakeN32Premul(SkScalarRoundToInt(this->width()),
+ SkScalarRoundToInt(this->height()));
+ fRasterSurface = SkSurface::MakeRaster(info, &props);
+ return fRasterSurface;
}
void draw(SkCanvas* canvas) override;
@@ -51,7 +52,7 @@
private:
bool findNextMatch(); // Set example to the first one that matches FLAGS_match.
void setTitle();
- void setUpRenderTarget();
+ void setUpGpuBackedSurface();
bool onHandleChar(SkUnichar unichar) override;
void tearDownBackend();
@@ -60,9 +61,9 @@
// support framework
DeviceType fType;
- SkSurface* fSurface;
+ sk_sp<SkSurface> fRasterSurface;
GrContext* fContext;
- GrRenderTarget* fRenderTarget;
+ sk_sp<SkSurface> fGpuSurface;
AttachmentInfo fAttachmentInfo;
const GrGLInterface* fInterface;
diff --git a/include/views/SkWindow.h b/include/views/SkWindow.h
index f81ec5d..e964fc6 100644
--- a/include/views/SkWindow.h
+++ b/include/views/SkWindow.h
@@ -78,7 +78,7 @@
void preConcat(const SkMatrix&);
void postConcat(const SkMatrix&);
- virtual SkSurface* createSurface();
+ virtual sk_sp<SkSurface> makeSurface();
protected:
virtual bool onEvent(const SkEvent&);
@@ -98,8 +98,8 @@
virtual bool onSetFocusView(SkView* focus);
#if SK_SUPPORT_GPU
- GrRenderTarget* renderTarget(const AttachmentInfo& attachmentInfo,
- const GrGLInterface* , GrContext* grContext);
+ sk_sp<SkSurface> makeGpuBackedSurface(const AttachmentInfo& attachmentInfo,
+ const GrGLInterface* , GrContext* grContext);
#endif
private:
diff --git a/samplecode/SampleApp.cpp b/samplecode/SampleApp.cpp
index 64d460f..0c9c3f0 100644
--- a/samplecode/SampleApp.cpp
+++ b/samplecode/SampleApp.cpp
@@ -36,6 +36,7 @@
#if SK_SUPPORT_GPU
# include "gl/GrGLInterface.h"
# include "gl/GrGLUtil.h"
+# include "GrDrawContext.h"
# include "GrRenderTarget.h"
# include "GrContext.h"
# include "SkGr.h"
@@ -187,7 +188,6 @@
#if SK_SUPPORT_GPU
fCurContext = nullptr;
fCurIntf = nullptr;
- fCurRenderTarget = nullptr;
fMSAASampleCount = 0;
fDeepColor = false;
fActualColorBits = 0;
@@ -199,7 +199,6 @@
#if SK_SUPPORT_GPU
SkSafeUnref(fCurContext);
SkSafeUnref(fCurIntf);
- SkSafeUnref(fCurRenderTarget);
#endif
}
@@ -272,7 +271,7 @@
win->release();
}
#endif // SK_SUPPORT_GPU
- // call windowSizeChanged to create the render target
+ // call windowSizeChanged to create the gpu-backed Surface
this->windowSizeChanged(win);
}
@@ -288,14 +287,13 @@
SkSafeUnref(fCurIntf);
fCurIntf = nullptr;
- SkSafeUnref(fCurRenderTarget);
- fCurRenderTarget = nullptr;
+ fGpuSurface = nullptr;
#endif
win->release();
fBackend = kNone_BackEndType;
}
- SkSurface* createSurface(SampleWindow::DeviceType dType, SampleWindow* win) override {
+ sk_sp<SkSurface> makeSurface(SampleWindow::DeviceType dType, SampleWindow* win) override {
#if SK_SUPPORT_GPU
if (IsGpuDeviceType(dType) && fCurContext) {
SkSurfaceProps props(win->getSurfaceProps());
@@ -306,11 +304,9 @@
// If we're using a deep (10-bit or higher) surface, we probably need an off-screen
// surface. 10-bit, in particular, has strange gamma behavior.
return SkSurface::MakeRenderTarget(fCurContext, SkBudgeted::kNo, win->info(),
- fMSAASampleCount, &props).release();
+ fMSAASampleCount, &props);
} else {
- return SkSurface::MakeRenderTargetDirect(fCurRenderTarget,
- sk_ref_sp(win->info().colorSpace()),
- &props).release();
+ return fGpuSurface;
}
}
#endif
@@ -336,14 +332,20 @@
bm.peekPixels(&pm);
sk_sp<SkImage> image(SkImage::MakeTextureFromPixmap(fCurContext, pm,
SkBudgeted::kNo));
- GrTexture* texture = as_IB(image)->peekTexture();
+
+ SkCanvas* canvas = fGpuSurface->getCanvas();
+
+ // Temporary code until applyGamma is replaced
+ GrDrawContext* dc = canvas->internal_private_accessTopLayerDrawContext();
+ GrRenderTarget* rt = dc->accessRenderTarget();
+ GrTexture* texture = image->getTexture();
SkASSERT(texture);
// With ten-bit output, we need to manually apply the gamma of the output device
// (unless we're in non-gamma correct mode, in which case our data is already
// fake-sRGB, like we're expected to put in the 10-bit buffer):
bool doGamma = (fActualColorBits == 30) && SkImageInfoIsGammaCorrect(win->info());
- fCurContext->applyGamma(fCurRenderTarget, texture, doGamma ? 1.0f / 2.2f : 1.0f);
+ fCurContext->applyGamma(rt, texture, doGamma ? 1.0f / 2.2f : 1.0f);
}
#endif
@@ -355,9 +357,8 @@
if (fCurContext) {
AttachmentInfo attachmentInfo;
win->attach(fBackend, fMSAASampleCount, fDeepColor, &attachmentInfo);
- SkSafeUnref(fCurRenderTarget);
fActualColorBits = SkTMax(attachmentInfo.fColorBits, 24);
- fCurRenderTarget = win->renderTarget(attachmentInfo, fCurIntf, fCurContext);
+ fGpuSurface = win->makeGpuBackedSurface(attachmentInfo, fCurIntf, fCurContext);
}
#endif
}
@@ -370,12 +371,8 @@
#endif
}
- GrRenderTarget* getGrRenderTarget() override {
-#if SK_SUPPORT_GPU
- return fCurRenderTarget;
-#else
- return nullptr;
-#endif
+ int numColorSamples() const override {
+ return fMSAASampleCount;
}
int getColorBits() override {
@@ -391,7 +388,7 @@
#if SK_SUPPORT_GPU
GrContext* fCurContext;
const GrGLInterface* fCurIntf;
- GrRenderTarget* fCurRenderTarget;
+ sk_sp<SkSurface> fGpuSurface;
int fMSAASampleCount;
bool fDeepColor;
int fActualColorBits;
@@ -2085,10 +2082,9 @@
#if SK_SUPPORT_GPU
if (IsGpuDeviceType(fDeviceType) &&
fDevManager &&
- fDevManager->getGrRenderTarget() &&
- fDevManager->getGrRenderTarget()->numColorSamples() > 0) {
+ fDevManager->numColorSamples() > 0) {
title.appendf(" [MSAA: %d]",
- fDevManager->getGrRenderTarget()->numColorSamples());
+ fDevManager->numColorSamples());
}
#endif
diff --git a/samplecode/SampleApp.h b/samplecode/SampleApp.h
index 8aea272..7541642 100644
--- a/samplecode/SampleApp.h
+++ b/samplecode/SampleApp.h
@@ -74,7 +74,7 @@
// called before drawing. should install correct device
// type on the canvas. Will skip drawing if returns false.
- virtual SkSurface* createSurface(DeviceType dType, SampleWindow* win) = 0;
+ virtual sk_sp<SkSurface> makeSurface(DeviceType dType, SampleWindow* win) = 0;
// called after drawing, should get the results onto the
// screen.
@@ -90,7 +90,7 @@
virtual GrContext* getGrContext() = 0;
// return the GrRenderTarget backing gpu devices (nullptr if not built with GPU support)
- virtual GrRenderTarget* getGrRenderTarget() = 0;
+ virtual int numColorSamples() const = 0;
// return the color depth of the output device
virtual int getColorBits() = 0;
@@ -102,13 +102,13 @@
SampleWindow(void* hwnd, int argc, char** argv, DeviceManager*);
virtual ~SampleWindow();
- SkSurface* createSurface() override {
- SkSurface* surface = nullptr;
+ sk_sp<SkSurface> makeSurface() override {
+ sk_sp<SkSurface> surface;
if (fDevManager) {
- surface = fDevManager->createSurface(fDeviceType, this);
+ surface = fDevManager->makeSurface(fDeviceType, this);
}
- if (nullptr == surface) {
- surface = this->INHERITED::createSurface();
+ if (!surface) {
+ surface = this->INHERITED::makeSurface();
}
return surface;
}
diff --git a/src/views/SkWindow.cpp b/src/views/SkWindow.cpp
index a1af704..e578b1a 100644
--- a/src/views/SkWindow.cpp
+++ b/src/views/SkWindow.cpp
@@ -30,10 +30,9 @@
fMenus.deleteAll();
}
-SkSurface* SkWindow::createSurface() {
+sk_sp<SkSurface> SkWindow::makeSurface() {
const SkBitmap& bm = this->getBitmap();
- return SkSurface::MakeRasterDirect(bm.info(), bm.getPixels(), bm.rowBytes(),
- &fSurfaceProps).release();
+ return SkSurface::MakeRasterDirect(bm.info(), bm.getPixels(), bm.rowBytes(), &fSurfaceProps);
}
void SkWindow::setMatrix(const SkMatrix& matrix) {
@@ -106,7 +105,7 @@
bool SkWindow::update(SkIRect* updateArea) {
if (!fDirtyRgn.isEmpty()) {
- SkAutoTUnref<SkSurface> surface(this->createSurface());
+ sk_sp<SkSurface> surface(this->makeSurface());
SkCanvas* canvas = surface->getCanvas();
canvas->clipRegion(fDirtyRgn);
@@ -322,11 +321,16 @@
#include "gl/GrGLUtil.h"
#include "SkGr.h"
-GrRenderTarget* SkWindow::renderTarget(const AttachmentInfo& attachmentInfo,
- const GrGLInterface* interface, GrContext* grContext) {
+sk_sp<SkSurface> SkWindow::makeGpuBackedSurface(const AttachmentInfo& attachmentInfo,
+ const GrGLInterface* interface,
+ GrContext* grContext) {
GrBackendRenderTargetDesc desc;
desc.fWidth = SkScalarRoundToInt(this->width());
desc.fHeight = SkScalarRoundToInt(this->height());
+ if (0 == desc.fWidth || 0 == desc.fHeight) {
+ return nullptr;
+ }
+
// TODO: Query the actual framebuffer for sRGB capable. However, to
// preserve old (fake-linear) behavior, we don't do this. Instead, rely
// on the flag (currently driven via 'C' mode in SampleApp).
@@ -347,7 +351,8 @@
GrGLint buffer;
GR_GL_GetIntegerv(interface, GR_GL_FRAMEBUFFER_BINDING, &buffer);
desc.fRenderTargetHandle = buffer;
- return grContext->textureProvider()->wrapBackendRenderTarget(desc);
+
+ return SkSurface::MakeFromBackendRenderTarget(grContext, desc, &fSurfaceProps);
}
#endif
diff --git a/src/views/mac/SkNSView.mm b/src/views/mac/SkNSView.mm
index 73b353f..ce29382 100644
--- a/src/views/mac/SkNSView.mm
+++ b/src/views/mac/SkNSView.mm
@@ -134,7 +134,7 @@
- (void)drawSkia {
fRedrawRequestPending = false;
if (fWind) {
- SkAutoTUnref<SkSurface> surface(fWind->createSurface());
+ sk_sp<SkSurface> surface(fWind->makeSurface());
fWind->draw(surface->getCanvas());
#ifdef FORCE_REDRAW
fWind->inval(NULL);
@@ -171,13 +171,13 @@
#include "SkKey.h"
enum {
- SK_MacReturnKey = 36,
- SK_MacDeleteKey = 51,
- SK_MacEndKey = 119,
- SK_MacLeftKey = 123,
- SK_MacRightKey = 124,
- SK_MacDownKey = 125,
- SK_MacUpKey = 126,
+ SK_MacReturnKey = 36,
+ SK_MacDeleteKey = 51,
+ SK_MacEndKey = 119,
+ SK_MacLeftKey = 123,
+ SK_MacRightKey = 124,
+ SK_MacDownKey = 125,
+ SK_MacUpKey = 126,
SK_Mac0Key = 0x52,
SK_Mac1Key = 0x53,
SK_Mac2Key = 0x54,
@@ -192,17 +192,17 @@
static SkKey raw2key(UInt32 raw)
{
- static const struct {
- UInt32 fRaw;
- SkKey fKey;
- } gKeys[] = {
- { SK_MacUpKey, kUp_SkKey },
- { SK_MacDownKey, kDown_SkKey },
- { SK_MacLeftKey, kLeft_SkKey },
- { SK_MacRightKey, kRight_SkKey },
- { SK_MacReturnKey, kOK_SkKey },
- { SK_MacDeleteKey, kBack_SkKey },
- { SK_MacEndKey, kEnd_SkKey },
+ static const struct {
+ UInt32 fRaw;
+ SkKey fKey;
+ } gKeys[] = {
+ { SK_MacUpKey, kUp_SkKey },
+ { SK_MacDownKey, kDown_SkKey },
+ { SK_MacLeftKey, kLeft_SkKey },
+ { SK_MacRightKey, kRight_SkKey },
+ { SK_MacReturnKey, kOK_SkKey },
+ { SK_MacDeleteKey, kBack_SkKey },
+ { SK_MacEndKey, kEnd_SkKey },
{ SK_Mac0Key, k0_SkKey },
{ SK_Mac1Key, k1_SkKey },
{ SK_Mac2Key, k2_SkKey },
@@ -213,12 +213,12 @@
{ SK_Mac7Key, k7_SkKey },
{ SK_Mac8Key, k8_SkKey },
{ SK_Mac9Key, k9_SkKey }
- };
+ };
- for (unsigned i = 0; i < SK_ARRAY_COUNT(gKeys); i++)
- if (gKeys[i].fRaw == raw)
- return gKeys[i].fKey;
- return kNONE_SkKey;
+ for (unsigned i = 0; i < SK_ARRAY_COUNT(gKeys); i++)
+ if (gKeys[i].fRaw == raw)
+ return gKeys[i].fKey;
+ return kNONE_SkKey;
}
- (void)keyDown:(NSEvent *)event {
diff --git a/tests/ApplyGammaTest.cpp b/tests/ApplyGammaTest.cpp
index 5872422..8db8fe0 100644
--- a/tests/ApplyGammaTest.cpp
+++ b/tests/ApplyGammaTest.cpp
@@ -10,9 +10,13 @@
#if SK_SUPPORT_GPU
#include "GrContext.h"
+#include "GrDrawContext.h"
#include "GrTexture.h"
#include "GrTextureProvider.h"
+#include "SkCanvas.h"
+#include "SkPixmap.h"
+#include "SkSurface.h"
#include "SkUtils.h"
// using anonymous namespace because these functions are used as template params.
@@ -78,75 +82,71 @@
baseDesc.fWidth = kW;
baseDesc.fHeight = kH;
+ const SkImageInfo ii = SkImageInfo::MakeN32Premul(kW, kH);
+
SkAutoTMalloc<uint32_t> srcPixels(kW * kH);
for (int i = 0; i < kW * kH; ++i) {
srcPixels.get()[i] = i;
}
- SkAutoTMalloc<uint32_t> dstPixels(kW * kH);
- for (int i = 0; i < kW * kH; ++i) {
- dstPixels.get()[i] = ~i;
- }
+ SkPixmap pm(ii, srcPixels.get(), kRowBytes);
SkAutoTMalloc<uint32_t> read(kW * kH);
// We allow more error on GPUs with lower precision shader variables.
float error = context->caps()->shaderCaps()->floatPrecisionVaries() ? 1.2f : 0.5f;
- for (auto sOrigin : { kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin }) {
- for (auto dOrigin : { kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin }) {
- for (auto sFlags : { kRenderTarget_GrSurfaceFlag, kNone_GrSurfaceFlags }) {
- for (auto gamma : { 1.0f, 1.0f / 1.8f, 1.0f / 2.2f }) {
- GrSurfaceDesc srcDesc = baseDesc;
- srcDesc.fOrigin = sOrigin;
- srcDesc.fFlags = sFlags;
- GrSurfaceDesc dstDesc = baseDesc;
- dstDesc.fOrigin = dOrigin;
- dstDesc.fFlags = kRenderTarget_GrSurfaceFlag;
+ for (auto dOrigin : { kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin }) {
+ for (auto gamma : { 1.0f, 1.0f / 1.8f, 1.0f / 2.2f }) {
+ sk_sp<SkImage> src(SkImage::MakeTextureFromPixmap(context, pm, SkBudgeted::kNo));
- SkAutoTUnref<GrTexture> src(
- context->textureProvider()->createTexture(srcDesc, SkBudgeted::kNo,
- srcPixels.get(),
- kRowBytes));
- SkAutoTUnref<GrTexture> dst(
- context->textureProvider()->createTexture(dstDesc, SkBudgeted::kNo,
- dstPixels.get(),
- kRowBytes));
- if (!src || !dst) {
- ERRORF(reporter, "Could not create surfaces for copy surface test.");
- continue;
- }
+ sk_sp<SkSurface> dst(SkSurface::MakeRenderTarget(context, SkBudgeted::kNo,
+ ii, 0, dOrigin, nullptr));
- bool result = context->applyGamma(dst->asRenderTarget(), src, gamma);
+ if (!src || !dst) {
+ ERRORF(reporter, "Could not create surfaces for copy surface test.");
+ continue;
+ }
- // To make the copied src rect correct we would apply any dst clipping
- // back to the src rect, but we don't use it again so don't bother.
- if (!result) {
- ERRORF(reporter, "Unexpected failure from applyGamma.");
- continue;
- }
+ SkCanvas* dstCanvas = dst->getCanvas();
- sk_memset32(read.get(), 0, kW * kH);
- if (!dst->readPixels(0, 0, kW, kH, baseDesc.fConfig, read.get(), kRowBytes)) {
- ERRORF(reporter, "Error calling readPixels");
- continue;
- }
+ dstCanvas->clear(SK_ColorRED);
+ dstCanvas->flush();
- bool abort = false;
- // Validate that pixels were copied/transformed correctly.
- for (int y = 0; y < kH && !abort; ++y) {
- for (int x = 0; x < kW && !abort; ++x) {
- uint32_t r = read.get()[y * kW + x];
- uint32_t s = srcPixels.get()[y * kW + x];
- uint32_t expected;
- if (!check_gamma(s, r, gamma, error, &expected)) {
- ERRORF(reporter, "Expected dst %d,%d to contain 0x%08x "
- "from src 0x%08x and gamma %f. Got %08x",
- x, y, expected, s, gamma, r);
- abort = true;
- break;
- }
- }
+ // Temporary code until applyGamma is replaced
+ GrDrawContext* dc = dstCanvas->internal_private_accessTopLayerDrawContext();
+ GrRenderTarget* rt = dc->accessRenderTarget();
+ GrTexture* texture = src->getTexture();
+ SkASSERT(texture);
+
+ bool result = context->applyGamma(rt, texture, gamma);
+
+ // To make the copied src rect correct we would apply any dst clipping
+ // back to the src rect, but we don't use it again so don't bother.
+ if (!result) {
+ ERRORF(reporter, "Unexpected failure from applyGamma.");
+ continue;
+ }
+
+ sk_memset32(read.get(), 0, kW * kH);
+ if (!dstCanvas->readPixels(ii, read.get(), kRowBytes, 0, 0)) {
+ ERRORF(reporter, "Error calling readPixels");
+ continue;
+ }
+
+ bool abort = false;
+ // Validate that pixels were copied/transformed correctly.
+ for (int y = 0; y < kH && !abort; ++y) {
+ for (int x = 0; x < kW && !abort; ++x) {
+ uint32_t r = read.get()[y * kW + x];
+ uint32_t s = srcPixels.get()[y * kW + x];
+ uint32_t expected;
+ if (!check_gamma(s, r, gamma, error, &expected)) {
+ ERRORF(reporter, "Expected dst %d,%d to contain 0x%08x "
+ "from src 0x%08x and gamma %f. Got %08x",
+ x, y, expected, s, gamma, r);
+ abort = true;
+ break;
}
}
}
