Handle invalid DDLRecorder case

Prior to this CL, if we failed to create the DDL Recorder's target
proxy while creating the target surface we could create an invalid
DDL.

The specific repro case involved too big of an target proxy but
many other scenarios could result in the same behavior.

Bug: 1105903
Change-Id: I519a072600c168aa590fbe920f4029d08fe29e6f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309777
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This commit is contained in:
Robert Phillips 2020-08-13 13:34:27 -04:00 committed by Skia Commit-Bot
parent 1d4b92d878
commit dc3697036e
4 changed files with 69 additions and 14 deletions

View File

@ -209,7 +209,7 @@ SkCanvas* SkDeferredDisplayListRecorder::getCanvas() {
}
sk_sp<SkDeferredDisplayList> SkDeferredDisplayListRecorder::detach() {
if (!fContext) {
if (!fContext || !fTargetProxy) {
return nullptr;
}

View File

@ -50,45 +50,50 @@ SkSurfaceCharacterization GrContextThreadSafeProxy::createCharacterization(
GrProtected isProtected) {
SkASSERT(fCaps);
if (!backendFormat.isValid()) {
return SkSurfaceCharacterization(); // return an invalid characterization
return {};
}
SkASSERT(isTextureable || !isMipMapped);
if (GrBackendApi::kOpenGL != backendFormat.backend() && willUseGLFBO0) {
// The willUseGLFBO0 flags can only be used for a GL backend.
return SkSurfaceCharacterization(); // return an invalid characterization
return {};
}
if (!fCaps->mipmapSupport()) {
isMipMapped = false;
}
if (ii.width() < 1 || ii.width() > fCaps->maxRenderTargetSize() ||
ii.height() < 1 || ii.height() > fCaps->maxRenderTargetSize()) {
return {};
}
GrColorType grColorType = SkColorTypeToGrColorType(ii.colorType());
if (!fCaps->areColorTypeAndFormatCompatible(grColorType, backendFormat)) {
return SkSurfaceCharacterization(); // return an invalid characterization
return {};
}
if (!fCaps->isFormatAsColorTypeRenderable(grColorType, backendFormat, sampleCnt)) {
return SkSurfaceCharacterization(); // return an invalid characterization
return {};
}
sampleCnt = fCaps->getRenderTargetSampleCount(sampleCnt, backendFormat);
SkASSERT(sampleCnt);
if (willUseGLFBO0 && isTextureable) {
return SkSurfaceCharacterization(); // return an invalid characterization
return {};
}
if (isTextureable && !fCaps->isFormatTexturable(backendFormat)) {
// Skia doesn't agree that this is textureable.
return SkSurfaceCharacterization(); // return an invalid characterization
return {};
}
if (GrBackendApi::kVulkan == backendFormat.backend()) {
if (GrBackendApi::kVulkan != fBackend) {
return SkSurfaceCharacterization(); // return an invalid characterization
return {};
}
#ifdef SK_VULKAN
@ -96,7 +101,7 @@ SkSurfaceCharacterization GrContextThreadSafeProxy::createCharacterization(
// The protection status of the characterization and the context need to match
if (isProtected != GrProtected(vkCaps->supportsProtectedMemory())) {
return SkSurfaceCharacterization(); // return an invalid characterization
return {};
}
#endif
}

View File

@ -935,11 +935,11 @@ static void dummy_done_proc(void*) {}
////////////////////////////////////////////////////////////////////////////////
// Test out the behavior of an invalid DDLRecorder
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLInvalidRecorder, reporter, ctxInfo) {
auto context = ctxInfo.directContext();
auto dContext = ctxInfo.directContext();
{
SkImageInfo ii = SkImageInfo::MakeN32Premul(32, 32);
sk_sp<SkSurface> s = SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, ii);
sk_sp<SkSurface> s = SkSurface::MakeRenderTarget(dContext, SkBudgeted::kNo, ii);
SkSurfaceCharacterization characterization;
SkAssertResult(s->characterize(&characterization));
@ -958,8 +958,8 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLInvalidRecorder, reporter, ctxInfo) {
REPORTER_ASSERT(reporter, !recorder.getCanvas());
REPORTER_ASSERT(reporter, !recorder.detach());
GrBackendFormat format = context->defaultBackendFormat(kRGBA_8888_SkColorType,
GrRenderable::kNo);
GrBackendFormat format = dContext->defaultBackendFormat(kRGBA_8888_SkColorType,
GrRenderable::kNo);
SkASSERT(format.isValid());
sk_sp<SkImage> image = recorder.makePromiseTexture(
@ -974,7 +974,57 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLInvalidRecorder, reporter, ctxInfo) {
SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew);
REPORTER_ASSERT(reporter, !image);
}
}
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(DDLCreateCharacterizationFailures, reporter, ctxInfo) {
auto dContext = ctxInfo.directContext();
size_t maxResourceBytes = dContext->getResourceCacheLimit();
auto proxy = dContext->threadSafeProxy().get();
auto check = [proxy, reporter, maxResourceBytes](const GrBackendFormat& backendFormat,
int width, int height,
SkColorType ct, bool willUseGLFBO0,
GrProtected prot) {
const SkSurfaceProps surfaceProps(0x0, kRGB_H_SkPixelGeometry);
SkImageInfo ii = SkImageInfo::Make(width, height, ct,
kPremul_SkAlphaType, nullptr);
SkSurfaceCharacterization c = proxy->createCharacterization(
maxResourceBytes, ii, backendFormat, 1,
kBottomLeft_GrSurfaceOrigin, surfaceProps, false,
willUseGLFBO0, true, prot);
REPORTER_ASSERT(reporter, !c.isValid());
};
GrBackendFormat goodBackendFormat = dContext->defaultBackendFormat(kRGBA_8888_SkColorType,
GrRenderable::kYes);
SkASSERT(goodBackendFormat.isValid());
GrBackendFormat badBackendFormat;
SkASSERT(!badBackendFormat.isValid());
SkColorType kGoodCT = kRGBA_8888_SkColorType;
SkColorType kBadCT = kUnknown_SkColorType;
static const bool kGoodUseFBO0 = false;
static const bool kBadUseFBO0 = true;
int goodWidth = 64;
int goodHeight = 64;
int badWidths[] = { 0, 1048576 };
int badHeights[] = { 0, 1048576 };
check(goodBackendFormat, goodWidth, badHeights[0], kGoodCT, kGoodUseFBO0, GrProtected::kNo);
check(goodBackendFormat, goodWidth, badHeights[1], kGoodCT, kGoodUseFBO0, GrProtected::kNo);
check(goodBackendFormat, badWidths[0], goodHeight, kGoodCT, kGoodUseFBO0, GrProtected::kNo);
check(goodBackendFormat, badWidths[1], goodHeight, kGoodCT, kGoodUseFBO0, GrProtected::kNo);
check(badBackendFormat, goodWidth, goodHeight, kGoodCT, kGoodUseFBO0, GrProtected::kNo);
check(goodBackendFormat, goodWidth, goodHeight, kBadCT, kGoodUseFBO0, GrProtected::kNo);
check(goodBackendFormat, goodWidth, goodHeight, kGoodCT, kBadUseFBO0, GrProtected::kNo);
if (dContext->backend() == GrBackendApi::kVulkan) {
check(goodBackendFormat, goodWidth, goodHeight, kGoodCT, kGoodUseFBO0, GrProtected::kYes);
}
}
////////////////////////////////////////////////////////////////////////////////

View File

@ -50,7 +50,7 @@ void DDLTileHelper::TileData::createTileSpecificSKP(SkData* compressedPictureDat
fReconstitutedPicture = helper.reinflateSKP(&recorder, compressedPictureData, &fPromiseImages);
auto ddl = recorder.detach();
if (ddl->priv().numRenderTasks()) {
if (ddl && ddl->priv().numRenderTasks()) {
// TODO: remove this once skbug.com/8424 is fixed. If the DDL resulting from the
// reinflation of the SKPs contains opsTasks that means some image subset operation
// created a draw.