Revert "Reland "Reland "Implement sample mask and sample locations support in Vulkan"""
This reverts commit 8a0f50944ba8cd46cdcda42523856afa1b05f252.
Reason for revert: Concerns about layout tests and bugs on Adreno
Original change's description:
> Reland "Reland "Implement sample mask and sample locations support in Vulkan""
>
> This is a reland of 97e917a25fa8568ccc90a2da671e375e727fa7e8, this
> time with MSAA CCPR disabled.
>
> Original change's description:
> > Reland "Implement sample mask and sample locations support in Vulkan"
> >
> > This is a reland of 8b915a0c27652d5932ff4013ba98c57baf553c3c
> >
> > Original change's description:
> > > Implement sample mask and sample locations support in Vulkan
> > >
> > > Change-Id: I372695ec5360def42a8a997675993264740b0da4
> > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252038
> > > Commit-Queue: Chris Dalton <csmartdalton@google.com>
> > > Reviewed-by: Chris Dalton <csmartdalton@google.com>
> >
> > TBR=ethannicholas@google.com
> >
> > Change-Id: I20de36719db52ad4dfc5290101d48a8fd9601c11
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/252936
> > Reviewed-by: Chris Dalton <csmartdalton@google.com>
> > Commit-Queue: Chris Dalton <csmartdalton@google.com>
>
> TBR=ethannicholas@google.com
>
> Change-Id: Ie0861dc20df1fa622cb26ed78369d29976e3be10
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254608
> Reviewed-by: Chris Dalton <csmartdalton@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
TBR=csmartdalton@google.com,ethannicholas@google.com
Change-Id: I8c9d8850c2b5bfe4d1516b9eaa2f724f478e6c8c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/254624
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
index bae50e9..6a07445 100644
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
@@ -294,8 +294,7 @@
if (CustomFeatures::kSampleLocations & fProgramBuilder->processorFeatures()) {
const SkTArray<SkPoint>& sampleLocations = fProgramBuilder->getSampleLocations();
- this->definitions().appendf("const float2 _sampleOffsets[%i] = float2[%i](",
- sampleLocations.count(), sampleLocations.count());
+ this->definitions().append("const float2 _sampleOffsets[] = float2[](");
for (int i = 0; i < sampleLocations.count(); ++i) {
SkPoint offset = sampleLocations[i] - SkPoint::Make(.5f, .5f);
if (kBottomLeft_GrSurfaceOrigin == this->getSurfaceOrigin()) {
diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp
index 35ffea6..a99f7f4 100644
--- a/src/gpu/vk/GrVkCaps.cpp
+++ b/src/gpu/vk/GrVkCaps.cpp
@@ -441,10 +441,6 @@
// GrCaps workarounds
////////////////////////////////////////////////////////////////////////////
- // Temporarily disable the MSAA implementation of CCPR while we work out a crash on Win10
- // GTX660 and incorrect rendring on Adreno.
- fDriverBlacklistMSAACCPR = true;
-
if (kARM_VkVendor == properties.vendorID) {
fInstanceAttribSupport = false;
fAvoidWritePixelsFastPath = true; // bugs.skia.org/8064
@@ -477,10 +473,6 @@
static const uint32_t kMaxVertexAttributes = 64;
fMaxVertexAttributes = SkTMin(properties.limits.maxVertexInputAttributes, kMaxVertexAttributes);
- if (properties.limits.standardSampleLocations) {
- fSampleLocationsSupport = true;
- }
-
if (extensions.hasExtension(VK_EXT_SAMPLE_LOCATIONS_EXTENSION_NAME, 1)) {
// We "disable" multisample by colocating all samples at pixel center.
fMultisampleDisableSupport = true;
@@ -556,7 +548,7 @@
// to be true with Vulkan as well.
shaderCaps->fPreferFlatInterpolation = kQualcomm_VkVendor != properties.vendorID;
- shaderCaps->fSampleMaskSupport = true;
+ // GrShaderCaps
shaderCaps->fShaderDerivativeSupport = true;
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index ef1a2b2..e8305ab 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -1833,59 +1833,6 @@
return GrBackendTexture(dimensions.width(), dimensions.height(), info);
}
-void GrVkGpu::querySampleLocations(GrRenderTarget* renderTarget,
- SkTArray<SkPoint>* sampleLocations) {
- // In Vulkan, sampleLocationsSupport() means that the platform uses the standard sample
- // locations defined by the spec.
- SkASSERT(this->caps()->sampleLocationsSupport());
- static SkPoint kStandardSampleLocations_1[1] = {
- {0.5f, 0.5f}};
- static SkPoint kStandardSampleLocations_2[2] = {
- {0.75f, 0.75f}, {0.25f, 0.25f}};
- static SkPoint kStandardSampleLocations_4[4] = {
- {0.375f, 0.125f}, {0.875f, 0.375f}, {0.125f, 0.625f}, {0.625f, 0.875f}};
- static SkPoint kStandardSampleLocations_8[8] = {
- {0.5625f, 0.3125f}, {0.4375f, 0.6875f}, {0.8125f, 0.5625f}, {0.3125f, 0.1875f},
- {0.1875f, 0.8125f}, {0.0625f, 0.4375f}, {0.6875f, 0.9375f}, {0.9375f, 0.0625f}};
- static SkPoint kStandardSampleLocations_16[16] = {
- {0.5625f, 0.5625f}, {0.4375f, 0.3125f}, {0.3125f, 0.625f}, {0.75f, 0.4375f},
- {0.1875f, 0.375f}, {0.625f, 0.8125f}, {0.8125f, 0.6875f}, {0.6875f, 0.1875f},
- {0.375f, 0.875f}, {0.5f, 0.0625f}, {0.25f, 0.125f}, {0.125f, 0.75f},
- {0.0f, 0.5f}, {0.9375f, 0.25f}, {0.875f, 0.9375f}, {0.0625f, 0.0f}};
-
- int numSamples = renderTarget->numSamples();
- if (1 == numSamples) {
- SkASSERT(this->caps()->mixedSamplesSupport());
- if (auto* stencil = renderTarget->renderTargetPriv().getStencilAttachment()) {
- numSamples = stencil->numSamples();
- }
- }
- SkASSERT(numSamples > 1);
- SkASSERT(!renderTarget->renderTargetPriv().getStencilAttachment() ||
- numSamples == renderTarget->renderTargetPriv().getStencilAttachment()->numSamples());
-
- switch (numSamples) {
- case 1:
- sampleLocations->push_back_n(1, kStandardSampleLocations_1);
- break;
- case 2:
- sampleLocations->push_back_n(2, kStandardSampleLocations_2);
- break;
- case 4:
- sampleLocations->push_back_n(4, kStandardSampleLocations_4);
- break;
- case 8:
- sampleLocations->push_back_n(8, kStandardSampleLocations_8);
- break;
- case 16:
- sampleLocations->push_back_n(16, kStandardSampleLocations_16);
- break;
- default:
- SK_ABORT("Invalid vulkan sample count.");
- break;
- }
-}
-
void GrVkGpu::deleteBackendTexture(const GrBackendTexture& tex) {
SkASSERT(GrBackendApi::kVulkan == tex.fBackend);
diff --git a/src/gpu/vk/GrVkGpu.h b/src/gpu/vk/GrVkGpu.h
index 42087fc..d25fabe 100644
--- a/src/gpu/vk/GrVkGpu.h
+++ b/src/gpu/vk/GrVkGpu.h
@@ -75,7 +75,10 @@
kSkip_SyncQueue
};
- void querySampleLocations(GrRenderTarget*, SkTArray<SkPoint>*) override;
+ void querySampleLocations(GrRenderTarget*, SkTArray<SkPoint>*) override {
+ SkASSERT(!this->caps()->sampleLocationsSupport());
+ SK_ABORT("Sample locations not yet implemented for Vulkan.");
+ }
void xferBarrier(GrRenderTarget*, GrXferBarrierType) override {}
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 2e78b99..b7c4f17 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -1166,6 +1166,9 @@
case SK_HEIGHT_BUILTIN:
fInputs.fRTHeight = true;
break;
+ case SK_SAMPLEMASK_BUILTIN:
+ fUsesSampleMask = true;
+ break;
#ifndef SKSL_STANDALONE
case SK_FRAGCOORD_BUILTIN:
fInputs.fFlipY = true;
@@ -2422,6 +2425,25 @@
}
}
+void IRGenerator::removeSampleMask(std::vector<std::unique_ptr<ProgramElement>>* out) {
+ for (const auto& e : *out) {
+ switch (e->fKind) {
+ case ProgramElement::kVar_Kind: {
+ VarDeclarations& vd = (VarDeclarations&) *e;
+ for (auto iter = vd.fVars.begin(); iter != vd.fVars.end(); ++iter) {
+ SkASSERT((*iter)->fKind == Statement::kVarDeclaration_Kind);
+ const auto& v = (VarDeclaration&) **iter;
+ if (v.fVar->fName == "sk_SampleMask") {
+ vd.fVars.erase(iter);
+ return;
+ }
+ }
+ }
+ default: break;
+ }
+ }
+}
+
void IRGenerator::convertProgram(Program::Kind kind,
const char* text,
size_t length,
@@ -2487,6 +2509,9 @@
ABORT("unsupported declaration: %s\n", decl.description().c_str());
}
}
+ if (!fUsesSampleMask) {
+ this->removeSampleMask(out);
+ }
}
diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h
index 6ca6248..abb787c 100644
--- a/src/sksl/SkSLIRGenerator.h
+++ b/src/sksl/SkSLIRGenerator.h
@@ -169,6 +169,7 @@
Variable* fRTAdjustInterfaceBlock;
int fRTAdjustFieldIndex;
bool fStarted = false;
+ bool fUsesSampleMask = false;
friend class AutoSymbolTable;
friend class AutoLoopLevel;
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index 858d8ca..ce5d73b 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2713,22 +2713,6 @@
}
}
-bool is_dead(const Variable& var) {
- if (var.fReadCount || var.fWriteCount) {
- return false;
- }
- // not entirely sure what the rules are for when it's safe to elide interface variables, but it
- // causes various problems to elide some of them even when dead. But it also causes problems
- // *not* to elide sk_SampleMask when it's not being used.
- if (!(var.fModifiers.fFlags & (Modifiers::kIn_Flag |
- Modifiers::kOut_Flag |
- Modifiers::kUniform_Flag |
- Modifiers::kBuffer_Flag))) {
- return true;
- }
- return var.fModifiers.fLayout.fBuiltin == SK_SAMPLEMASK_BUILTIN;
-}
-
#define BUILTIN_IGNORE 9999
void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclarations& decl,
OutputStream& out) {
@@ -2753,7 +2737,13 @@
SkASSERT(!fProgram.fSettings.fFragColorIsInOut);
continue;
}
- if (is_dead(*var)) {
+ if (!var->fReadCount && !var->fWriteCount &&
+ !(var->fModifiers.fFlags & (Modifiers::kIn_Flag |
+ Modifiers::kOut_Flag |
+ Modifiers::kUniform_Flag |
+ Modifiers::kBuffer_Flag))) {
+ // variable is dead and not an input / output var (the Vulkan debug layers complain if
+ // we elide an interface var, even if it's dead)
continue;
}
SpvStorageClass_ storageClass;
@@ -3171,8 +3161,7 @@
SpvId id = this->writeInterfaceBlock(intf);
if (((intf.fVariable.fModifiers.fFlags & Modifiers::kIn_Flag) ||
(intf.fVariable.fModifiers.fFlags & Modifiers::kOut_Flag)) &&
- intf.fVariable.fModifiers.fLayout.fBuiltin == -1 &&
- !is_dead(intf.fVariable)) {
+ intf.fVariable.fModifiers.fLayout.fBuiltin == -1) {
interfaceVars.insert(id);
}
}
@@ -3201,7 +3190,7 @@
const Variable* var = entry.first;
if (var->fStorage == Variable::kGlobal_Storage &&
((var->fModifiers.fFlags & Modifiers::kIn_Flag) ||
- (var->fModifiers.fFlags & Modifiers::kOut_Flag)) && !is_dead(*var)) {
+ (var->fModifiers.fFlags & Modifiers::kOut_Flag))) {
interfaceVars.insert(entry.second);
}
}
