From 31f25a037d7e3ee9de8f26e8cdd5c1f88da0f765 Mon Sep 17 00:00:00 2001 From: jvanverth Date: Wed, 4 May 2016 10:08:12 -0700 Subject: [PATCH] =?utf8?q?Revert=20of=20Allow=20stencils=20to=20be=20attac?= =?utf8?q?hed=20to=20render=20targets=20created=20via=20SkSurface::MakeFro?= =?utf8?q?mBackendTextureAsRend=E2=80=A6=20(patchset=20#1=20id:60001=20of?= =?utf8?q?=20https://codereview.chromium.org/1941353003/=20)?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Reason for revert: Breaking the ANGLE bots. Message is: Caught exception 3221225477 EXCEPTION_ACCESS_VIOLATION 8.59m elapsed, 1 active, 82 queued, 315MB RAM, 1442MB peak unit test SurfaceAttachStencil_Gpu step returned non-zero exit code: -1073741819 Original issue's description: > Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget > > This is a regression from "Refactor to separate backend object lifecycle > and GpuResource budget decision". > > GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false > for all wrapped render targets. This function should return false only if > the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach > stencils. > > BUG=608238 > > GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1924183003 > > Committed: https://skia.googlesource.com/skia/+/0736f3386820f19c0fe90b5dda2094e253780071 TBR=bsalomon@google.com,kkinnunen@nvidia.com,ericrk@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=608238 Review-Url: https://codereview.chromium.org/1947143002 --- src/gpu/gl/GrGLRenderTarget.cpp | 6 +- tests/SurfaceTest.cpp | 238 +++++++++++++++------------------------- 2 files changed, 94 insertions(+), 150 deletions(-) diff --git a/src/gpu/gl/GrGLRenderTarget.cpp b/src/gpu/gl/GrGLRenderTarget.cpp index 026c46b..3c27535 100644 --- a/src/gpu/gl/GrGLRenderTarget.cpp +++ b/src/gpu/gl/GrGLRenderTarget.cpp @@ -159,8 +159,10 @@ GrGLGpu* GrGLRenderTarget::getGLGpu() const { } bool GrGLRenderTarget::canAttemptStencilAttachment() const { - // Only modify the FBO's attachments if we have created the FBO. - return this->fRTFBOOwnership == GrBackendObjectOwnership::kOwned; + // When we have not created the FBO ID we do not attempt to modify its attachments. + // Direct GrGLRenderTarget instances are always created with CreateWrapped. + SkASSERT(this->resourcePriv().refsWrappedObjects()); + return false; } void GrGLRenderTarget::dumpMemoryStatistics(SkTraceMemoryDump* traceMemoryDump) const { diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp index 2497b52..878aded 100644 --- a/tests/SurfaceTest.cpp +++ b/tests/SurfaceTest.cpp @@ -20,7 +20,6 @@ #include "GrContext.h" #include "GrDrawContext.h" #include "GrGpu.h" -#include "GrResourceProvider.h" #endif #include @@ -84,6 +83,75 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SurfaceEmpty_Gpu, reporter, ctxInfo) { } #endif +#if SK_SUPPORT_GPU +DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SurfaceWrappedTexture, reporter, ctxInfo) { + GrGpu* gpu = ctxInfo.fGrContext->getGpu(); + if (!gpu) { + return; + } + + // Test the wrapped factory for SkSurface by creating a backend texture and then wrap it in + // a SkSurface. + static const int kW = 100; + static const int kH = 100; + static const uint32_t kOrigColor = 0xFFAABBCC; + SkAutoTArray pixels(kW * kH); + sk_memset32(pixels.get(), kOrigColor, kW * kH); + GrBackendObject texHandle = gpu->createTestingOnlyBackendTexture(pixels.get(), kW, kH, + kRGBA_8888_GrPixelConfig); + + GrBackendTextureDesc wrappedDesc; + wrappedDesc.fConfig = kRGBA_8888_GrPixelConfig; + wrappedDesc.fWidth = kW; + wrappedDesc.fHeight = kH; + wrappedDesc.fOrigin = kBottomLeft_GrSurfaceOrigin; + wrappedDesc.fSampleCnt = 0; + wrappedDesc.fFlags = kRenderTarget_GrBackendTextureFlag; + wrappedDesc.fTextureHandle = texHandle; + + auto surface(SkSurface::MakeFromBackendTexture(ctxInfo.fGrContext, wrappedDesc, nullptr)); + REPORTER_ASSERT(reporter, surface); + if (surface) { + // Validate that we can draw to the canvas and that the original texture color is preserved + // in pixels that aren't rendered to via the surface. + SkPaint paint; + static const SkColor kRectColor = ~kOrigColor | 0xFF000000; + paint.setColor(kRectColor); + surface->getCanvas()->drawRect(SkRect::MakeWH(SkIntToScalar(kW), SkIntToScalar(kH)/2), + paint); + SkImageInfo readInfo = SkImageInfo::MakeN32Premul(kW, kH); + surface->readPixels(readInfo, pixels.get(), kW * sizeof(uint32_t), 0, 0); + bool stop = false; + SkPMColor origColorPM = SkPackARGB32((kOrigColor >> 24 & 0xFF), + (kOrigColor >> 0 & 0xFF), + (kOrigColor >> 8 & 0xFF), + (kOrigColor >> 16 & 0xFF)); + SkPMColor rectColorPM = SkPackARGB32((kRectColor >> 24 & 0xFF), + (kRectColor >> 16 & 0xFF), + (kRectColor >> 8 & 0xFF), + (kRectColor >> 0 & 0xFF)); + for (int y = 0; y < kH/2 && !stop; ++y) { + for (int x = 0; x < kW && !stop; ++x) { + REPORTER_ASSERT(reporter, rectColorPM == pixels[x + y * kW]); + if (rectColorPM != pixels[x + y * kW]) { + stop = true; + } + } + } + stop = false; + for (int y = kH/2; y < kH && !stop; ++y) { + for (int x = 0; x < kW && !stop; ++x) { + REPORTER_ASSERT(reporter, origColorPM == pixels[x + y * kW]); + if (origColorPM != pixels[x + y * kW]) { + stop = true; + } + } + } + } + gpu->deleteTestingOnlyBackendTexture(texHandle); +} +#endif + static void test_canvas_peek(skiatest::Reporter* reporter, sk_sp& surface, const SkImageInfo& requestInfo, @@ -752,56 +820,10 @@ DEF_TEST(surface_rowbytes, reporter) { } #if SK_SUPPORT_GPU -static sk_sp create_gpu_surface_backend_texture( - GrContext* context, int sampleCnt, uint32_t color, GrBackendObject* outTexture) { - const int kWidth = 10; - const int kHeight = 10; - SkAutoTDeleteArray pixels(new uint32_t[kWidth * kHeight]); - sk_memset32(pixels.get(), color, kWidth * kHeight); - GrBackendTextureDesc desc; - desc.fConfig = kRGBA_8888_GrPixelConfig; - desc.fWidth = kWidth; - desc.fHeight = kHeight; - desc.fFlags = kRenderTarget_GrBackendTextureFlag; - desc.fTextureHandle = context->getGpu()->createTestingOnlyBackendTexture( - pixels.get(), kWidth, kHeight, kRGBA_8888_GrPixelConfig); - desc.fSampleCnt = sampleCnt; - sk_sp surface = SkSurface::MakeFromBackendTexture(context, desc, nullptr); - if (!surface) { - context->getGpu()->deleteTestingOnlyBackendTexture(desc.fTextureHandle); - return nullptr; - } - *outTexture = desc.fTextureHandle; - return surface; -} - -static sk_sp create_gpu_surface_backend_texture_as_render_target( - GrContext* context, int sampleCnt, uint32_t color, GrBackendObject* outTexture) { - const int kWidth = 10; - const int kHeight = 10; - SkAutoTDeleteArray pixels(new uint32_t[kWidth * kHeight]); - sk_memset32(pixels.get(), color, kWidth * kHeight); - GrBackendTextureDesc desc; - desc.fConfig = kRGBA_8888_GrPixelConfig; - desc.fWidth = kWidth; - desc.fHeight = kHeight; - desc.fFlags = kRenderTarget_GrBackendTextureFlag; - desc.fTextureHandle = context->getGpu()->createTestingOnlyBackendTexture( - pixels.get(), kWidth, kHeight, kRGBA_8888_GrPixelConfig); - desc.fSampleCnt = sampleCnt; - sk_sp surface = SkSurface::MakeFromBackendTextureAsRenderTarget(context, desc, - nullptr); - if (!surface) { - context->getGpu()->deleteTestingOnlyBackendTexture(desc.fTextureHandle); - return nullptr; - } - *outTexture = desc.fTextureHandle; - return surface; -} -static void test_surface_clear(skiatest::Reporter* reporter, sk_sp surface, - std::function grSurfaceGetter, - uint32_t expectedValue) { +void test_surface_clear(skiatest::Reporter* reporter, sk_sp surface, + std::function grSurfaceGetter, + uint32_t expectedValue) { if (!surface) { ERRORF(reporter, "Could not create GPU SkSurface."); return; @@ -809,7 +831,7 @@ static void test_surface_clear(skiatest::Reporter* reporter, sk_sp su int w = surface->width(); int h = surface->height(); SkAutoTDeleteArray pixels(new uint32_t[w * h]); - sk_memset32(pixels.get(), ~expectedValue, w * h); + memset(pixels.get(), ~expectedValue, sizeof(uint32_t) * w * h); SkAutoTUnref grSurface(SkSafeRef(grSurfaceGetter(surface.get()))); if (!grSurface) { @@ -839,7 +861,6 @@ static void test_surface_clear(skiatest::Reporter* reporter, sk_sp su DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SurfaceClear_Gpu, reporter, ctxInfo) { GrContext* context = ctxInfo.fGrContext; - std::function grSurfaceGetters[] = { [] (SkSurface* s){ GrDrawContext* dc = s->getCanvas()->internal_private_accessTopLayerDrawContext(); @@ -849,112 +870,33 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SurfaceClear_Gpu, reporter, ctxInfo) { s->getCanvas()->getDevice_just_for_deprecated_compatibility_testing(); return d->accessRenderTarget(); }, [] (SkSurface* s){ sk_sp i(s->makeImageSnapshot()); - return as_IB(i)->peekTexture(); } + return as_IB(i)->peekTexture(); }, }; - for (auto grSurfaceGetter : grSurfaceGetters) { - // Test that non-wrapped RTs are created clear. for (auto& surface_func : {&create_gpu_surface, &create_gpu_scratch_surface}) { auto surface = surface_func(context, kPremul_SkAlphaType, nullptr); test_surface_clear(reporter, surface, grSurfaceGetter, 0x0); } // Wrapped RTs are *not* supposed to clear (to allow client to partially update a surface). - const uint32_t kOrigColor = 0xABABABAB; - for (auto& surfaceFunc : {&create_gpu_surface_backend_texture, - &create_gpu_surface_backend_texture_as_render_target}) { - GrBackendObject textureObject; - auto surface = surfaceFunc(context, 0, kOrigColor, &textureObject); - test_surface_clear(reporter, surface, grSurfaceGetter, kOrigColor); - surface.reset(); - context->getGpu()->deleteTestingOnlyBackendTexture(textureObject); - } - } -} + static const int kWidth = 10; + static const int kHeight = 10; + SkAutoTDeleteArray pixels(new uint32_t[kWidth * kHeight]); + memset(pixels.get(), 0xAB, sizeof(uint32_t) * kWidth * kHeight); -static void test_surface_draw_partially( - skiatest::Reporter* reporter, sk_sp surface, uint32_t origColor) { - const int kW = surface->width(); - const int kH = surface->height(); - SkPaint paint; - const SkColor kRectColor = ~origColor | 0xFF000000; - paint.setColor(kRectColor); - surface->getCanvas()->drawRect(SkRect::MakeWH(SkIntToScalar(kW), SkIntToScalar(kH)/2), - paint); - SkAutoTDeleteArray pixels(new uint32_t[kW * kH]); - sk_memset32(pixels.get(), ~origColor, kW * kH); - SkImageInfo readInfo = SkImageInfo::MakeN32Premul(kW, kH); - SkAssertResult(surface->readPixels(readInfo, pixels.get(), kW * sizeof(uint32_t), 0, 0)); - bool stop = false; - SkPMColor origColorPM = SkPackARGB32((origColor >> 24 & 0xFF), - (origColor >> 0 & 0xFF), - (origColor >> 8 & 0xFF), - (origColor >> 16 & 0xFF)); - SkPMColor rectColorPM = SkPackARGB32((kRectColor >> 24 & 0xFF), - (kRectColor >> 16 & 0xFF), - (kRectColor >> 8 & 0xFF), - (kRectColor >> 0 & 0xFF)); - for (int y = 0; y < kH/2 && !stop; ++y) { - for (int x = 0; x < kW && !stop; ++x) { - REPORTER_ASSERT(reporter, rectColorPM == pixels[x + y * kW]); - if (rectColorPM != pixels[x + y * kW]) { - stop = true; - } - } - } - stop = false; - for (int y = kH/2; y < kH && !stop; ++y) { - for (int x = 0; x < kW && !stop; ++x) { - REPORTER_ASSERT(reporter, origColorPM == pixels[x + y * kW]); - if (origColorPM != pixels[x + y * kW]) { - stop = true; - } - } - } -} - -DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SurfacePartialDraw_Gpu, reporter, ctxInfo) { - GrGpu* gpu = ctxInfo.fGrContext->getGpu(); - if (!gpu) { - return; - } - static const uint32_t kOrigColor = 0xFFAABBCC; - - for (auto& surfaceFunc : {&create_gpu_surface_backend_texture, - &create_gpu_surface_backend_texture_as_render_target}) { - // Validate that we can draw to the canvas and that the original texture color is - // preserved in pixels that aren't rendered to via the surface. - // This works only for non-multisampled case. - GrBackendObject textureObject; - auto surface = surfaceFunc(ctxInfo.fGrContext, 0, kOrigColor, &textureObject); - if (surface) { - test_surface_draw_partially(reporter, surface, kOrigColor); - surface.reset(); - gpu->deleteTestingOnlyBackendTexture(textureObject); - } - } -} + GrBackendObject textureObject = + context->getGpu()->createTestingOnlyBackendTexture(pixels.get(), kWidth, kHeight, + kRGBA_8888_GrPixelConfig); + GrBackendTextureDesc desc; + desc.fConfig = kRGBA_8888_GrPixelConfig; + desc.fWidth = kWidth; + desc.fHeight = kHeight; + desc.fFlags = kRenderTarget_GrBackendTextureFlag; + desc.fTextureHandle = textureObject; -DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(SurfaceAttachStencil_Gpu, reporter, ctxInfo) { - GrGpu* gpu = ctxInfo.fGrContext->getGpu(); - if (!gpu) { - return; - } - static const uint32_t kOrigColor = 0xFFAABBCC; - - for (auto& surfaceFunc : {&create_gpu_surface_backend_texture, - &create_gpu_surface_backend_texture_as_render_target}) { - for (int sampleCnt : {0, 4, 8}) { - GrBackendObject textureObject; - auto surface = surfaceFunc(ctxInfo.fGrContext, sampleCnt, kOrigColor, &textureObject); - // Validate that we can attach a stencil buffer to an SkSurface created by either of - // our surface functions. - GrRenderTarget* rt = surface->getCanvas()->internal_private_accessTopLayerDrawContext() - ->accessRenderTarget(); - REPORTER_ASSERT(reporter, - ctxInfo.fGrContext->resourceProvider()->attachStencilAttachment(rt)); - gpu->deleteTestingOnlyBackendTexture(textureObject); - } + auto surface = SkSurface::MakeFromBackendTexture(context, desc, nullptr); + test_surface_clear(reporter, surface, grSurfaceGetter, 0xABABABAB); + context->getGpu()->deleteTestingOnlyBackendTexture(textureObject); } } #endif -- 2.7.4