Skip to content

Commit c25ac68

Browse files
bejadobejado
authored
Better set default shader stages for samplers (#8992)
1 parent 0f572f7 commit c25ac68

File tree

7 files changed

+182
-15
lines changed

7 files changed

+182
-15
lines changed

‎filament/src/materials/blitDepth.mat

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ material {
1919
target : depth,
2020
type : float
2121
}
22-
],
22+
],
2323
variables : [
2424
vertex
2525
],

‎filament/src/materials/skybox.mat

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ material {
1111
},
1212
{
1313
type : samplerCubemap,
14-
name : skybox
14+
name : skybox,
15+
stages : [ "fragment" ]
1516
},
1617
{
1718
type : float4,

‎libs/filamat/include/filamat/MaterialBuilder.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include <utility>
4242
#include <vector>
4343
#include <variant>
44+
#include <optional>
4445

4546
#include <stddef.h>
4647
#include <stdint.h>
@@ -327,7 +328,7 @@ class UTILS_PUBLIC MaterialBuilder : public MaterialBuilderBase {
327328
ParameterPrecision precision = ParameterPrecision::DEFAULT,
328329
bool filterable = true, /* defaulting to filterable because format is default to float */
329330
bool multisample = false, const char* transformName = "",
330-
ShaderStageFlags stages = ShaderStageFlags::ALL_SHADER_STAGE_FLAGS);
331+
std::optional<ShaderStageFlags> stages = {});
331332

332333
MaterialBuilder& buffer(filament::BufferInterfaceBlock bib);
333334

