Add a workaround for PowerVRRogue to never disable color writes
Instead we leave color writes enabled and use a blend state that
preserves the dst color. This allows us to re-enable msaa ccpr on
PowerVR.
Bug: skia:
Change-Id: I1e902d695ad483ffb13dff6a7920749e307b49c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/229387
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrPipeline.cpp b/src/gpu/GrPipeline.cpp
index cbdd4dc..98f1487 100644
--- a/src/gpu/GrPipeline.cpp
+++ b/src/gpu/GrPipeline.cpp
@@ -114,8 +114,7 @@
}
uint32_t GrPipeline::getBlendInfoKey() const {
- GrXferProcessor::BlendInfo blendInfo;
- this->getXferProcessor().getBlendInfo(&blendInfo);
+ const GrXferProcessor::BlendInfo& blendInfo = this->getXferProcessor().getBlendInfo();
static const uint32_t kBlendWriteShift = 1;
static const uint32_t kBlendCoeffShift = 5;
diff --git a/src/gpu/GrXferProcessor.cpp b/src/gpu/GrXferProcessor.cpp
index c378e1b..14d0355 100644
--- a/src/gpu/GrXferProcessor.cpp
+++ b/src/gpu/GrXferProcessor.cpp
@@ -30,15 +30,6 @@
return this->dstReadUsesMixedSamples();
}
-void GrXferProcessor::getBlendInfo(BlendInfo* blendInfo) const {
- blendInfo->reset();
- if (!this->willReadDstColor()) {
- this->onGetBlendInfo(blendInfo);
- } else if (this->dstReadUsesMixedSamples()) {
- blendInfo->fDstBlend = kIS2A_GrBlendCoeff;
- }
-}
-
void GrXferProcessor::getGLSLProcessorKey(const GrShaderCaps& caps, GrProcessorKeyBuilder* b,
const GrSurfaceOrigin* originIfDstTexture) const {
uint32_t key = this->willReadDstColor() ? 0x1 : 0x0;
diff --git a/src/gpu/GrXferProcessor.h b/src/gpu/GrXferProcessor.h
index b0fb1e7..aa53d57 100644
--- a/src/gpu/GrXferProcessor.h
+++ b/src/gpu/GrXferProcessor.h
@@ -123,24 +123,24 @@
}
struct BlendInfo {
- void reset() {
- fEquation = kAdd_GrBlendEquation;
- fSrcBlend = kOne_GrBlendCoeff;
- fDstBlend = kZero_GrBlendCoeff;
- fBlendConstant = SK_PMColor4fTRANSPARENT;
- fWriteColor = true;
- }
-
SkDEBUGCODE(SkString dump() const;)
- GrBlendEquation fEquation;
- GrBlendCoeff fSrcBlend;
- GrBlendCoeff fDstBlend;
- SkPMColor4f fBlendConstant;
- bool fWriteColor;
+ GrBlendEquation fEquation = kAdd_GrBlendEquation;
+ GrBlendCoeff fSrcBlend = kOne_GrBlendCoeff;
+ GrBlendCoeff fDstBlend = kZero_GrBlendCoeff;
+ SkPMColor4f fBlendConstant = SK_PMColor4fTRANSPARENT;
+ bool fWriteColor = true;
};
- void getBlendInfo(BlendInfo* blendInfo) const;
+ inline BlendInfo getBlendInfo() const {
+ BlendInfo blendInfo;
+ if (!this->willReadDstColor()) {
+ this->onGetBlendInfo(&blendInfo);
+ } else if (this->dstReadUsesMixedSamples()) {
+ blendInfo.fDstBlend = kIS2A_GrBlendCoeff;
+ }
+ return blendInfo;
+ }
bool willReadDstColor() const { return fWillReadDstColor; }
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index e76b3eb..fa604ae 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -57,6 +57,7 @@
fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines = false;
fDetachStencilFromMSAABuffersBeforeReadPixels = false;
fDontSetBaseOrMaxLevelForExternalTextures = false;
+ fNeverDisableColorWrites = false;
fProgramBinarySupport = false;
fProgramParameterSupport = false;
fSamplerObjectSupport = false;
@@ -3261,9 +3262,8 @@
// Temporarily disable the MSAA implementation of CCPR on various platforms while we work out
// specific issues.
- if (kATI_GrGLVendor == ctxInfo.vendor() || // Radeon hts an internal compiler error.
- kQualcomm_GrGLVendor == ctxInfo.vendor() || // Pixel2 crashes in nanobench.
- kImagination_GrGLVendor == ctxInfo.vendor() /* PowerVR doesn't draw curves */) {
+ if (kATI_GrGLVendor == ctxInfo.vendor() || // Radeon drops stencil draws that use sample mask.
+ kQualcomm_GrGLVendor == ctxInfo.vendor() /* Pixel2 crashes in nanobench. */) {
fDriverBlacklistMSAACCPR = true;
}
@@ -3274,6 +3274,12 @@
fDontSetBaseOrMaxLevelForExternalTextures = true;
#endif
+ // PowerVRGX6250 drops every pixel if we modify the sample mask while color writes are disabled.
+ if (kPowerVRRogue_GrGLRenderer == ctxInfo.renderer()) {
+ fNeverDisableColorWrites = true;
+ shaderCaps->fMustWriteToFragColor = true;
+ }
+
// It appears that Qualcomm drivers don't actually support
// GL_NV_shader_noperspective_interpolation in ES 3.00 or 3.10 shaders, only 3.20.
// https://crbug.com/986581
@@ -3336,6 +3342,8 @@
SkASSERT(!fUseDrawInsteadOfAllRenderTargetWrites);
SkASSERT(!fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines);
SkASSERT(!fDetachStencilFromMSAABuffersBeforeReadPixels);
+ SkASSERT(!fDontSetBaseOrMaxLevelForExternalTextures);
+ SkASSERT(!fNeverDisableColorWrites);
}
if (options.fDoManualMipmapping) {
fDoManualMipmapping = true;
diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index a84c269..1ab505d 100644
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -412,6 +412,9 @@
return fDontSetBaseOrMaxLevelForExternalTextures;
}
+ // PowerVRGX6250 drops every pixel if we modify the sample mask while color writes are disabled.
+ bool neverDisableColorWrites() const { return fNeverDisableColorWrites; }
+
// Returns the observed maximum number of instances the driver can handle in a single draw call
// without crashing, or 'pendingInstanceCount' if this workaround is not necessary.
// NOTE: the return value may be larger than pendingInstanceCount.
@@ -564,6 +567,7 @@
bool fRequiresCullFaceEnableDisableWhenDrawingLinesAfterNonLines : 1;
bool fDetachStencilFromMSAABuffersBeforeReadPixels : 1;
bool fDontSetBaseOrMaxLevelForExternalTextures : 1;
+ bool fNeverDisableColorWrites : 1;
int fMaxInstancesPerDrawWithoutCrashing;
uint32_t fBlitFramebufferFlags;
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 859adcc..6f82223 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -1948,15 +1948,11 @@
this->resolveAndGenerateMipMapsForProcessorTextures(
primProc, pipeline, primProcProxiesForMipRegen, numPrimProcTextureSets);
- GrXferProcessor::BlendInfo blendInfo;
- pipeline.getXferProcessor().getBlendInfo(&blendInfo);
-
- this->flushColorWrite(blendInfo.fWriteColor);
-
this->flushProgram(std::move(program));
// Swizzle the blend to match what the shader will output.
- this->flushBlend(blendInfo, pipeline.outputSwizzle());
+ this->flushBlendAndColorWrite(
+ pipeline.getXferProcessor().getBlendInfo(), pipeline.outputSwizzle());
fHWProgram->updateUniformsAndTextureBindings(renderTarget, origin,
primProc, pipeline, primProcProxiesToBind);
@@ -2719,17 +2715,29 @@
}
}
-void GrGLGpu::flushBlend(const GrXferProcessor::BlendInfo& blendInfo, const GrSwizzle& swizzle) {
- // Any optimization to disable blending should have already been applied and
- // tweaked the equation to "add" or "subtract", and the coeffs to (1, 0).
+void GrGLGpu::flushBlendAndColorWrite(
+ const GrXferProcessor::BlendInfo& blendInfo, const GrSwizzle& swizzle) {
+ if (this->glCaps().neverDisableColorWrites() && !blendInfo.fWriteColor) {
+ // We need to work around a driver bug by using a blend state that preserves the dst color,
+ // rather than disabling color writes.
+ GrXferProcessor::BlendInfo preserveDstBlend;
+ preserveDstBlend.fSrcBlend = kZero_GrBlendCoeff;
+ preserveDstBlend.fDstBlend = kOne_GrBlendCoeff;
+ this->flushBlendAndColorWrite(preserveDstBlend, swizzle);
+ return;
+ }
GrBlendEquation equation = blendInfo.fEquation;
GrBlendCoeff srcCoeff = blendInfo.fSrcBlend;
GrBlendCoeff dstCoeff = blendInfo.fDstBlend;
+
+ // Any optimization to disable blending should have already been applied and
+ // tweaked the equation to "add" or "subtract", and the coeffs to (1, 0).
bool blendOff =
((kAdd_GrBlendEquation == equation || kSubtract_GrBlendEquation == equation) &&
kOne_GrBlendCoeff == srcCoeff && kZero_GrBlendCoeff == dstCoeff) ||
!blendInfo.fWriteColor;
+
if (blendOff) {
if (kNo_TriState != fHWBlendState.fEnabled) {
GL_CALL(Disable(GR_GL_BLEND));
@@ -2747,41 +2755,42 @@
fHWBlendState.fEnabled = kNo_TriState;
}
- return;
- }
+ } else {
+ if (kYes_TriState != fHWBlendState.fEnabled) {
+ GL_CALL(Enable(GR_GL_BLEND));
- if (kYes_TriState != fHWBlendState.fEnabled) {
- GL_CALL(Enable(GR_GL_BLEND));
+ fHWBlendState.fEnabled = kYes_TriState;
+ }
- fHWBlendState.fEnabled = kYes_TriState;
- }
+ if (fHWBlendState.fEquation != equation) {
+ GL_CALL(BlendEquation(gXfermodeEquation2Blend[equation]));
+ fHWBlendState.fEquation = equation;
+ }
- if (fHWBlendState.fEquation != equation) {
- GL_CALL(BlendEquation(gXfermodeEquation2Blend[equation]));
- fHWBlendState.fEquation = equation;
- }
+ if (GrBlendEquationIsAdvanced(equation)) {
+ SkASSERT(this->caps()->advancedBlendEquationSupport());
+ // Advanced equations have no other blend state.
+ return;
+ }
- if (GrBlendEquationIsAdvanced(equation)) {
- SkASSERT(this->caps()->advancedBlendEquationSupport());
- // Advanced equations have no other blend state.
- return;
- }
+ if (fHWBlendState.fSrcCoeff != srcCoeff || fHWBlendState.fDstCoeff != dstCoeff) {
+ GL_CALL(BlendFunc(gXfermodeCoeff2Blend[srcCoeff],
+ gXfermodeCoeff2Blend[dstCoeff]));
+ fHWBlendState.fSrcCoeff = srcCoeff;
+ fHWBlendState.fDstCoeff = dstCoeff;
+ }
- if (fHWBlendState.fSrcCoeff != srcCoeff || fHWBlendState.fDstCoeff != dstCoeff) {
- GL_CALL(BlendFunc(gXfermodeCoeff2Blend[srcCoeff],
- gXfermodeCoeff2Blend[dstCoeff]));
- fHWBlendState.fSrcCoeff = srcCoeff;
- fHWBlendState.fDstCoeff = dstCoeff;
- }
-
- if ((BlendCoeffReferencesConstant(srcCoeff) || BlendCoeffReferencesConstant(dstCoeff))) {
- SkPMColor4f blendConst = swizzle.applyTo(blendInfo.fBlendConstant);
- if (!fHWBlendState.fConstColorValid || fHWBlendState.fConstColor != blendConst) {
- GL_CALL(BlendColor(blendConst.fR, blendConst.fG, blendConst.fB, blendConst.fA));
- fHWBlendState.fConstColor = blendConst;
- fHWBlendState.fConstColorValid = true;
+ if ((BlendCoeffReferencesConstant(srcCoeff) || BlendCoeffReferencesConstant(dstCoeff))) {
+ SkPMColor4f blendConst = swizzle.applyTo(blendInfo.fBlendConstant);
+ if (!fHWBlendState.fConstColorValid || fHWBlendState.fConstColor != blendConst) {
+ GL_CALL(BlendColor(blendConst.fR, blendConst.fG, blendConst.fB, blendConst.fA));
+ fHWBlendState.fConstColor = blendConst;
+ fHWBlendState.fConstColorValid = true;
+ }
}
}
+
+ this->flushColorWrite(blendInfo.fWriteColor);
}
static void get_gl_swizzle_values(const GrSwizzle& swizzle, GrGLenum glValues[4]) {
@@ -3514,10 +3523,7 @@
GL_CALL(Uniform4f(fCopyPrograms[progIdx].fTexCoordXformUniform,
sx1 - sx0, sy1 - sy0, sx0, sy0));
GL_CALL(Uniform1i(fCopyPrograms[progIdx].fTextureUniform, 0));
- GrXferProcessor::BlendInfo blendInfo;
- blendInfo.reset();
- this->flushBlend(blendInfo, GrSwizzle::RGBA());
- this->flushColorWrite(true);
+ this->flushBlendAndColorWrite(GrXferProcessor::BlendInfo(), GrSwizzle::RGBA());
this->flushHWAAState(nullptr, false);
this->disableScissor();
this->disableWindowRectangles();
@@ -3651,10 +3657,7 @@
kFloat2_GrSLType, 2 * sizeof(GrGLfloat), 0);
// Set "simple" state once:
- GrXferProcessor::BlendInfo blendInfo;
- blendInfo.reset();
- this->flushBlend(blendInfo, GrSwizzle::RGBA());
- this->flushColorWrite(true);
+ this->flushBlendAndColorWrite(GrXferProcessor::BlendInfo(), GrSwizzle::RGBA());
this->flushHWAAState(nullptr, false);
this->disableScissor();
this->disableWindowRectangles();
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index 44f08fe..e2ddf8d 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -289,7 +289,7 @@
int baseInstance,
GrPrimitiveRestart);
- void flushBlend(const GrXferProcessor::BlendInfo& blendInfo, const GrSwizzle&);
+ void flushBlendAndColorWrite(const GrXferProcessor::BlendInfo& blendInfo, const GrSwizzle&);
void onFinishFlush(GrSurfaceProxy*[], int n, SkSurface::BackendSurfaceAccess access,
const GrFlushInfo&, const GrPrepareForExternalIORequests&) override;
diff --git a/src/gpu/mtl/GrMtlPipelineState.mm b/src/gpu/mtl/GrMtlPipelineState.mm
index f7a9910..2edcbb8 100644
--- a/src/gpu/mtl/GrMtlPipelineState.mm
+++ b/src/gpu/mtl/GrMtlPipelineState.mm
@@ -176,8 +176,7 @@
return;
}
- GrXferProcessor::BlendInfo blendInfo;
- xferProcessor.getBlendInfo(&blendInfo);
+ const GrXferProcessor::BlendInfo& blendInfo = xferProcessor.getBlendInfo();
GrBlendCoeff srcCoeff = blendInfo.fSrcBlend;
GrBlendCoeff dstCoeff = blendInfo.fDstBlend;
if (blend_coeff_refs_constant(srcCoeff) || blend_coeff_refs_constant(dstCoeff)) {
diff --git a/src/gpu/mtl/GrMtlPipelineStateBuilder.mm b/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
index d3c5479..327d0de 100644
--- a/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
+++ b/src/gpu/mtl/GrMtlPipelineStateBuilder.mm
@@ -278,8 +278,7 @@
mtlColorAttachment.pixelFormat = format;
// blending
- GrXferProcessor::BlendInfo blendInfo;
- pipeline.getXferProcessor().getBlendInfo(&blendInfo);
+ const GrXferProcessor::BlendInfo& blendInfo = pipeline.getXferProcessor().getBlendInfo();
GrBlendEquation equation = blendInfo.fEquation;
GrBlendCoeff srcCoeff = blendInfo.fSrcBlend;
diff --git a/src/gpu/vk/GrVkPipeline.cpp b/src/gpu/vk/GrVkPipeline.cpp
index 3f70d37..04fefff 100644
--- a/src/gpu/vk/GrVkPipeline.cpp
+++ b/src/gpu/vk/GrVkPipeline.cpp
@@ -431,8 +431,7 @@
static void setup_color_blend_state(const GrPipeline& pipeline,
VkPipelineColorBlendStateCreateInfo* colorBlendInfo,
VkPipelineColorBlendAttachmentState* attachmentState) {
- GrXferProcessor::BlendInfo blendInfo;
- pipeline.getXferProcessor().getBlendInfo(&blendInfo);
+ const GrXferProcessor::BlendInfo& blendInfo = pipeline.getXferProcessor().getBlendInfo();
GrBlendEquation equation = blendInfo.fEquation;
GrBlendCoeff srcCoeff = blendInfo.fSrcBlend;
@@ -628,8 +627,7 @@
GrVkCommandBuffer* cmdBuffer,
const GrSwizzle& swizzle,
const GrXferProcessor& xferProcessor) {
- GrXferProcessor::BlendInfo blendInfo;
- xferProcessor.getBlendInfo(&blendInfo);
+ const GrXferProcessor::BlendInfo& blendInfo = xferProcessor.getBlendInfo();
GrBlendCoeff srcCoeff = blendInfo.fSrcBlend;
GrBlendCoeff dstCoeff = blendInfo.fDstBlend;
float floatColors[4];
diff --git a/tests/GrPorterDuffTest.cpp b/tests/GrPorterDuffTest.cpp
index 7e399f0..a01d0e5 100644
--- a/tests/GrPorterDuffTest.cpp
+++ b/tests/GrPorterDuffTest.cpp
@@ -101,7 +101,7 @@
// should always go hand in hand for Porter Duff modes.
TEST_ASSERT(analysis.requiresDstTexture() == analysis.requiresNonOverlappingDraws());
GetXPOutputTypes(xp.get(), &fPrimaryOutputType, &fSecondaryOutputType);
- xp->getBlendInfo(&fBlendInfo);
+ fBlendInfo = xp->getBlendInfo();
TEST_ASSERT(!xp->willReadDstColor() ||
(isLCD && (SkBlendMode::kSrcOver != xfermode ||
!inputColor.isOpaque())));
@@ -960,8 +960,7 @@
return;
}
- GrXferProcessor::BlendInfo blendInfo;
- xp_opaque->getBlendInfo(&blendInfo);
+ GrXferProcessor::BlendInfo blendInfo = xp_opaque->getBlendInfo();
TEST_ASSERT(blendInfo.fWriteColor);
// Test with non-opaque alpha
@@ -976,7 +975,7 @@
return;
}
- xp->getBlendInfo(&blendInfo);
+ blendInfo = xp->getBlendInfo();
TEST_ASSERT(blendInfo.fWriteColor);
}