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>
diff --git a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
index 6a07445..bae50e9 100644
--- a/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
+++ b/src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp
@@ -294,7 +294,8 @@
if (CustomFeatures::kSampleLocations & fProgramBuilder->processorFeatures()) {
const SkTArray<SkPoint>& sampleLocations = fProgramBuilder->getSampleLocations();
- this->definitions().append("const float2 _sampleOffsets[] = float2[](");
+ this->definitions().appendf("const float2 _sampleOffsets[%i] = float2[%i](",
+ sampleLocations.count(), sampleLocations.count());
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 a99f7f4..35ffea6 100644
--- a/src/gpu/vk/GrVkCaps.cpp
+++ b/src/gpu/vk/GrVkCaps.cpp
@@ -441,6 +441,10 @@
// 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
@@ -473,6 +477,10 @@
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;
@@ -548,7 +556,7 @@
// to be true with Vulkan as well.
shaderCaps->fPreferFlatInterpolation = kQualcomm_VkVendor != properties.vendorID;
- // GrShaderCaps
+ shaderCaps->fSampleMaskSupport = true;
shaderCaps->fShaderDerivativeSupport = true;
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index e8305ab..ef1a2b2 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -1833,6 +1833,59 @@
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 d25fabe..42087fc 100644
--- a/src/gpu/vk/GrVkGpu.h
+++ b/src/gpu/vk/GrVkGpu.h
@@ -75,10 +75,7 @@
kSkip_SyncQueue
};
- void querySampleLocations(GrRenderTarget*, SkTArray<SkPoint>*) override {
- SkASSERT(!this->caps()->sampleLocationsSupport());
- SK_ABORT("Sample locations not yet implemented for Vulkan.");
- }
+ void querySampleLocations(GrRenderTarget*, SkTArray<SkPoint>*) override;
void xferBarrier(GrRenderTarget*, GrXferBarrierType) override {}
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index b7c4f17..2e78b99 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -1166,9 +1166,6 @@
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;
@@ -2425,25 +2422,6 @@
}
}
-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,
@@ -2509,9 +2487,6 @@
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 abb787c..6ca6248 100644
--- a/src/sksl/SkSLIRGenerator.h
+++ b/src/sksl/SkSLIRGenerator.h
@@ -169,7 +169,6 @@
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 ce5d73b..858d8ca 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2713,6 +2713,22 @@
}
}
+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) {
@@ -2737,13 +2753,7 @@
SkASSERT(!fProgram.fSettings.fFragColorIsInOut);
continue;
}
- 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)
+ if (is_dead(*var)) {
continue;
}
SpvStorageClass_ storageClass;
@@ -3161,7 +3171,8 @@
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) {
+ intf.fVariable.fModifiers.fLayout.fBuiltin == -1 &&
+ !is_dead(intf.fVariable)) {
interfaceVars.insert(id);
}
}
@@ -3190,7 +3201,7 @@
const Variable* var = entry.first;
if (var->fStorage == Variable::kGlobal_Storage &&
((var->fModifiers.fFlags & Modifiers::kIn_Flag) ||
- (var->fModifiers.fFlags & Modifiers::kOut_Flag))) {
+ (var->fModifiers.fFlags & Modifiers::kOut_Flag)) && !is_dead(*var)) {
interfaceVars.insert(entry.second);
}
}