@@ -660,7 +661,7 @@ class UTILS_PUBLIC MaterialBuilder : public MaterialBuilderBase {
660661

661662
// Sampler
662663
Parameter(const char* paramName, SamplerType t, SamplerFormat f, ParameterPrecision p,
663-
bool filterable, bool ms, const char* tn, ShaderStageFlags s)
664+
bool filterable, bool ms, const char* tn, std::optional<ShaderStageFlags> s)
664665
: name(paramName),
665666
size(1),
666667
precision(p),
@@ -704,7 +705,7 @@ class UTILS_PUBLIC MaterialBuilder : public MaterialBuilderBase {
704705
bool filterable;
705706
bool multisample;
706707
utils::CString transformName;
707-
ShaderStageFlags stages;
708+
std::optional<ShaderStageFlags> stages;
708709
enum {
709710
INVALID,
710711
UNIFORM,

‎libs/filamat/src/MaterialBuilder.cpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ MaterialBuilder& MaterialBuilder::parameter(const char* name, UniformType const
302302

303303
MaterialBuilder& MaterialBuilder::parameter(const char* name, SamplerType samplerType,
304304
SamplerFormat format, ParameterPrecision precision, bool filterable, bool multisample,
305-
const char* transformName, ShaderStageFlags stages) {
305+
const char* transformName, std::optional<ShaderStageFlags> stages) {
306306
FILAMENT_CHECK_PRECONDITION(
307307
!multisample || (format != SamplerFormat::SHADOW &&
308308
(samplerType == SamplerType::SAMPLER_2D ||
@@ -629,6 +629,16 @@ bool MaterialBuilder::hasSamplerType(SamplerType const samplerType) const noexce
629629
void MaterialBuilder::prepareToBuild(MaterialInfo& info) noexcept {
630630
prepare(mEnableFramebufferFetch, mFeatureLevel);
631631

632+
const bool hasEmptyVertexCode = mMaterialVertexCode.getResolved().empty();
633+
const bool isPostProcessMaterial = mMaterialDomain == MaterialDomain::POST_PROCESS;
634+
// TODO: Currently, for surface materials, we rely on the presence of a custom vertex shader to
635+
// infer the default shader stages. We could do better by analyzing the AST of the vertex shader
636+
// to see if the sampler is actually used.
637+
const ShaderStageFlags defaultShaderStages =
638+
isPostProcessMaterial || hasEmptyVertexCode
639+
? (ShaderStageFlags::FRAGMENT)
640+
: (ShaderStageFlags::FRAGMENT | ShaderStageFlags::VERTEX);
641+
632642
// Build the per-material sampler block and uniform block.
633643
SamplerInterfaceBlock::Builder sbb;
634644
BufferInterfaceBlock::Builder ibb;
@@ -638,9 +648,10 @@ void MaterialBuilder::prepareToBuild(MaterialInfo& info) noexcept {
638648
auto const& param = mParameters[i];
639649
assert_invariant(!param.isSubpass());
640650
if (param.isSampler()) {
651+
ShaderStageFlags stages = param.stages.value_or(defaultShaderStages);
641652
sbb.add({ param.name.data(), param.name.size() }, binding, param.samplerType,
642653
param.format, param.precision, param.filterable, param.multisample,
643-
param.stages);
654+
stages);
644655
if (!param.transformName.empty()) {
645656
ibb.add({ { { param.transformName.data(), param.transformName.size() },
646657
uint8_t(binding), 0, UniformType::MAT3, Precision::DEFAULT,

‎libs/matdbg/src/CommonWriter.h

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,105 @@ const char* toString(backend::ConstantType type) noexcept {
247247
return "--";
248248
}
249249

250+
inline
251+
const char* toString(backend::DescriptorType type) noexcept {
252+
switch (type) {
253+
case backend::DescriptorType::SAMPLER_2D_FLOAT:
254+
return "sampler2d float";
255+
case backend::DescriptorType::SAMPLER_2D_INT:
256+
return "sampler2d int";
257+
case backend::DescriptorType::SAMPLER_2D_UINT:
258+
return "sampler2d uint";
259+
case backend::DescriptorType::SAMPLER_2D_DEPTH:
260+
return "sampler2d depth";
261+
case backend::DescriptorType::SAMPLER_2D_ARRAY_FLOAT:
262+
return "sampler2d array float";
263+
case backend::DescriptorType::SAMPLER_2D_ARRAY_INT:
264+
return "sampler2d array int";
265+
case backend::DescriptorType::SAMPLER_2D_ARRAY_UINT:
266+
return "sampler2d array uint";
267+
case backend::DescriptorType::SAMPLER_2D_ARRAY_DEPTH:
268+
return "sampler2d array depth";
269+
case backend::DescriptorType::SAMPLER_CUBE_FLOAT:
270+
return "samplercube float";
271+
case backend::DescriptorType::SAMPLER_CUBE_INT:
272+
return "samplercube int";
273+
case backend::DescriptorType::SAMPLER_CUBE_UINT:
274+
return "samplercube uint";
275+
case backend::DescriptorType::SAMPLER_CUBE_DEPTH:
276+
return "samplercube depth";
277+
case backend::DescriptorType::SAMPLER_CUBE_ARRAY_FLOAT:
278+
return "samplercube array float";
279+
case backend::DescriptorType::SAMPLER_CUBE_ARRAY_INT:
280+
return "samplercube array int";
281+
case backend::DescriptorType::SAMPLER_CUBE_ARRAY_UINT:
282+
return "samplercube array uint";
283+
case backend::DescriptorType::SAMPLER_CUBE_ARRAY_DEPTH:
284+
return "samplercube array depth";
285+
case backend::DescriptorType::SAMPLER_3D_FLOAT:
286+
return "sampler3d float";
287+
case backend::DescriptorType::SAMPLER_3D_INT:
288+
return "sampler3d int";
289+
case backend::DescriptorType::SAMPLER_3D_UINT:
290+
return "sampler3d uint";
291+
case backend::DescriptorType::SAMPLER_2D_MS_FLOAT:
292+
return "sampler2d ms float";
293+
case backend::DescriptorType::SAMPLER_2D_MS_INT:
294+
return "sampler2d ms int";
295+
case backend::DescriptorType::SAMPLER_2D_MS_UINT:
296+
return "sampler2d ms uint";
297+
case backend::DescriptorType::SAMPLER_2D_MS_ARRAY_FLOAT:
298+
return "sampler2d ms array float";
299+
case backend::DescriptorType::SAMPLER_2D_MS_ARRAY_INT:
300+
return "sampler2d ms array int";
301+
case backend::DescriptorType::SAMPLER_2D_MS_ARRAY_UINT:
302+
return "sampler2d ms array uint";
303+
case backend::DescriptorType::SAMPLER_EXTERNAL:
304+
return "sampler external";
305+
case backend::DescriptorType::UNIFORM_BUFFER:
306+
return "uniform buffer";
307+
case backend::DescriptorType::SHADER_STORAGE_BUFFER:
308+
return "shader storage buffer";
309+
case backend::DescriptorType::INPUT_ATTACHMENT:
310+
return "input attachment";
311+
}
312+
return "--";
313+
}
314+
315+
inline
316+
const char* toString(backend::ShaderStageFlags flags) noexcept {
317+
switch (uint8_t(flags)) {
318+
case uint8_t(backend::ShaderStageFlags::VERTEX):
319+
return "vertex";
320+
case uint8_t(backend::ShaderStageFlags::FRAGMENT):
321+
return "fragment";
322+
case uint8_t(backend::ShaderStageFlags::VERTEX | backend::ShaderStageFlags::FRAGMENT):
323+
return "vertex | fragment";
324+
case uint8_t(backend::ShaderStageFlags::COMPUTE):
325+
return "compute";
326+
case uint8_t(backend::ShaderStageFlags::VERTEX | backend::ShaderStageFlags::COMPUTE):
327+
return "vertex | compute";
328+
case uint8_t(backend::ShaderStageFlags::FRAGMENT | backend::ShaderStageFlags::COMPUTE):
329+
return "fragment | compute";
330+
case uint8_t(backend::ShaderStageFlags::ALL_SHADER_STAGE_FLAGS):
331+
return "vertex | fragment | compute";
332+
}
333+
return "--";
334+
}
335+
336+
inline
337+
const char* toString(backend::DescriptorFlags flags) noexcept {
338+
switch (uint8_t(flags)) {
339+
case uint8_t(backend::DescriptorFlags::DYNAMIC_OFFSET):
340+
return "dynamic offset";
341+
case uint8_t(backend::DescriptorFlags::UNFILTERABLE):
342+
return "unfilterable";
343+
case uint8_t(backend::DescriptorFlags::DYNAMIC_OFFSET | backend::DescriptorFlags::UNFILTERABLE):
344+
return "dynamic offset | unfilterable";
345+
}
346+
return "--";
347+
}
348+
250349
// Returns a human-readable variant description.
251350
// For example: DYN|DIR
252351
std::string formatVariantString(Variant variant, MaterialDomain domain) noexcept;

‎libs/matdbg/src/TextWriter.cpp

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,61 @@ static bool printMaterial(ostream& text, const ChunkContainer& container) {
156156
return true;
157157
}
158158

159+
static bool printDescriptorSetLayout(ostream& text, const ChunkContainer& container) {
160+
if (!container.hasChunk(ChunkType::MaterialDescriptorSetLayoutInfo)) {
161+
return true;
162+
}
163+
164+
auto [startDsli, endDsli] = container.getChunkRange(ChunkType::MaterialDescriptorSetLayoutInfo);
165+
Unflattener dsli(startDsli, endDsli);
166+
167+
uint8_t descriptorCount;
168+
if (!dsli.read(&descriptorCount)) {
169+
return false;
170+
}
171+
172+
text << "Descriptors:" << endl;
173+
174+
for (int i = 0; i < descriptorCount; i++) {
175+
uint8_t type;
176+
uint8_t stages;
177+
uint8_t binding;
178+
uint8_t flags;
179+
180+
if (!dsli.read(&type)) {
181+
return false;
182+
}
183+
184+
if (!dsli.read(&stages)) {
185+
return false;
186+
}
187+
188+
if (!dsli.read(&binding)) {
189+
return false;
190+
}
191+
192+
if (!dsli.read(&flags)) {
193+
return false;
194+
}
195+
196+
uint16_t padding;
197+
if (!dsli.read(&padding)) {
198+
return false;
199+
}
200+
201+
text << " "
202+
<< setw(alignment) << toString(DescriptorType(type))
203+
<< setw(alignment) << toString(ShaderStageFlags(stages))
204+
<< setw(shortAlignment) << static_cast<int>(binding)
205+
<< setw(alignment) << toString(DescriptorFlags(flags))
206+
<< endl;
207+
}
208+
209+
text << endl;
210+
211+
return true;
212+
}
213+
159214
static bool printParametersInfo(ostream& text, const ChunkContainer& container) {
160215
if (!container.hasChunk(ChunkType::MaterialUib)) {
161216
return true;
@@ -460,6 +515,9 @@ bool TextWriter::writeMaterialInfo(const filaflat::ChunkContainer& container) {
460515
if (!printParametersInfo(text, container)) {
461516
return false;
462517
}
518+
if (!printDescriptorSetLayout(text, container)) {
519+
return false;
520+
}
463521
if (!printConstantInfo(text, container)) {
464522
return false;
465523
}

‎tools/matc/src/matc/ParametersProcessor.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,9 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json
205205

206206
const JsonishValue* stagesValue = jsonObject.getValue("stages");
207207
using filament::backend::ShaderStageFlags;
208-
auto stages = ShaderStageFlags::NONE;
208+
std::optional<ShaderStageFlags> stages;
209209
if (stagesValue) {
210+
ShaderStageFlags parsedStages = ShaderStageFlags::NONE;
210211
if (stagesValue->getType() != JsonishValue::ARRAY) {
211212
std::cerr << "parameters: stages must be an ARRAY." << std::endl;
212213
return false;
@@ -217,7 +218,7 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json
217218
using Qualifier = filament::BufferInterfaceBlock::Qualifier;
218219
auto stageString = value->toJsonString()->getString();
219220
if (Enums::isValid<ShaderStageType>(stageString)) {
220-
stages |= Enums::toEnum<ShaderStageType>(stageString);
221+
parsedStages |= Enums::toEnum<ShaderStageType>(stageString);
221222
} else {
222223
std::cerr << "stages: the stage '" << stageString
223224
<< "' for parameter with name '" << nameString
@@ -229,12 +230,13 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json
229230
std::cerr << "parameters: stages must be an array of STRINGs." << std::endl;
230231
return false;
231232
}
233+
stages = parsedStages;
232234
}
233235

234236
size_t const arraySize = extractArraySize(typeString);
235237

236238
if (Enums::isValid<UniformType>(typeString)) {
237-
if (stages != ShaderStageFlags::NONE) {
239+
if (stages.has_value()) {
238240
std::cerr << "parameters: the uniform parameter with name '" << nameString << "'"
239241
<< " has shader stages specified. Shader stages are only supported for"
240242
<< " samplers." << std::endl;
@@ -288,11 +290,6 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json
288290

289291
auto multisample = multiSampleValue ? multiSampleValue->toJsonBool()->getBool() : false;
290292

291-
if (stages == ShaderStageFlags::NONE) {
292-
// TODO: Infer the default shader stages based on which blocks are present in the
293-
// material.
294-
stages = ShaderStageFlags::VERTEX | ShaderStageFlags::FRAGMENT;
295-
}
296293
if (transformNameValue) {
297294
if (type != MaterialBuilder::SamplerType::SAMPLER_EXTERNAL) {
298295
std::cerr << "parameters: the parameter with name '" << nameString << "'"

0 commit comments

Comments
 (0)