From c68d0a4e465c83ebf99f2d6506fc29dd72118171 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Mon, 19 Oct 2020 10:41:10 -0700 Subject: [PATCH 01/24] Vulkan: add missing field to VmaVulkanFunctions. --- filament/backend/src/vulkan/VulkanContext.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/filament/backend/src/vulkan/VulkanContext.cpp b/filament/backend/src/vulkan/VulkanContext.cpp index 3ef4527f15..c024ceb0d9 100644 --- a/filament/backend/src/vulkan/VulkanContext.cpp +++ b/filament/backend/src/vulkan/VulkanContext.cpp @@ -244,6 +244,7 @@ void createLogicalDevice(VulkanContext& context) { .vkDestroyBuffer = vkDestroyBuffer, .vkCreateImage = vkCreateImage, .vkDestroyImage = vkDestroyImage, + .vkCmdCopyBuffer = vkCmdCopyBuffer, .vkGetBufferMemoryRequirements2KHR = vkGetBufferMemoryRequirements2KHR, .vkGetImageMemoryRequirements2KHR = vkGetImageMemoryRequirements2KHR }; From e7250571e7785c08e38a592cb610a70d2d445b18 Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Mon, 19 Oct 2020 16:28:26 -0600 Subject: [PATCH 02/24] Add subpass parameter type to materials (#3193) --- .../backend/include/backend/DriverEnums.h | 5 ++ .../colorGrading/colorGradingAsSubpass.mat | 14 +++-- .../include/filament/MaterialChunkType.h | 1 + .../include/private/filament/SubpassInfo.h | 51 +++++++++++++++++ libs/filamat/include/filamat/Enums.h | 2 + .../filamat/include/filamat/MaterialBuilder.h | 44 ++++++++++++--- libs/filamat/src/Enums.cpp | 9 +++ libs/filamat/src/MaterialBuilder.cpp | 46 +++++++++++++-- .../src/eiff/MaterialInterfaceBlockChunk.cpp | 18 ++++++ .../src/eiff/MaterialInterfaceBlockChunk.h | 12 ++++ libs/filamat/src/shaders/CodeGenerator.cpp | 23 ++++++++ libs/filamat/src/shaders/CodeGenerator.h | 5 ++ libs/filamat/src/shaders/MaterialInfo.h | 2 + libs/filamat/src/shaders/ShaderGenerator.cpp | 3 + libs/matdbg/src/CommonWriter.h | 7 +++ libs/matdbg/src/TextWriter.cpp | 56 ++++++++++++++++++- tools/matc/src/matc/MaterialCompiler.cpp | 15 +++-- tools/matc/src/matc/ParametersProcessor.cpp | 28 +++++++++- 18 files changed, 317 insertions(+), 24 deletions(-) create mode 100644 libs/filabridge/include/private/filament/SubpassInfo.h diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index f4564f1421..cb17bdf010 100644 --- a/filament/backend/include/backend/DriverEnums.h +++ b/filament/backend/include/backend/DriverEnums.h @@ -215,6 +215,11 @@ enum class SamplerType : uint8_t { SAMPLER_3D, //!< 3D texture }; +//! Subpass type +enum class SubpassType : uint8_t { + SUBPASS_INPUT +}; + //! Texture sampler format enum class SamplerFormat : uint8_t { INT = 0, //!< signed integer sampler diff --git a/filament/src/materials/colorGrading/colorGradingAsSubpass.mat b/filament/src/materials/colorGrading/colorGradingAsSubpass.mat index 1ca147a377..d98f29713f 100644 --- a/filament/src/materials/colorGrading/colorGradingAsSubpass.mat +++ b/filament/src/materials/colorGrading/colorGradingAsSubpass.mat @@ -26,6 +26,12 @@ material { { type : float4, name : vignetteColor + }, + { + type : subpassInput, + format : float, + precision : medium, + name : colorBuffer, } ], variables : [ @@ -45,9 +51,7 @@ vertex { fragment { - // TODO: this should be specified as a parameter - // In our Vulkan backend, subpass inputs always live in descriptor set 2. (ignored for GLES) - layout (input_attachment_index = 0, set = 2, binding = 0) uniform mediump subpassInput colorBuffer; + // TODO: specify an output for this at location 1. layout(location = 1) out vec4 tonemappedOutput; #include "../../../../shaders/src/dithering.fs" @@ -64,11 +68,11 @@ fragment { } vec3 resolveFragment(const ivec2 uv) { - return subpassLoad(colorBuffer).rgb; + return subpassLoad(materialParams_colorBuffer).rgb; } vec4 resolveAlphaFragment(const ivec2 uv) { - return subpassLoad(colorBuffer); + return subpassLoad(materialParams_colorBuffer); } vec4 resolve() { diff --git a/libs/filabridge/include/filament/MaterialChunkType.h b/libs/filabridge/include/filament/MaterialChunkType.h index b1bc9c4222..56d6eb50d5 100644 --- a/libs/filabridge/include/filament/MaterialChunkType.h +++ b/libs/filabridge/include/filament/MaterialChunkType.h @@ -40,6 +40,7 @@ enum UTILS_PUBLIC ChunkType : uint64_t { Unknown = charTo64bitNum("UNKNOWN "), MaterialUib = charTo64bitNum("MAT_UIB "), MaterialSib = charTo64bitNum("MAT_SIB "), + MaterialSubpass = charTo64bitNum("MAT_SUB "), MaterialGlsl = charTo64bitNum("MAT_GLSL"), MaterialSpirv = charTo64bitNum("MAT_SPIR"), MaterialMetal = charTo64bitNum("MAT_METL"), diff --git a/libs/filabridge/include/private/filament/SubpassInfo.h b/libs/filabridge/include/private/filament/SubpassInfo.h new file mode 100644 index 0000000000..e3ecbb6f7e --- /dev/null +++ b/libs/filabridge/include/private/filament/SubpassInfo.h @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef TNT_FILAMENT_SUBPASSINFO_H +#define TNT_FILAMENT_SUBPASSINFO_H + +#include + +#include + +namespace filament { + +using Type = backend::SubpassType; +using Format = backend::SamplerFormat; +using Precision = backend::Precision; + +struct SubpassInfo { + SubpassInfo() = default; + SubpassInfo(utils::CString block, utils::CString name, Type type, Format format, + Precision precision, uint8_t attachmentIndex, uint8_t binding) noexcept + : block(std::move(block)), name(std::move(name)), type(type), format(format), + precision(precision), attachmentIndex(attachmentIndex), binding(binding), + isValid(true) { + } + // name of the block this subpass belongs to + utils::CString block = utils::CString("MaterialParams"); + utils::CString name; // name of this subpass + Type type; // type of this subpass + Format format; // format of this subpass + Precision precision; // precision of this subpass + uint8_t attachmentIndex = 0; + uint8_t binding = 0; + bool isValid = false; +}; + +} // namespace filament + +#endif // TNT_FILAMENT_SUBPASSINFO_H diff --git a/libs/filamat/include/filamat/Enums.h b/libs/filamat/include/filamat/Enums.h index 900976925b..cb0600ba7a 100644 --- a/libs/filamat/include/filamat/Enums.h +++ b/libs/filamat/include/filamat/Enums.h @@ -28,6 +28,7 @@ namespace filamat { using Property = MaterialBuilder::Property; using UniformType = MaterialBuilder::UniformType; using SamplerType = MaterialBuilder::SamplerType; +using SubpassType = MaterialBuilder::SubpassType; using SamplerFormat = MaterialBuilder::SamplerFormat; using SamplerPrecision = MaterialBuilder::SamplerPrecision; using OutputTarget = MaterialBuilder::OutputTarget; @@ -70,6 +71,7 @@ private: static std::unordered_map mStringToProperty; static std::unordered_map mStringToUniformType; static std::unordered_map mStringToSamplerType; + static std::unordered_map mStringToSubpassType; static std::unordered_map mStringToSamplerFormat; static std::unordered_map mStringToSamplerPrecision; static std::unordered_map mStringToOutputTarget; diff --git a/libs/filamat/include/filamat/MaterialBuilder.h b/libs/filamat/include/filamat/MaterialBuilder.h index 079e0e9b69..fbfbf0091c 100644 --- a/libs/filamat/include/filamat/MaterialBuilder.h +++ b/libs/filamat/include/filamat/MaterialBuilder.h @@ -189,6 +189,7 @@ public: using UniformType = filament::backend::UniformType; using SamplerType = filament::backend::SamplerType; + using SubpassType = filament::backend::SubpassType; using SamplerFormat = filament::backend::SamplerFormat; using SamplerPrecision = filament::backend::Precision; using CullingMode = filament::backend::CullingMode; @@ -481,24 +482,50 @@ public: // The methods and types below are for internal use /// @cond never + /** + * Add a subpass parameter to this material. + */ + MaterialBuilder& parameter(SubpassType subpassType, SamplerFormat format, SamplerPrecision + precision, const char* name) noexcept; + MaterialBuilder& parameter(SubpassType subpassType, SamplerFormat format, const char* name) + noexcept; + MaterialBuilder& parameter(SubpassType subpassType, SamplerPrecision precision, + const char* name) noexcept; + MaterialBuilder& parameter(SubpassType subpassType, const char* name) noexcept; + struct Parameter { - Parameter() noexcept = default; + Parameter() noexcept : parameterType(INVALID) {} Parameter(const char* paramName, SamplerType t, SamplerFormat f, SamplerPrecision p) - : name(paramName), size(1), samplerType(t), samplerFormat(f), samplerPrecision(p), - isSampler(true) { } + : name(paramName), size(1), samplerType(t), format(f), precision(p), + parameterType(SAMPLER) { } Parameter(const char* paramName, UniformType t, size_t typeSize) - : name(paramName), size(typeSize), uniformType(t), isSampler(false) { } + : name(paramName), size(typeSize), uniformType(t), parameterType(UNIFORM) { } + Parameter(const char* paramName, SubpassType t, SamplerFormat f, SamplerPrecision p) + : name(paramName), size(1), subpassType(t), format(f), precision(p), + parameterType(SUBPASS) { } utils::CString name; size_t size; union { UniformType uniformType; struct { - SamplerType samplerType; - SamplerFormat samplerFormat; - SamplerPrecision samplerPrecision; + union { + SamplerType samplerType; + SubpassType subpassType; + }; + SamplerFormat format; + SamplerPrecision precision; }; }; - bool isSampler; + enum { + INVALID, + UNIFORM, + SAMPLER, + SUBPASS + } parameterType; + + bool isSampler() const { return parameterType == SAMPLER; } + bool isUniform() const { return parameterType == UNIFORM; } + bool isSubpass() const { return parameterType == SUBPASS; } }; struct Output { @@ -535,6 +562,7 @@ public: bool hasExternalSampler() const noexcept; static constexpr size_t MAX_PARAMETERS_COUNT = 48; + static constexpr size_t MAX_SUBPASS_COUNT = 1; using ParameterList = Parameter[MAX_PARAMETERS_COUNT]; // returns the number of parameters declared in this material diff --git a/libs/filamat/src/Enums.cpp b/libs/filamat/src/Enums.cpp index 653d498e98..6d9b968078 100644 --- a/libs/filamat/src/Enums.cpp +++ b/libs/filamat/src/Enums.cpp @@ -94,6 +94,15 @@ std::unordered_map& Enums::getMap() noexc return mStringToSamplerType; }; +std::unordered_map Enums::mStringToSubpassType = { + { "subpassInput", SubpassType::SUBPASS_INPUT }, +}; + +template <> +std::unordered_map& Enums::getMap() noexcept { + return mStringToSubpassType; +}; + std::unordered_map Enums::mStringToSamplerPrecision = { { "default", SamplerPrecision::DEFAULT }, { "low", SamplerPrecision::LOW }, diff --git a/libs/filamat/src/MaterialBuilder.cpp b/libs/filamat/src/MaterialBuilder.cpp index 910c733e24..da0548cc0c 100644 --- a/libs/filamat/src/MaterialBuilder.cpp +++ b/libs/filamat/src/MaterialBuilder.cpp @@ -196,6 +196,20 @@ MaterialBuilder& MaterialBuilder::parameter( return *this; } +MaterialBuilder& MaterialBuilder::parameter(SubpassType subpassType, SamplerFormat format, + SamplerPrecision precision, const char* name) noexcept { + ASSERT_PRECONDITION(format == SamplerFormat::FLOAT, + "Subpass parameters must have FLOAT format."); + + auto subpassCount = std::count_if(std::begin(mParameters), std::end(mParameters), + [](const auto& p) { return p.isSubpass(); }); + + ASSERT_POSTCONDITION(subpassCount < MAX_SUBPASS_COUNT, "Too many subpasses"); + ASSERT_POSTCONDITION(mParameterCount < MAX_PARAMETERS_COUNT, "Too many parameters"); + mParameters[mParameterCount++] = { name, subpassType, format, precision }; + return *this; +} + MaterialBuilder& MaterialBuilder::parameter( SamplerType samplerType, SamplerFormat format, const char* name) noexcept { return parameter(samplerType, format, SamplerPrecision::DEFAULT, name); @@ -211,6 +225,20 @@ MaterialBuilder& MaterialBuilder::parameter( return parameter(samplerType, SamplerFormat::FLOAT, SamplerPrecision::DEFAULT, name); } +MaterialBuilder& MaterialBuilder::parameter(SubpassType subpassType, SamplerFormat format, + const char* name) noexcept { + return parameter(subpassType, format, SamplerPrecision::DEFAULT, name); +} + +MaterialBuilder& MaterialBuilder::parameter(SubpassType subpassType, SamplerPrecision precision, + const char* name) noexcept { + return parameter(subpassType, SamplerFormat::FLOAT, precision, name); +} + +MaterialBuilder& MaterialBuilder::parameter(SubpassType subpassType, const char* name) noexcept { + return parameter(subpassType, SamplerFormat::FLOAT, SamplerPrecision::DEFAULT, name); +} + MaterialBuilder& MaterialBuilder::require(filament::VertexAttribute attribute) noexcept { mRequiredAttributes.set(attribute); return *this; @@ -363,7 +391,7 @@ MaterialBuilder& MaterialBuilder::shaderDefine(const char* name, const char* val bool MaterialBuilder::hasExternalSampler() const noexcept { for (size_t i = 0, c = mParameterCount; i < c; i++) { auto const& param = mParameters[i]; - if (param.isSampler && param.samplerType == SamplerType::SAMPLER_EXTERNAL) { + if (param.isSampler() && param.samplerType == SamplerType::SAMPLER_EXTERNAL) { return true; } } @@ -378,10 +406,17 @@ void MaterialBuilder::prepareToBuild(MaterialInfo& info) noexcept { filament::UniformInterfaceBlock::Builder ibb; for (size_t i = 0, c = mParameterCount; i < c; i++) { auto const& param = mParameters[i]; - if (param.isSampler) { - sbb.add(param.name, param.samplerType, param.samplerFormat, param.samplerPrecision); - } else { + if (param.isSampler()) { + sbb.add(param.name, param.samplerType, param.format, param.precision); + } else if (param.isUniform()) { ibb.add(param.name, param.size, param.uniformType); + } else if (param.isSubpass()) { + // For now, we only support a single subpass for attachment 0. + // Subpasses blong to the "MaterialParams" block. + const uint8_t attachmentIndex = 0; + const uint8_t binding = 0; + info.subpass = { utils::CString("MaterialParams"), param.name, param.subpassType, + param.format, param.precision, attachmentIndex, binding }; } } @@ -864,6 +899,9 @@ void MaterialBuilder::writeCommonChunks(ChunkContainer& container, MaterialInfo& // SIB container.addChild(info.sib); + // Subpass + container.addChild(info.subpass); + container.addSimpleChild(ChunkType::MaterialDoubleSidedSet, mDoubleSidedCapability); container.addSimpleChild(ChunkType::MaterialDoubleSided, mDoubleSided); diff --git a/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.cpp b/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.cpp index 60928aff94..94b38e230b 100644 --- a/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.cpp +++ b/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.cpp @@ -56,4 +56,22 @@ void MaterialSamplerInterfaceBlockChunk::flatten(Flattener &f) { } } +MaterialSubpassInterfaceBlockChunk::MaterialSubpassInterfaceBlockChunk(SubpassInfo& subpass) : + Chunk(ChunkType::MaterialSubpass), + mSubpass(subpass) { +} + +void MaterialSubpassInterfaceBlockChunk::flatten(Flattener &f) { + f.writeString(mSubpass.block.c_str()); + f.writeUint64(mSubpass.isValid ? 1 : 0); // only ever a single subpass for now + if (mSubpass.isValid) { + f.writeString(mSubpass.name.c_str()); + f.writeUint8(static_cast(mSubpass.type)); + f.writeUint8(static_cast(mSubpass.format)); + f.writeUint8(static_cast(mSubpass.precision)); + f.writeUint8(static_cast(mSubpass.attachmentIndex)); + f.writeUint8(static_cast(mSubpass.binding)); + } +} + } diff --git a/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.h b/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.h index 110174dba8..e321dde7a0 100644 --- a/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.h +++ b/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.h @@ -21,6 +21,7 @@ #include #include +#include namespace filamat { @@ -46,6 +47,17 @@ private: filament::SamplerInterfaceBlock& mSib; }; +class MaterialSubpassInterfaceBlockChunk final : public Chunk { +public: + explicit MaterialSubpassInterfaceBlockChunk(filament::SubpassInfo& subpass); + ~MaterialSubpassInterfaceBlockChunk() = default; + +private: + void flatten(Flattener &) override; + + filament::SubpassInfo& mSubpass; +}; + } // namespace filamat #endif // TNT_FILAMAT_MAT_INTEFFACE_BLOCK_CHUNK_H diff --git a/libs/filamat/src/shaders/CodeGenerator.cpp b/libs/filamat/src/shaders/CodeGenerator.cpp index 46be66e237..9c22daf135 100644 --- a/libs/filamat/src/shaders/CodeGenerator.cpp +++ b/libs/filamat/src/shaders/CodeGenerator.cpp @@ -339,6 +339,29 @@ io::sstream& CodeGenerator::generateSamplers( return out; } +utils::io::sstream& CodeGenerator::generateSubpass(utils::io::sstream& out, + SubpassInfo subpass) const { + if (!subpass.isValid) { + return out; + } + + CString subpassName = + SamplerInterfaceBlock::getUniformName(subpass.block.c_str(), subpass.name.c_str()); + + char const* const typeName = "subpassInput"; + // In our Vulkan backend, subpass inputs always live in descriptor set 2. (ignored for GLES) + char const* const precision = getPrecisionQualifier(subpass.precision, Precision::DEFAULT); + out << "layout(input_attachment_index = " << (int) subpass.attachmentIndex + << ", set = 2, binding = " << (int) subpass.binding + << ") "; + out << "uniform " << precision << " " << typeName << " " << subpassName.c_str(); + out << ";\n"; + + out << "\n"; + + return out; +} + void CodeGenerator::fixupExternalSamplers( std::string& shader, SamplerInterfaceBlock const& sib) noexcept { auto const& infos = sib.getSamplerInfoList(); diff --git a/libs/filamat/src/shaders/CodeGenerator.h b/libs/filamat/src/shaders/CodeGenerator.h index 993420c3e2..159e2edc2b 100644 --- a/libs/filamat/src/shaders/CodeGenerator.h +++ b/libs/filamat/src/shaders/CodeGenerator.h @@ -30,6 +30,7 @@ #include #include #include +#include #include @@ -105,6 +106,10 @@ public: utils::io::sstream& generateSamplers( utils::io::sstream& out, uint8_t firstBinding, const filament::SamplerInterfaceBlock& sib) const; + // generate subpass + utils::io::sstream& generateSubpass(utils::io::sstream& out, + filament::SubpassInfo subpass) const; + // generate material properties getters utils::io::sstream& generateMaterialProperty(utils::io::sstream& out, MaterialBuilder::Property property, bool isSet) const; diff --git a/libs/filamat/src/shaders/MaterialInfo.h b/libs/filamat/src/shaders/MaterialInfo.h index f1066bd95c..17219a4cbe 100644 --- a/libs/filamat/src/shaders/MaterialInfo.h +++ b/libs/filamat/src/shaders/MaterialInfo.h @@ -22,6 +22,7 @@ #include #include #include +#include #include @@ -51,6 +52,7 @@ struct UTILS_PUBLIC MaterialInfo { filament::Shading shading; filament::UniformInterfaceBlock uib; filament::SamplerInterfaceBlock sib; + filament::SubpassInfo subpass; filament::SamplerBindingMap samplerBindings; }; diff --git a/libs/filamat/src/shaders/ShaderGenerator.cpp b/libs/filamat/src/shaders/ShaderGenerator.cpp index c83cd9390f..173f31a14e 100644 --- a/libs/filamat/src/shaders/ShaderGenerator.cpp +++ b/libs/filamat/src/shaders/ShaderGenerator.cpp @@ -535,6 +535,9 @@ std::string ShaderGenerator::createPostProcessFragmentProgram( material.samplerBindings.getBlockOffset(BindingPoints::PER_MATERIAL_INSTANCE), material.sib); + // subpass + cg.generateSubpass(fs, material.subpass); + cg.generateCommon(fs, ShaderType::FRAGMENT); cg.generatePostProcessGetters(fs, ShaderType::FRAGMENT); diff --git a/libs/matdbg/src/CommonWriter.h b/libs/matdbg/src/CommonWriter.h index e8e7f7ddca..b60824aec2 100644 --- a/libs/matdbg/src/CommonWriter.h +++ b/libs/matdbg/src/CommonWriter.h @@ -196,6 +196,13 @@ const char* toString(backend::SamplerType type) noexcept { } } +inline +const char* toString(backend::SubpassType type) noexcept { + switch (type) { + case backend::SubpassType::SUBPASS_INPUT: return "subpassInput"; + } +} + inline const char* toString(backend::Precision precision) noexcept { switch (precision) { diff --git a/libs/matdbg/src/TextWriter.cpp b/libs/matdbg/src/TextWriter.cpp index b3b2f4ce37..a9e760ef2a 100644 --- a/libs/matdbg/src/TextWriter.cpp +++ b/libs/matdbg/src/TextWriter.cpp @@ -39,7 +39,7 @@ namespace filament { namespace matdbg { constexpr int alignment = 32; -constexpr int shortAlignment = 12; +constexpr int shortAlignment = 15; static string arraySizeToString(uint64_t size) { if (size > 1) { @@ -249,6 +249,60 @@ static bool printParametersInfo(ostream& text, const ChunkContainer& container) << endl; } + // Subpasses are optional. + if (container.hasChunk(ChunkType::MaterialSubpass)) { + Unflattener subpasses( + container.getChunkStart(ChunkType::MaterialSubpass), + container.getChunkEnd(ChunkType::MaterialSubpass)); + + CString name; + if (!subpasses.read(&name)) { + return false; + } + + uint64_t subpassCount; + subpasses.read(&subpassCount); + + for (uint64_t i = 0; i < subpassCount; i++) { + CString fieldName; + uint8_t fieldType; + uint8_t fieldFormat; + uint8_t fieldPrecision; + uint8_t attachmentIndex; + uint8_t binding; + + if (!subpasses.read(&fieldName)) { + return false; + } + + if (!subpasses.read(&fieldType)) { + return false; + } + + if (!subpasses.read(&fieldFormat)) + return false; + + if (!subpasses.read(&fieldPrecision)) { + return false; + } + + if (!subpasses.read(&attachmentIndex)) { + return false; + } + + if (!subpasses.read(&binding)) { + return false; + } + + text << " " + << setw(alignment) << fieldName.c_str() + << setw(shortAlignment) << toString(SubpassType(fieldType)) + << setw(shortAlignment) << toString(Precision(fieldPrecision)) + << toString(SamplerFormat(fieldFormat)) + << endl; + } + } + text << endl; return true; diff --git a/tools/matc/src/matc/MaterialCompiler.cpp b/tools/matc/src/matc/MaterialCompiler.cpp index feb3b2df7d..1477d07273 100644 --- a/tools/matc/src/matc/MaterialCompiler.cpp +++ b/tools/matc/src/matc/MaterialCompiler.cpp @@ -188,17 +188,24 @@ static bool reflectParameters(const MaterialBuilder& builder) { const MaterialBuilder::Parameter& parameter = parameters[i]; std::cout << " {" << std::endl; std::cout << R"( "name": ")" << parameter.name.c_str() << "\"," << std::endl; - if (parameter.isSampler) { + if (parameter.isSampler()) { std::cout << R"( "type": ")" << Enums::toString(parameter.samplerType) << "\"," << std::endl; std::cout << R"( "format": ")" << - Enums::toString(parameter.samplerFormat) << "\"," << std::endl; + Enums::toString(parameter.format) << "\"," << std::endl; std::cout << R"( "precision": ")" << - Enums::toString(parameter.samplerPrecision) << "\"" << std::endl; - } else { + Enums::toString(parameter.precision) << "\"" << std::endl; + } else if (parameter.isUniform()) { std::cout << R"( "type": ")" << Enums::toString(parameter.uniformType) << "\"," << std::endl; std::cout << R"( "size": ")" << parameter.size << "\"" << std::endl; + } else if (parameter.isSubpass()) { + std::cout << R"( "type": ")" << + Enums::toString(parameter.subpassType) << "\"," << std::endl; + std::cout << R"( "format": ")" << + Enums::toString(parameter.format) << "\"," << std::endl; + std::cout << R"( "precision": ")" << + Enums::toString(parameter.precision) << "\"" << std::endl; } std::cout << " }"; if (i < count - 1) std::cout << ","; diff --git a/tools/matc/src/matc/ParametersProcessor.cpp b/tools/matc/src/matc/ParametersProcessor.cpp index 361434aa84..6d472725b2 100644 --- a/tools/matc/src/matc/ParametersProcessor.cpp +++ b/tools/matc/src/matc/ParametersProcessor.cpp @@ -203,6 +203,30 @@ static bool processParameter(MaterialBuilder& builder, const JsonishObject& json } else { builder.parameter(type, nameString.c_str()); } + } else if (Enums::isValid(typeString)) { + if (arraySize > 0) { + std::cerr << "parameters: the parameter with name '" << nameString << "'" + << " is an array of subpasses of size " << arraySize << ". Arrays of subpasses" + << " are currently not supported." << std::endl; + return false; + } + + MaterialBuilder::SubpassType type = Enums::toEnum(typeString); + if (precisionValue && formatValue) { + auto format = Enums::toEnum(formatValue->toJsonString()->getString()); + auto precision = + Enums::toEnum(precisionValue->toJsonString()->getString()); + builder.parameter(type, format, precision, nameString.c_str()); + } else if (formatValue) { + auto format = Enums::toEnum(formatValue->toJsonString()->getString()); + builder.parameter(type, format, nameString.c_str()); + } else if (precisionValue) { + auto precision = + Enums::toEnum(precisionValue->toJsonString()->getString()); + builder.parameter(type, precision, nameString.c_str()); + } else { + builder.parameter(type, nameString.c_str()); + } } else { std::cerr << "parameters: the type '" << typeString << "' for parameter with name '" << nameString << "' is neither a valid uniform " @@ -219,7 +243,7 @@ static bool processParameters(MaterialBuilder& builder, const JsonishValue& v) { bool ok = true; for (auto value : jsonArray->getElements()) { if (value->getType() == JsonishValue::Type::OBJECT) { - ok |= processParameter(builder, *value->toJsonObject()); + ok &= processParameter(builder, *value->toJsonObject()); continue; } std::cerr << "parameters must be an array of OBJECTs." << std::endl; @@ -436,7 +460,7 @@ static bool processOutputs(MaterialBuilder& builder, const JsonishValue& v) { bool ok = true; for (auto value : jsonArray->getElements()) { if (value->getType() == JsonishValue::Type::OBJECT) { - ok |= processOutput(builder, *value->toJsonObject()); + ok &= processOutput(builder, *value->toJsonObject()); continue; } std::cerr << "outputs must be an array of OBJECTs." << std::endl; From 7260bab7b3b4f5f197b0202fd3dd6f24c96ac89c Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Sun, 18 Oct 2020 13:05:22 -0700 Subject: [PATCH 03/24] Fix TypeScript binding for TextureUsage. --- web/filament-js/filament.d.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/web/filament-js/filament.d.ts b/web/filament-js/filament.d.ts index 67b5c33a02..85513a3813 100644 --- a/web/filament-js/filament.d.ts +++ b/web/filament-js/filament.d.ts @@ -154,7 +154,7 @@ export class Texture$Builder { public levels(levels: number): Texture$Builder; public sampler(sampler: Texture$Sampler): Texture$Builder; public format(format: Texture$InternalFormat): Texture$Builder; - public usage(usage: Texture$Usage): Texture$Builder; + public usage(usage: number): Texture$Builder; public build(engine: Engine) : Texture; } @@ -895,13 +895,13 @@ export enum Texture$Sampler { // It is a "const enum" which means TypeScript will simply create a constant for each member. // It does not contain the $ delimiter to avoid interference with the embind class. export const enum TextureUsage { - DEFAULT, - COLOR_ATTACHMENT, - DEPTH_ATTACHMENT, - STENCIL_ATTACHMENT, - UPLOADABLE, - SAMPLEABLE, - SUBPASS_INPUT, + COLOR_ATTACHMENT = 1, + DEPTH_ATTACHMENT = 2, + STENCIL_ATTACHMENT = 4, + UPLOADABLE = 8, + SAMPLEABLE = 16, + SUBPASS_INPUT = 32, + DEFAULT = UPLOADABLE | SAMPLEABLE, } export enum Texture$CubemapFace { From 75c4f36f961bad6a7fbf7c08418509560c5c1737 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 7 Oct 2020 20:19:53 +0000 Subject: [PATCH 04/24] Add Dockerfile and test script for SwiftShader. This adds a Dockerfile and a new bash script that makes it east to invoke the appopriate Docker commands. This does not yet enable a GitHub Action because of intermittent issues that we have not yet ironed out. --- .gitignore | 1 + BUILDING.md | 16 ++++ build/swiftshader/Dockerfile | 53 +++++++++++ build/swiftshader/gallery.py | 56 ++++++++++++ build/swiftshader/patch_00.diff | 62 +++++++++++++ build/swiftshader/test.sh | 127 +++++++++++++++++++++++++++ libs/viewer/src/AutomationEngine.cpp | 6 +- libs/viewer/src/AutomationSpec.cpp | 2 +- 8 files changed, 318 insertions(+), 5 deletions(-) create mode 100644 build/swiftshader/Dockerfile create mode 100755 build/swiftshader/gallery.py create mode 100644 build/swiftshader/patch_00.diff create mode 100755 build/swiftshader/test.sh diff --git a/.gitignore b/.gitignore index 91bb94fcc5..196b33a061 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ civetweb.txt settings.json test*.png test*.json +results diff --git a/BUILDING.md b/BUILDING.md index d41123c510..9522e5ae1c 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -468,3 +468,19 @@ export SWIFTSHADER_LD_LIBRARY_PATH=`pwd` ``` Next, go to your Filament repo and use the [easy build](#easy-build) script with `-t`. + +## SwiftShader for CI + +Continuous testing turnaround can be quite slow if you need to build SwiftShader from scratch, so we +provide an Ubuntu-based Docker image that has it already built. The Docker image also includes +everything necessary for building Filament. You can fetch and run the image as follows: + +``` +docker pull ghcr.io/filament-assets/swiftshader +docker run -it ghcr.io/filament-assets/swiftshader +``` + +To do more with the container, see the helper script at `build/swiftshader/test.sh`. + +If you are a team member, you can update the public image to the latest SwiftShader by +following the instructions at the top of `build/swiftshader/Dockerfile`. diff --git a/build/swiftshader/Dockerfile b/build/swiftshader/Dockerfile new file mode 100644 index 0000000000..4c0ce984fd --- /dev/null +++ b/build/swiftshader/Dockerfile @@ -0,0 +1,53 @@ +# Build the image: +# docker build --no-cache --tag ssfilament -f build/swiftshader/Dockerfile . +# docker tag ssfilament ghcr.io/filament-assets/swiftshader +# +# Publish the image: +# docker login ghcr.io --username --password +# docker push ghcr.io/filament-assets/swiftshader +# +# Run the image and mount the current directory: +# docker run -it -v `pwd`:/trees/filament -t ssfilament + +FROM ubuntu:focal +WORKDIR /trees +ARG DEBIAN_FRONTEND=noninteractive +ENV SWIFTSHADER_LD_LIBRARY_PATH=/trees/swiftshader/build +ENV CXXFLAGS='-fno-builtin -Wno-pass-failed' + +RUN apt-get update && \ + apt-get --no-install-recommends install -y \ + apt-transport-https \ + apt-utils \ + build-essential \ + cmake \ + ca-certificates \ + git \ + ninja-build \ + python \ + python3 \ + xorg-dev \ + clang-7 \ + libc++-7-dev \ + libc++abi-7-dev \ + lldb + +# Ensure that clang is used instead of gcc. +RUN set -eux ;\ + update-alternatives --install /usr/bin/clang clang /usr/bin/clang-7 100 ;\ + update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-7 100 ;\ + update-alternatives --install /usr/bin/cc cc /usr/bin/clang 100 ;\ + update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++ 100 + +# Get patch files from the local Filament tree. +COPY build/swiftshader/*.diff . + +# Clone SwiftShader, apply patches, and build it. +RUN set -eux ;\ + git clone https://swiftshader.googlesource.com/SwiftShader swiftshader ;\ + cd swiftshader ;\ + git checkout 139f5c3 ;\ + git apply /trees/*.diff ;\ + cd build ;\ + cmake .. -GNinja -DCMAKE_BUILD_TYPE=Release ;\ + ninja diff --git a/build/swiftshader/gallery.py b/build/swiftshader/gallery.py new file mode 100755 index 0000000000..01c27cafb3 --- /dev/null +++ b/build/swiftshader/gallery.py @@ -0,0 +1,56 @@ +#!/usr/bin/env python3 + +from pathlib import Path +import os + +spath = os.path.dirname(os.path.realpath(__file__)) + +path = Path(spath) + +folder = "../../results/" + +images = list(path.glob(folder + '*.png')) + +images.sort() + +gallery = open(path.absolute().joinpath(folder + 'index.html'), 'w') + +gallery.write(""" + + + + + + + +""") + +tag = '' + +for image in images: + group = image.stem.rstrip('0123456789') + before = f'https://filament-assets.github.io/golden/{group}/{image.name}' + after = image.name + gallery.write('\n') + gallery.write(f'

{image.stem}.json

\n') + gallery.write('\n') + gallery.write(f' \n') + gallery.write('\n') + +gallery.write(""" + +""") diff --git a/build/swiftshader/patch_00.diff b/build/swiftshader/patch_00.diff new file mode 100644 index 0000000000..fbdfd64699 --- /dev/null +++ b/build/swiftshader/patch_00.diff @@ -0,0 +1,62 @@ +diff --git a/src/Vulkan/VkPipeline.cpp b/src/Vulkan/VkPipeline.cpp +index 86913ec72..3b35345af 100644 +--- a/src/Vulkan/VkPipeline.cpp ++++ b/src/Vulkan/VkPipeline.cpp +@@ -71,7 +71,56 @@ std::vector preprocessSpirv( + if(optimize) + { + // Full optimization list taken from spirv-opt. +- opt.RegisterPerformancePasses(); ++ ++ // We have removed CreateRedundancyEliminationPass because it segfaults when encountering: ++ // %389 = OpCompositeConstruct %7 %386 %387 %388 %86 ++ // When inserting an entry into instruction_to_value_ (which is an unordered_map) ++ // This could perhaps be investigated further with help from asan. ++ ++ using namespace spvtools; ++ opt.RegisterPass(CreateWrapOpKillPass()) ++ .RegisterPass(CreateDeadBranchElimPass()) ++ .RegisterPass(CreateMergeReturnPass()) ++ .RegisterPass(CreateInlineExhaustivePass()) ++ .RegisterPass(CreateAggressiveDCEPass()) ++ .RegisterPass(CreatePrivateToLocalPass()) ++ .RegisterPass(CreateLocalSingleBlockLoadStoreElimPass()) ++ .RegisterPass(CreateLocalSingleStoreElimPass()) ++ .RegisterPass(CreateAggressiveDCEPass()) ++ .RegisterPass(CreateScalarReplacementPass()) ++ .RegisterPass(CreateLocalAccessChainConvertPass()) ++ .RegisterPass(CreateLocalSingleBlockLoadStoreElimPass()) ++ .RegisterPass(CreateLocalSingleStoreElimPass()) ++ .RegisterPass(CreateAggressiveDCEPass()) ++ .RegisterPass(CreateLocalMultiStoreElimPass()) ++ .RegisterPass(CreateAggressiveDCEPass()) ++ .RegisterPass(CreateCCPPass()) ++ .RegisterPass(CreateAggressiveDCEPass()) ++ .RegisterPass(CreateLoopUnrollPass(true)) ++ .RegisterPass(CreateDeadBranchElimPass()) ++ .RegisterPass(CreateRedundancyEliminationPass()) // workaround for SEGFAULT ++ .RegisterPass(CreateCombineAccessChainsPass()) ++ .RegisterPass(CreateSimplificationPass()) ++ .RegisterPass(CreateScalarReplacementPass()) ++ .RegisterPass(CreateLocalAccessChainConvertPass()) ++ .RegisterPass(CreateLocalSingleBlockLoadStoreElimPass()) ++ .RegisterPass(CreateLocalSingleStoreElimPass()) ++ .RegisterPass(CreateAggressiveDCEPass()) ++ .RegisterPass(CreateSSARewritePass()) ++ .RegisterPass(CreateAggressiveDCEPass()) ++ .RegisterPass(CreateVectorDCEPass()) ++ .RegisterPass(CreateDeadInsertElimPass()) ++ .RegisterPass(CreateDeadBranchElimPass()) ++ .RegisterPass(CreateSimplificationPass()) ++ .RegisterPass(CreateIfConversionPass()) ++ .RegisterPass(CreateCopyPropagateArraysPass()) ++ .RegisterPass(CreateReduceLoadSizePass()) ++ .RegisterPass(CreateAggressiveDCEPass()) ++ .RegisterPass(CreateBlockMergePass()) ++ .RegisterPass(CreateRedundancyEliminationPass()) // workaround for SEGFAULT ++ .RegisterPass(CreateDeadBranchElimPass()) ++ .RegisterPass(CreateBlockMergePass()) ++ .RegisterPass(CreateSimplificationPass()); + } + + std::vector optimized; diff --git a/build/swiftshader/test.sh b/build/swiftshader/test.sh new file mode 100755 index 0000000000..47320e217f --- /dev/null +++ b/build/swiftshader/test.sh @@ -0,0 +1,127 @@ +#!/bin/bash +set -e + +function print_help { + local self_name=$(basename "$0") + echo "This script issues docker commands for testing Filament with SwiftShader." + echo "The usual sequence of commands is: fetch, start, build filament release, and run." + echo "" + echo "Usage:" + echo " $self_name [command]" + echo "" + echo "Commands:" + echo " build filament [debug | release]" + echo " Use the container to build Filament." + echo " build swiftshader [debug | release]" + echo " Use the container to do a clean rebuild of SwiftShader." + echo " (Note that the container already has SwiftShader built.)" + echo " fetch" + echo " Download the docker image from the central repository." + echo " help" + echo " Print this help message." + echo " logs" + echo " Print messages from the container's kernel ring buffer." + echo " This is useful for diagnosing OOM issues." + echo " run [lldb]" + echo " Launch a test inside the container, optionally via lldb." + echo " shell" + echo " Interact with a bash prompt in the container." + echo " start" + echo " Start a container from the image." + echo " stop" + echo " Stop the container." + echo "" +} + +# Change the current working directory to the Filament root. +pushd "$(dirname "$0")/../.." > /dev/null + +if [[ "$1" == "build" ]] && [[ "$2" == "filament" ]]; then + docker exec runner filament/build.sh -t $3 gltf_viewer + exit $? +fi + +if [[ "$1" == "build" ]] && [[ "$2" == "swiftshader" ]]; then + BUILD_TYPE="$3" + BUILD_TYPE="$(tr '[:lower:]' '[:upper:]' <<< ${BUILD_TYPE:0:1})${BUILD_TYPE:1}" + docker exec --workdir /trees/swiftshader runner rm -rf build + docker exec --workdir /trees/swiftshader runner mkdir build + docker exec --workdir /trees/swiftshader/build runner cmake -GNinja -DCMAKE_BUILD_TYPE="$BUILD_TYPE" .. + docker exec --workdir /trees/swiftshader/build runner ninja + exit $? +fi + +if [[ "$1" == "fetch" ]]; then + docker pull ghcr.io/filament-assets/swiftshader:latest + docker tag ghcr.io/filament-assets/swiftshader:latest ssfilament + exit $? +fi + +if [[ "$1" == "help" ]]; then + print_help + exit 0 +fi + +if [[ "$1" == "logs" ]]; then + docker exec runner dmesg --human --read-clear + exit $? +fi + +if [[ "$1" == "run" ]] && [[ "$2" == "lldb" ]]; then + docker exec -i --workdir /trees/filament/results runner \ + lldb --batch -o run -o bt -- \ + ../out/cmake-release/samples/gltf_viewer \ + --headless \ + --batch ../libs/viewer/tests/basic.json \ + --api vulkan + docker exec runner /trees/filament/build/swiftshader/gallery.py + exit $? +fi + +if [[ "$1" == "run" ]]; then + docker exec --tty --workdir /trees/filament/results runner \ + /usr/bin/catchsegv \ + ../out/cmake-release/samples/gltf_viewer \ + --headless \ + --batch ../libs/viewer/tests/basic.json \ + --api vulkan + docker exec runner /trees/filament/build/swiftshader/gallery.py + exit $? +fi + +if [[ "$1" == "shell" ]]; then + docker exec --interactive --tty runner /bin/bash + exit $? +fi + +# Notes on options being passed to docker's run command: +# +# - The memory constraint seems to prevent an OOM signal in GitHub Actions. +# - The cap / security args allow use of lldb and creation of core dumps. +# - The privileged arg allows use of dmesg for examining OOM logs. +# +# Currently, a GitHub Actions VM has 2 CPUs, 7 GB RAM, and 14 GB of SSD disk space. +# +# Please be aware that Docker Desktop might impose additional resource constraints, and that those +# settings can only be controlled with its GUI. We recommend at least 7 GB of memory and 2 GB swap. +if [[ "$1" == "start" ]]; then + mkdir -p results + docker run --tty --rm --detach --privileged \ + --memory 6.5g \ + --name runner \ + --cap-add=SYS_PTRACE \ + --security-opt seccomp=unconfined \ + --security-opt apparmor=unconfined \ + --volume `pwd`:/trees/filament \ + --workdir /trees \ + ssfilament + exit $? +fi + +if [[ "$1" == "stop" ]]; then + docker container rm runner --force + exit $? +fi + +print_help +exit 1 diff --git a/libs/viewer/src/AutomationEngine.cpp b/libs/viewer/src/AutomationEngine.cpp index 2bc7dcc1c1..87a143be66 100644 --- a/libs/viewer/src/AutomationEngine.cpp +++ b/libs/viewer/src/AutomationEngine.cpp @@ -162,10 +162,8 @@ void AutomationEngine::tick(View* view, MaterialInstance* const* materials, size const int digits = (int) log10 ((double) mSpec->size()) + 1; std::ostringstream stringStream; - stringStream << "test" - << std::setfill('0') << std::setw(digits) - << std::to_string(mCurrentTest) << "_" - << mSpec->getName(mCurrentTest); + stringStream << mSpec->getName(mCurrentTest) + << std::setfill('0') << std::setw(digits) << mCurrentTest; std::string prefix = stringStream.str(); if (mOptions.exportSettings) { diff --git a/libs/viewer/src/AutomationSpec.cpp b/libs/viewer/src/AutomationSpec.cpp index 9717854aa0..d01386b675 100644 --- a/libs/viewer/src/AutomationSpec.cpp +++ b/libs/viewer/src/AutomationSpec.cpp @@ -48,7 +48,7 @@ static const char* DEFAULT_AUTOMATION = R"TXT([ "name": "viewopts", "base": { "view.dof.focusDistance": 0.1 - } + }, "permute": { "view.sampleCount": [1, 4], "view.taa.enabled": [false, true], From 239246e221691de391c5486def17fa9d55769c96 Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Wed, 21 Oct 2020 12:19:46 -0600 Subject: [PATCH 05/24] Add ability to specify post-process material output location (#3205) --- .../colorGrading/colorGradingAsSubpass.mat | 19 +++++++++++++++---- .../filamat/include/filamat/MaterialBuilder.h | 8 +++++--- libs/filamat/src/MaterialBuilder.cpp | 13 +++++++++++-- libs/filamat/src/shaders/ShaderGenerator.cpp | 3 +-- tools/matc/src/matc/ParametersProcessor.cpp | 15 ++++++++++++++- 5 files changed, 46 insertions(+), 12 deletions(-) diff --git a/filament/src/materials/colorGrading/colorGradingAsSubpass.mat b/filament/src/materials/colorGrading/colorGradingAsSubpass.mat index d98f29713f..b44ad79fa8 100644 --- a/filament/src/materials/colorGrading/colorGradingAsSubpass.mat +++ b/filament/src/materials/colorGrading/colorGradingAsSubpass.mat @@ -34,6 +34,20 @@ material { name : colorBuffer, } ], + outputs : [ + { + name : color, + target : color, + type : float4, + location : 0 + }, + { + name : tonemappedOutput, + target : color, + type : float4, + location : 1 + } + ], variables : [ vertex ], @@ -51,9 +65,6 @@ vertex { fragment { - // TODO: specify an output for this at location 1. - layout(location = 1) out vec4 tonemappedOutput; - #include "../../../../shaders/src/dithering.fs" #include "../../../../shaders/src/vignette.fs" @@ -108,8 +119,8 @@ fragment { #else postProcess.color = dithered; #endif + postProcess.tonemappedOutput = postProcess.color; } - tonemappedOutput = postProcess.color; } } diff --git a/libs/filamat/include/filamat/MaterialBuilder.h b/libs/filamat/include/filamat/MaterialBuilder.h index fbfbf0091c..536291b443 100644 --- a/libs/filamat/include/filamat/MaterialBuilder.h +++ b/libs/filamat/include/filamat/MaterialBuilder.h @@ -470,7 +470,7 @@ public: //! Add a new fragment shader output variable. Only valid for materials in the POST_PROCESS domain. MaterialBuilder& output(VariableQualifier qualifier, OutputTarget target, - OutputType type, const char* name) noexcept; + OutputType type, const char* name, int location = -1) noexcept; MaterialBuilder& enableFramebufferFetch() noexcept; @@ -531,13 +531,15 @@ public: struct Output { Output() noexcept = default; Output(const char* outputName, VariableQualifier qualifier, OutputTarget target, - OutputType type) - : name(outputName), qualifier(qualifier), target(target), type(type) { } + OutputType type, int location) noexcept + : name(outputName), qualifier(qualifier), target(target), type(type), + location(location) { } utils::CString name; VariableQualifier qualifier; OutputTarget target; OutputType type; + int location; }; static constexpr size_t MATERIAL_PROPERTIES_COUNT = filament::MATERIAL_PROPERTIES_COUNT; diff --git a/libs/filamat/src/MaterialBuilder.cpp b/libs/filamat/src/MaterialBuilder.cpp index da0548cc0c..15cf3f466d 100644 --- a/libs/filamat/src/MaterialBuilder.cpp +++ b/libs/filamat/src/MaterialBuilder.cpp @@ -761,15 +761,24 @@ bool MaterialBuilder::generateShaders(const std::vector& variants, Chun } MaterialBuilder& MaterialBuilder::output(VariableQualifier qualifier, OutputTarget target, - OutputType type, const char* name) noexcept { + OutputType type, const char* name, int location) noexcept { ASSERT_PRECONDITION(target != OutputTarget::DEPTH || type == OutputType::FLOAT, "Depth outputs must be of type FLOAT."); ASSERT_PRECONDITION(target != OutputTarget::DEPTH || qualifier == VariableQualifier::OUT, "Depth outputs must use OUT qualifier."); + ASSERT_PRECONDITION(location >= -1, + "Output location must be >= 0 (or use -1 for default location)."); + + // A location value of -1 signals using the default location. We'll simply take the previous + // output's location and add 1. + if (location == -1) { + location = mOutputs.empty() ? 0 : mOutputs.back().location + 1; + } + // Unconditionally add this output, then we'll check if we've maxed on on any particular target. - mOutputs.emplace_back(name, qualifier, target, type); + auto& output = mOutputs.emplace_back(name, qualifier, target, type, location); uint8_t colorOutputCount = 0; uint8_t depthOutputCount = 0; diff --git a/libs/filamat/src/shaders/ShaderGenerator.cpp b/libs/filamat/src/shaders/ShaderGenerator.cpp index 173f31a14e..b464b5c7e5 100644 --- a/libs/filamat/src/shaders/ShaderGenerator.cpp +++ b/libs/filamat/src/shaders/ShaderGenerator.cpp @@ -542,10 +542,9 @@ std::string ShaderGenerator::createPostProcessFragmentProgram( cg.generatePostProcessGetters(fs, ShaderType::FRAGMENT); // Generate post-process outputs. - size_t outputIndex = 0; for (const auto& output : mOutputs) { if (output.target == MaterialBuilder::OutputTarget::COLOR) { - cg.generateOutput(fs, ShaderType::FRAGMENT, output.name, outputIndex++, + cg.generateOutput(fs, ShaderType::FRAGMENT, output.name, output.location, output.qualifier, output.type); } if (output.target == MaterialBuilder::OutputTarget::DEPTH) { diff --git a/tools/matc/src/matc/ParametersProcessor.cpp b/tools/matc/src/matc/ParametersProcessor.cpp index 6d472725b2..36d80acd87 100644 --- a/tools/matc/src/matc/ParametersProcessor.cpp +++ b/tools/matc/src/matc/ParametersProcessor.cpp @@ -428,6 +428,14 @@ static bool processOutput(MaterialBuilder& builder, const JsonishObject& jsonObj } } + const JsonishValue* locationValue = jsonObject.getValue("location"); + if (locationValue) { + if (locationValue->getType() != JsonishValue::NUMBER) { + std::cerr << "outputs: location must be a NUMBER." << std::endl; + return false; + } + } + const char* name = nameValue->toJsonString()->getString().c_str(); OutputTarget target = OutputTarget::COLOR; @@ -449,7 +457,12 @@ static bool processOutput(MaterialBuilder& builder, const JsonishObject& jsonObj qualifier = Enums::toEnum(qualifierValue->toJsonString()->getString()); } - builder.output(qualifier, target, type, name); + int location = -1; + if (locationValue) { + location = static_cast(locationValue->toJsonNumber()->getFloat()); + } + + builder.output(qualifier, target, type, name, location); return true; } From 548b28c6e2b092172dd74eed61bfb877a4a42d79 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Mon, 19 Oct 2020 18:24:30 -0700 Subject: [PATCH 06/24] Vulkan: improve the ReadPixels implementation. This adds support for more format conversions and removes a bogus assert that prevented ReadPixels within beginFrame / endFrame. This was tested with: backend_test_mac --api vulkan --gtest_filter=BackendTest.ReadPixels --- filament/backend/src/DataReshaper.h | 128 ++++++++++++++---- filament/backend/src/DriverBase.h | 2 +- filament/backend/src/TextureReshaper.cpp | 3 +- filament/backend/src/vulkan/VulkanContext.cpp | 1 - filament/backend/src/vulkan/VulkanDriver.cpp | 74 ++++------ filament/backend/src/vulkan/VulkanDriver.h | 11 ++ filament/backend/src/vulkan/VulkanUtility.cpp | 96 +++++++++++++ filament/backend/src/vulkan/VulkanUtility.h | 1 + 8 files changed, 245 insertions(+), 71 deletions(-) diff --git a/filament/backend/src/DataReshaper.h b/filament/backend/src/DataReshaper.h index 20e53507bf..2217969f46 100644 --- a/filament/backend/src/DataReshaper.h +++ b/filament/backend/src/DataReshaper.h @@ -24,19 +24,24 @@ namespace filament { namespace backend { -// This little utility adds padding to multi-channel interleaved data by inserting dummy values, or -// discards trailing channels. This is useful for platforms that only accept 4-component data, since -// users often wish to submit (or receive) 3-component data. +// Provides an alpha value when expanding 3-channel images to 4-channel. +// Also used as a normalization scale when converting between numeric types. +template inline componentType getMaxValue(); + class DataReshaper { public: - template::max()> + + // Adds padding to multi-channel interleaved data by inserting dummy values, or discards + // trailing channels. This is useful for platforms that only accept 4-component data, since + // users often wish to submit (or receive) 3-component data. + template static void reshape(void* dest, const void* src, size_t numSrcBytes) { + const componentType maxValue = getMaxValue(); const componentType* in = (const componentType*) src; componentType* out = (componentType*) dest; - const size_t srcWordCount = (numSrcBytes / sizeof(componentType)) / srcChannelCount; + const size_t width = (numSrcBytes / sizeof(componentType)) / srcChannelCount; const int minChannelCount = filament::math::min(srcChannelCount, dstChannelCount); - for (size_t word = 0; word < srcWordCount; ++word) { + for (size_t column = 0; column < width; ++column) { for (size_t channel = 0; channel < minChannelCount; ++channel) { out[channel] = in[channel]; } @@ -48,37 +53,114 @@ public: } } - template::max()> - static void reshapeImage(uint8_t* dest, const uint8_t* src, size_t srcBytesPerRow, - size_t dstBytesPerRow, size_t height, bool swizzle03) { - const size_t srcWordCount = (srcBytesPerRow / sizeof(componentType)) / srcChannelCount; - const int minChannelCount = filament::math::min(srcChannelCount, dstChannelCount); + // Converts a 4-channel image of UBYTE, INT, UINT, or FLOAT to a different type. + template + static void reshapeImage(uint8_t* dest, const uint8_t* src, size_t srcBytesPerRow, + size_t dstBytesPerRow, size_t dstChannelCount, size_t height, bool swizzle, bool flip) { + const size_t srcChannelCount = 4; + const dstComponentType dstMaxValue = getMaxValue(); + const srcComponentType srcMaxValue = getMaxValue(); + const size_t width = (srcBytesPerRow / sizeof(srcComponentType)) / srcChannelCount; + const size_t minChannelCount = filament::math::min(srcChannelCount, dstChannelCount); assert(minChannelCount <= 4); - int inds[4] = {0, 1, 2, 3}; - if (swizzle03) { - inds[0] = 2; - inds[2] = 0; + const int inds[4] = {swizzle ? 2 : 0, 1, swizzle ? 0 : 2, 3}; + + int srcStride; + if (flip) { + src += srcBytesPerRow * (height - 1); + srcStride = -srcBytesPerRow; + } else { + srcStride = srcBytesPerRow; } + for (size_t row = 0; row < height; ++row) { - const componentType* in = (const componentType*) src; - componentType* out = (componentType*) dest; - for (size_t word = 0; word < srcWordCount; ++word) { + const srcComponentType* in = (const srcComponentType*) src; + dstComponentType* out = (dstComponentType*) dest; + for (size_t column = 0; column < width; ++column) { for (size_t channel = 0; channel < minChannelCount; ++channel) { - out[channel] = in[inds[channel]]; + out[channel] = in[inds[channel]] * dstMaxValue / srcMaxValue; } for (size_t channel = srcChannelCount; channel < dstChannelCount; ++channel) { - out[channel] = maxValue; + out[channel] = dstMaxValue; } in += srcChannelCount; out += dstChannelCount; } - src += srcBytesPerRow; + src += srcStride; dest += dstBytesPerRow; } } + + // Converts a 4-channel image of UBYTE, INT, UINT, or FLOAT to a different type. + static bool reshapeImage(PixelBufferDescriptor* dst, PixelDataType srcType, + const uint8_t* srcBytes, int srcBytesPerRow, int width, int height, bool swizzle, + bool flip) { + size_t dstChannelCount; + switch (dst->format) { + case PixelDataFormat::RGB: dstChannelCount = 3; break; + case PixelDataFormat::RGBA: dstChannelCount = 4; break; + default: return false; + } + void (*reshaper)(uint8_t*, const uint8_t*, size_t, size_t, size_t, size_t, bool, bool) + = nullptr; + constexpr auto UBYTE = PixelDataType::UBYTE, FLOAT = PixelDataType::FLOAT, + UINT = PixelDataType::UINT, INT = PixelDataType::INT; + switch (dst->type) { + case UBYTE: + switch (srcType) { + case UBYTE: reshaper = reshapeImage; break; + case FLOAT: reshaper = reshapeImage; break; + case INT: reshaper = reshapeImage; break; + case UINT: reshaper = reshapeImage; break; + default: return false; + } + break; + case FLOAT: + switch (srcType) { + case UBYTE: reshaper = reshapeImage; break; + case FLOAT: reshaper = reshapeImage; break; + case INT: reshaper = reshapeImage; break; + case UINT: reshaper = reshapeImage; break; + default: return false; + } + break; + case INT: + switch (srcType) { + case UBYTE: reshaper = reshapeImage; break; + case FLOAT: reshaper = reshapeImage; break; + case INT: reshaper = reshapeImage; break; + case UINT: reshaper = reshapeImage; break; + default: return false; + } + break; + case UINT: + switch (srcType) { + case UBYTE: reshaper = reshapeImage; break; + case FLOAT: reshaper = reshapeImage; break; + case INT: reshaper = reshapeImage; break; + case UINT: reshaper = reshapeImage; break; + default: return false; + } + break; + default: + return false; + } + uint8_t* dstBytes = (uint8_t*) dst->buffer; + const int dstBytesPerRow = PixelBufferDescriptor::computeDataSize(dst->format, dst->type, + dst->stride ? dst->stride : width, 1, dst->alignment); + reshaper(dstBytes, srcBytes, srcBytesPerRow, dstBytesPerRow, dstChannelCount, height, + swizzle, flip); + return true; + } + }; +template<> inline float getMaxValue() { return 1.0f; } +template<> inline int32_t getMaxValue() { return 0x7fffffff; } +template<> inline uint32_t getMaxValue() { return 0xffffffff; } +template<> inline uint16_t getMaxValue() { return 0x3c00; } // 0x3c00 is 1.0 in half-float. +template<> inline uint8_t getMaxValue() { return 0xff; } + } // namespace backend } // namespace filament diff --git a/filament/backend/src/DriverBase.h b/filament/backend/src/DriverBase.h index 2fd026f8a8..d481cb9191 100644 --- a/filament/backend/src/DriverBase.h +++ b/filament/backend/src/DriverBase.h @@ -167,7 +167,7 @@ public: explicit DriverBase(Dispatcher* dispatcher) noexcept; ~DriverBase() noexcept override; - void purge() noexcept final; + void purge() noexcept override; Dispatcher& getDispatcher() noexcept final { return *mDispatcher; } diff --git a/filament/backend/src/TextureReshaper.cpp b/filament/backend/src/TextureReshaper.cpp index 6549d5de89..08212708e8 100644 --- a/filament/backend/src/TextureReshaper.cpp +++ b/filament/backend/src/TextureReshaper.cpp @@ -38,8 +38,7 @@ TextureReshaper::TextureReshaper(TextureFormat requestedFormat) noexcept { const size_t reshapedSize = p.size / 6 * 8; // reshaping from 6 to 8 bytes per pixel void* reshapeBuffer = malloc(reshapedSize); ASSERT_POSTCONDITION(reshapeBuffer, "Could not allocate memory to reshape pixels."); - // 0x3c00 is 1.0 in 16 bit floating point. - DataReshaper::reshape(reshapeBuffer, p.buffer, p.size); + DataReshaper::reshape(reshapeBuffer, p.buffer, p.size); PixelBufferDescriptor reshaped(reshapeBuffer, reshapedSize, PixelBufferDescriptor::PixelDataFormat::RGBA, diff --git a/filament/backend/src/vulkan/VulkanContext.cpp b/filament/backend/src/vulkan/VulkanContext.cpp index c024ceb0d9..ca428e1950 100644 --- a/filament/backend/src/vulkan/VulkanContext.cpp +++ b/filament/backend/src/vulkan/VulkanContext.cpp @@ -367,7 +367,6 @@ void createSwapChain(VulkanContext& context, VulkanSurfaceContext& surfaceContex for (const VkSurfaceFormatKHR& format : surfaceContext.surfaceFormats) { if (format.format == VK_FORMAT_R8G8B8A8_UNORM) { surfaceContext.surfaceFormat = format; - break; } } const auto compositionCaps = caps.supportedCompositeAlpha; diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 8339307b24..1bd7c58bd6 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -1293,25 +1293,22 @@ void VulkanDriver::stopCapture(int) { } -void VulkanDriver::readPixels(Handle src, - uint32_t x, uint32_t y, uint32_t width, uint32_t height, - PixelBufferDescriptor&& pbd) { - // TODO: add support for all types listed in the Renderer docstring for readPixels. - assert(pbd.type == PixelBufferDescriptor::PixelDataType::UBYTE); - +void VulkanDriver::readPixels(Handle src, uint32_t x, uint32_t y, + uint32_t width, uint32_t height, PixelBufferDescriptor&& pbd) { const VkDevice device = mContext.device; + const VulkanRenderTarget* srcTarget = handle_cast(mHandleMap, src); + const VulkanTexture* srcTexture = srcTarget->getColor(0).texture; + const VkFormat swapChainFormat = mContext.currentSurface->surfaceFormat.format; + const VkFormat srcFormat = srcTexture ? srcTexture->vkformat : swapChainFormat; + const bool swizzle = srcFormat == VK_FORMAT_B8G8R8A8_UNORM; // Create a host visible, linearly tiled image as a staging area. VkImageCreateInfo imageInfo { .sType = VK_STRUCTURE_TYPE_IMAGE_CREATE_INFO, .imageType = VK_IMAGE_TYPE_2D, - .format = VK_FORMAT_R8G8B8A8_UNORM, - .extent = { - .width = width, - .height = height, - .depth = 1, - }, + .format = srcFormat, + .extent = { width, height, 1 }, .mipLevels = 1, .arrayLayers = 1, .samples = VK_SAMPLE_COUNT_1_BIT, @@ -1336,10 +1333,8 @@ void VulkanDriver::readPixels(Handle src, vkAllocateMemory(device, &allocInfo, nullptr, &stagingMemory); vkBindImageMemory(device, stagingImage, stagingMemory, 0); - // TODO: Should we allow readPixels within beginFrame / endFrame? - - assert(mContext.currentCommands == nullptr); - acquireWorkCommandBuffer(mContext); + // TODO: replace waitForIdle with an image barrier coupled with acquireWorkCommandBuffer. + waitForIdle(mContext); // Transition the staging image layout. @@ -1347,9 +1342,12 @@ void VulkanDriver::readPixels(Handle src, VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 0, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT); + const uint8_t srcMipLevel = srcTarget->getColor(0).level; + VkImageCopy imageCopyRegion = { .srcSubresource = { .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT, + .mipLevel = srcMipLevel, .layerCount = 1, }, .srcOffset = { @@ -1369,11 +1367,10 @@ void VulkanDriver::readPixels(Handle src, // Transition the source image layout (which might be the swap chain) - VulkanRenderTarget* srcTarget = handle_cast(mHandleMap, src); VkImage srcImage = srcTarget->getColor(0).image; VulkanTexture::transitionImageLayout(mContext.work.cmdbuffer, srcImage, - VK_IMAGE_LAYOUT_UNDEFINED, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, 0, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT); + VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, srcMipLevel, 1, 1, + VK_IMAGE_ASPECT_COLOR_BIT); // Perform the blit. @@ -1383,16 +1380,15 @@ void VulkanDriver::readPixels(Handle src, // Restore the source image layout. - VulkanTexture* srcTexture = srcTarget->getColor(0).texture; if (srcTexture || mContext.currentSurface->presentQueue) { const VkImageLayout present = VK_IMAGE_LAYOUT_PRESENT_SRC_KHR; VulkanTexture::transitionImageLayout(mContext.work.cmdbuffer, srcImage, VK_IMAGE_LAYOUT_UNDEFINED, srcTexture ? getTextureLayout(srcTexture->usage) : present, - 0, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT); + srcMipLevel, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT); } else { VulkanTexture::transitionImageLayout(mContext.work.cmdbuffer, srcImage, VK_IMAGE_LAYOUT_UNDEFINED, VK_IMAGE_LAYOUT_GENERAL, - 0, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT); + srcMipLevel, 1, 1, VK_IMAGE_ASPECT_COLOR_BIT); } // Transition the staging image layout to GENERAL. @@ -1440,28 +1436,18 @@ void VulkanDriver::readPixels(Handle src, vkMapMemory(device, stagingMemory, 0, VK_WHOLE_SIZE, 0, (void**) &srcPixels); srcPixels += subResourceLayout.offset; - uint8_t* dstPixels = (uint8_t*) closure->buffer; - const uint32_t dstStride = closure->stride ? closure->stride : width; - const int dstBytesPerRow = PixelBufferDescriptor::computeDataSize(closure->format, - closure->type, dstStride, 1, closure->alignment); - const int srcBytesPerRow = subResourceLayout.rowPitch; - const VkFormat swapChainFormat = mContext.currentSurface->surfaceFormat.format; - const bool swizzle = !srcTexture && swapChainFormat == VK_FORMAT_B8G8R8A8_UNORM; - - switch (closure->format) { - case PixelDataFormat::RGB: - case PixelDataFormat::RGB_INTEGER: - DataReshaper::reshapeImage(dstPixels, srcPixels, srcBytesPerRow, - dstBytesPerRow, height, swizzle); - break; - case PixelDataFormat::RGBA: - case PixelDataFormat::RGBA_INTEGER: - DataReshaper::reshapeImage(dstPixels, srcPixels, srcBytesPerRow, - dstBytesPerRow, height, swizzle); - break; - default: - utils::slog.e << "ReadPixels: invalid PixelDataFormat" << utils::io::endl; - break; + // TODO: investigate why this Y-flip conditional exists. At least two SwiftShader-based + // tests (viewer_basic_test.cc and gltf_viewer batch mode) seem to require "false". However + // test_ReadPixels.cpp with MoltenVK requires "true" to be consistent with OpenGL and Metal. + // One hypothesis is that this is due to the layout of the SwiftShader backbuffer. + #ifdef FILAMENT_USE_SWIFTSHADER + constexpr bool flipY = false; + #else + constexpr bool flipY = true; + #endif + if (!DataReshaper::reshapeImage(closure, getComponentType(srcFormat), srcPixels, + subResourceLayout.rowPitch, width, height, swizzle, flipY)) { + utils::slog.e << "Unsupported PixelDataFormat or PixelDataType" << utils::io::endl; } vkUnmapMemory(device, stagingMemory); diff --git a/filament/backend/src/vulkan/VulkanDriver.h b/filament/backend/src/vulkan/VulkanDriver.h index ecba991476..ad6acb80ce 100644 --- a/filament/backend/src/vulkan/VulkanDriver.h +++ b/filament/backend/src/vulkan/VulkanDriver.h @@ -77,6 +77,17 @@ private: VulkanDriver(VulkanDriver const&) = delete; VulkanDriver& operator = (VulkanDriver const&) = delete; + void purge() noexcept override { + // First we trigger garbage collection of Vulkan resources. This ensures that transient + // resources (e.g. the ReadPixels staging buffer) are removed if their refcount is 0, which + // allows related BufferDescriptors to move to the purge list. + mDisposer.gc(); + + // Next, allow the base class to clean up the purge list in order to trigger the + // BufferDescriptor destructors, which in turn triggers the user-provided callbacks. + DriverBase::purge(); + } + private: backend::VulkanPlatform& mContextManager; diff --git a/filament/backend/src/vulkan/VulkanUtility.cpp b/filament/backend/src/vulkan/VulkanUtility.cpp index 2a4d27dfc6..615de8b730 100644 --- a/filament/backend/src/vulkan/VulkanUtility.cpp +++ b/filament/backend/src/vulkan/VulkanUtility.cpp @@ -281,5 +281,101 @@ VkFrontFace getFrontFace(bool inverseFrontFaces) { VkFrontFace::VK_FRONT_FACE_CLOCKWISE : VkFrontFace::VK_FRONT_FACE_COUNTER_CLOCKWISE; } +PixelDataType getComponentType(VkFormat format) { + switch (format) { + case VK_FORMAT_R8_UNORM: + case VK_FORMAT_R8_SNORM: + case VK_FORMAT_R8_USCALED: + case VK_FORMAT_R8_SSCALED: + case VK_FORMAT_R8_UINT: return PixelDataType::UBYTE; + case VK_FORMAT_R8_SINT: return PixelDataType::BYTE; + case VK_FORMAT_R8_SRGB: + case VK_FORMAT_R8G8_UNORM: + case VK_FORMAT_R8G8_SNORM: + case VK_FORMAT_R8G8_USCALED: + case VK_FORMAT_R8G8_SSCALED: + case VK_FORMAT_R8G8_UINT: return PixelDataType::UBYTE; + case VK_FORMAT_R8G8_SINT: return PixelDataType::BYTE; + case VK_FORMAT_R8G8_SRGB: + case VK_FORMAT_R8G8B8_UNORM: + case VK_FORMAT_R8G8B8_SNORM: + case VK_FORMAT_R8G8B8_USCALED: + case VK_FORMAT_R8G8B8_SSCALED: + case VK_FORMAT_R8G8B8_UINT: return PixelDataType::UBYTE; + case VK_FORMAT_R8G8B8_SINT: return PixelDataType::BYTE; + case VK_FORMAT_R8G8B8_SRGB: + case VK_FORMAT_B8G8R8_UNORM: return PixelDataType::UBYTE; + case VK_FORMAT_B8G8R8_SNORM: return PixelDataType::BYTE; + case VK_FORMAT_B8G8R8_USCALED: + case VK_FORMAT_B8G8R8_SSCALED: + case VK_FORMAT_B8G8R8_UINT: return PixelDataType::UBYTE; + case VK_FORMAT_B8G8R8_SINT: return PixelDataType::BYTE; + case VK_FORMAT_B8G8R8_SRGB: + case VK_FORMAT_R8G8B8A8_UNORM: + case VK_FORMAT_R8G8B8A8_SNORM: + case VK_FORMAT_R8G8B8A8_USCALED: + case VK_FORMAT_R8G8B8A8_SSCALED: + case VK_FORMAT_R8G8B8A8_UINT: return PixelDataType::UBYTE; + case VK_FORMAT_R8G8B8A8_SINT: return PixelDataType::BYTE; + case VK_FORMAT_R8G8B8A8_SRGB: + case VK_FORMAT_B8G8R8A8_UNORM: + case VK_FORMAT_B8G8R8A8_SNORM: + case VK_FORMAT_B8G8R8A8_USCALED: + case VK_FORMAT_B8G8R8A8_SSCALED: + case VK_FORMAT_B8G8R8A8_UINT: return PixelDataType::UBYTE; + case VK_FORMAT_B8G8R8A8_SINT: return PixelDataType::BYTE; + case VK_FORMAT_B8G8R8A8_SRGB: + case VK_FORMAT_A8B8G8R8_UNORM_PACK32: + case VK_FORMAT_A8B8G8R8_SNORM_PACK32: + case VK_FORMAT_A8B8G8R8_USCALED_PACK32: + case VK_FORMAT_A8B8G8R8_SSCALED_PACK32: + case VK_FORMAT_A8B8G8R8_UINT_PACK32: return PixelDataType::UBYTE; + case VK_FORMAT_A8B8G8R8_SINT_PACK32: return PixelDataType::BYTE; + case VK_FORMAT_A8B8G8R8_SRGB_PACK32: return PixelDataType::UBYTE; + case VK_FORMAT_R16_UNORM: + case VK_FORMAT_R16_SNORM: + case VK_FORMAT_R16_USCALED: + case VK_FORMAT_R16_SSCALED: + case VK_FORMAT_R16_UINT: return PixelDataType::USHORT; + case VK_FORMAT_R16_SINT: return PixelDataType::SHORT; + case VK_FORMAT_R16_SFLOAT: return PixelDataType::HALF; + case VK_FORMAT_R16G16_UNORM: + case VK_FORMAT_R16G16_SNORM: + case VK_FORMAT_R16G16_USCALED: + case VK_FORMAT_R16G16_SSCALED: + case VK_FORMAT_R16G16_UINT: return PixelDataType::USHORT; + case VK_FORMAT_R16G16_SINT: return PixelDataType::SHORT; + case VK_FORMAT_R16G16_SFLOAT: return PixelDataType::HALF; + case VK_FORMAT_R16G16B16_UNORM: + case VK_FORMAT_R16G16B16_SNORM: + case VK_FORMAT_R16G16B16_USCALED: + case VK_FORMAT_R16G16B16_SSCALED: + case VK_FORMAT_R16G16B16_UINT: return PixelDataType::USHORT; + case VK_FORMAT_R16G16B16_SINT: return PixelDataType::SHORT; + case VK_FORMAT_R16G16B16_SFLOAT: return PixelDataType::HALF; + case VK_FORMAT_R16G16B16A16_UNORM: + case VK_FORMAT_R16G16B16A16_SNORM: + case VK_FORMAT_R16G16B16A16_USCALED: + case VK_FORMAT_R16G16B16A16_SSCALED: + case VK_FORMAT_R16G16B16A16_UINT: return PixelDataType::USHORT; + case VK_FORMAT_R16G16B16A16_SINT: return PixelDataType::SHORT; + case VK_FORMAT_R16G16B16A16_SFLOAT: return PixelDataType::HALF; + case VK_FORMAT_R32_UINT: return PixelDataType::UINT; + case VK_FORMAT_R32_SINT: return PixelDataType::INT; + case VK_FORMAT_R32_SFLOAT: return PixelDataType::FLOAT; + case VK_FORMAT_R32G32_UINT: return PixelDataType::UINT; + case VK_FORMAT_R32G32_SINT: return PixelDataType::INT; + case VK_FORMAT_R32G32_SFLOAT: return PixelDataType::FLOAT; + case VK_FORMAT_R32G32B32_UINT: return PixelDataType::UINT; + case VK_FORMAT_R32G32B32_SINT: return PixelDataType::INT; + case VK_FORMAT_R32G32B32_SFLOAT: return PixelDataType::FLOAT; + case VK_FORMAT_R32G32B32A32_UINT: return PixelDataType::UINT; + case VK_FORMAT_R32G32B32A32_SINT: return PixelDataType::INT; + case VK_FORMAT_R32G32B32A32_SFLOAT: return PixelDataType::FLOAT; + default: assert(false && "Unknown data type, conversion is not supported."); + } + return {}; +} + } // namespace filament } // namespace backend diff --git a/filament/backend/src/vulkan/VulkanUtility.h b/filament/backend/src/vulkan/VulkanUtility.h index 1cff95b5da..d7f3765d63 100644 --- a/filament/backend/src/vulkan/VulkanUtility.h +++ b/filament/backend/src/vulkan/VulkanUtility.h @@ -32,6 +32,7 @@ VkCompareOp getCompareOp(SamplerCompareFunc func); VkBlendFactor getBlendFactor(BlendFunction mode); VkCullModeFlags getCullMode(CullingMode mode); VkFrontFace getFrontFace(bool inverseFrontFaces); +PixelDataType getComponentType(VkFormat format); } // namespace filament } // namespace backend From 5f6cbc779a182012b7a5d3fcb395f9be1cad7e51 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 21 Oct 2020 16:24:27 -0700 Subject: [PATCH 07/24] Allow test_ReadPixels with SwiftShader. --- filament/backend/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/backend/CMakeLists.txt b/filament/backend/CMakeLists.txt index 0743b9fde2..8c1bf68110 100644 --- a/filament/backend/CMakeLists.txt +++ b/filament/backend/CMakeLists.txt @@ -337,7 +337,7 @@ if (APPLE) endif() endif() -if (APPLE AND NOT IOS AND NOT FILAMENT_USE_SWIFTSHADER) +if (APPLE AND NOT IOS) add_executable(backend_test_mac test/mac_runner.mm) target_link_libraries(backend_test_mac PRIVATE "-framework Metal -framework AppKit -framework QuartzCore") # Because each test case is a separate file, the -force_load flag is necessary to prevent the From 853247f32fa63c9679b2b9e5bdbc4e61ccc6280a Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 21 Oct 2020 17:41:31 -0700 Subject: [PATCH 08/24] Vulkan: ReadPixels y-flip workaround. --- filament/backend/src/vulkan/VulkanDriver.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 1bd7c58bd6..3e4a00438d 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -1436,15 +1436,8 @@ void VulkanDriver::readPixels(Handle src, uint32_t x, uint32_t y vkMapMemory(device, stagingMemory, 0, VK_WHOLE_SIZE, 0, (void**) &srcPixels); srcPixels += subResourceLayout.offset; - // TODO: investigate why this Y-flip conditional exists. At least two SwiftShader-based - // tests (viewer_basic_test.cc and gltf_viewer batch mode) seem to require "false". However - // test_ReadPixels.cpp with MoltenVK requires "true" to be consistent with OpenGL and Metal. - // One hypothesis is that this is due to the layout of the SwiftShader backbuffer. - #ifdef FILAMENT_USE_SWIFTSHADER - constexpr bool flipY = false; - #else + // TODO: investigate why this Y-flip exists. constexpr bool flipY = true; - #endif if (!DataReshaper::reshapeImage(closure, getComponentType(srcFormat), srcPixels, subResourceLayout.rowPitch, width, height, swizzle, flipY)) { utils::slog.e << "Unsupported PixelDataFormat or PixelDataType" << utils::io::endl; From 4533a42a6f4eed6cd3b140b6d92be14131dfdbb7 Mon Sep 17 00:00:00 2001 From: dsternfeld7 Date: Thu, 22 Oct 2020 11:27:29 -0700 Subject: [PATCH 09/24] Makes it possible to have multiple instances of ImGuiHelper at the same time This change fixes an issue where ImGuiHelper would crash on destruction if you created more than one instance at the same time. This crash occurred when ImGui::DestroyContext() was called, because when the second instance was destroyed there was no current context causing it to crash. This fixes it by making ImGuiHelper store and manage it's own context. Having two instances of ImGuiHelper is useful in cases where you want to use imgui with multiple views. --- libs/filagui/include/filagui/ImGuiHelper.h | 2 ++ libs/filagui/src/ImGuiHelper.cpp | 10 +++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/libs/filagui/include/filagui/ImGuiHelper.h b/libs/filagui/include/filagui/ImGuiHelper.h index 5a20ec67da..aa436c2930 100644 --- a/libs/filagui/include/filagui/ImGuiHelper.h +++ b/libs/filagui/include/filagui/ImGuiHelper.h @@ -33,6 +33,7 @@ struct ImDrawData; struct ImGuiIO; +struct ImGuiContext; namespace filagui { @@ -87,6 +88,7 @@ public: utils::Entity mRenderable; filament::Texture* mTexture = nullptr; bool mHasSynced = false; + ImGuiContext* mImGuiContext; }; } // namespace filagui diff --git a/libs/filagui/src/ImGuiHelper.cpp b/libs/filagui/src/ImGuiHelper.cpp index 8aecd808f9..e45f679f5f 100644 --- a/libs/filagui/src/ImGuiHelper.cpp +++ b/libs/filagui/src/ImGuiHelper.cpp @@ -43,8 +43,8 @@ namespace filagui { #include "generated/resources/filagui_resources.h" ImGuiHelper::ImGuiHelper(Engine* engine, filament::View* view, const Path& fontPath) : - mEngine(engine), mView(view), mScene(engine->createScene()) { - ImGui::CreateContext(); + mEngine(engine), mView(view), mScene(engine->createScene()), + mImGuiContext(ImGui::CreateContext()) { ImGuiIO& io = ImGui::GetIO(); // Create a simple alpha-blended 2D blitting material. @@ -111,7 +111,8 @@ ImGuiHelper::~ImGuiHelper() { for (auto& ib : mIndexBuffers) { mEngine->destroy(ib); } - ImGui::DestroyContext(); + ImGui::DestroyContext(mImGuiContext); + mImGuiContext = nullptr; } void ImGuiHelper::setDisplaySize(int width, int height, float scaleX, float scaleY) { @@ -122,6 +123,7 @@ void ImGuiHelper::setDisplaySize(int width, int height, float scaleX, float scal } void ImGuiHelper::render(float timeStepInSeconds, Callback imguiCommands) { + ImGui::SetCurrentContext(mImGuiContext); ImGuiIO& io = ImGui::GetIO(); io.DeltaTime = timeStepInSeconds; // First, let ImGui process events and increment its internal frame count. @@ -137,6 +139,8 @@ void ImGuiHelper::render(float timeStepInSeconds, Callback imguiCommands) { } void ImGuiHelper::processImGuiCommands(ImDrawData* commands, const ImGuiIO& io) { + ImGui::SetCurrentContext(mImGuiContext); + mHasSynced = false; auto& rcm = mEngine->getRenderableManager(); From d47189536f7e7833937aed6f26942371792668a0 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 22 Oct 2020 11:09:51 -0700 Subject: [PATCH 10/24] Vulkan: fix thread safety regression in Disposer. This fixes a regression introduced by 548b28c6e manifesting as intermittent assertions. The VulkanDisposer gc() should only be called from the driver thread. --- filament/backend/src/DriverBase.h | 2 +- filament/backend/src/vulkan/VulkanDisposer.h | 2 +- filament/backend/src/vulkan/VulkanDriver.h | 11 ----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/filament/backend/src/DriverBase.h b/filament/backend/src/DriverBase.h index d481cb9191..2fd026f8a8 100644 --- a/filament/backend/src/DriverBase.h +++ b/filament/backend/src/DriverBase.h @@ -167,7 +167,7 @@ public: explicit DriverBase(Dispatcher* dispatcher) noexcept; ~DriverBase() noexcept override; - void purge() noexcept override; + void purge() noexcept final; Dispatcher& getDispatcher() noexcept final { return *mDispatcher; } diff --git a/filament/backend/src/vulkan/VulkanDisposer.h b/filament/backend/src/vulkan/VulkanDisposer.h index 8f73777e10..c2d6b0e3bc 100644 --- a/filament/backend/src/vulkan/VulkanDisposer.h +++ b/filament/backend/src/vulkan/VulkanDisposer.h @@ -61,7 +61,7 @@ public: private: struct Disposable { - size_t refcount = 1; + ssize_t refcount = 1; std::function destructor; }; tsl::robin_map mDisposables; diff --git a/filament/backend/src/vulkan/VulkanDriver.h b/filament/backend/src/vulkan/VulkanDriver.h index ad6acb80ce..ecba991476 100644 --- a/filament/backend/src/vulkan/VulkanDriver.h +++ b/filament/backend/src/vulkan/VulkanDriver.h @@ -77,17 +77,6 @@ private: VulkanDriver(VulkanDriver const&) = delete; VulkanDriver& operator = (VulkanDriver const&) = delete; - void purge() noexcept override { - // First we trigger garbage collection of Vulkan resources. This ensures that transient - // resources (e.g. the ReadPixels staging buffer) are removed if their refcount is 0, which - // allows related BufferDescriptors to move to the purge list. - mDisposer.gc(); - - // Next, allow the base class to clean up the purge list in order to trigger the - // BufferDescriptor destructors, which in turn triggers the user-provided callbacks. - DriverBase::purge(); - } - private: backend::VulkanPlatform& mContextManager; From 47b1ece804ccb908962079ffd19c61351bcae182 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 22 Oct 2020 09:27:17 -0700 Subject: [PATCH 11/24] Vulkan: Improve refcounting for vertex buffers. This avoids the following validation error when clients create then immediately destroy vertex buffers, index buffers, and uniform buffers. VUID-vkDestroyBuffer-buffer-00922 Cannot free VkBuffer that is in use by a command buffer. Buffers that are used across multiple frames were fine, but create-and-destroy scenarios were problematic. --- filament/backend/src/vulkan/VulkanBuffer.cpp | 5 ++++- filament/backend/src/vulkan/VulkanBuffer.h | 6 ++++-- filament/backend/src/vulkan/VulkanDriver.cpp | 16 +++++++--------- filament/backend/src/vulkan/VulkanHandles.cpp | 13 +++++++------ filament/backend/src/vulkan/VulkanHandles.h | 17 +++++++++-------- 5 files changed, 31 insertions(+), 26 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanBuffer.cpp b/filament/backend/src/vulkan/VulkanBuffer.cpp index 74aad5f593..b74a2e218f 100644 --- a/filament/backend/src/vulkan/VulkanBuffer.cpp +++ b/filament/backend/src/vulkan/VulkanBuffer.cpp @@ -22,7 +22,9 @@ namespace filament { namespace backend { VulkanBuffer::VulkanBuffer(VulkanContext& context, VulkanStagePool& stagePool, - VkBufferUsageFlags usage, uint32_t numBytes) : mContext(context), mStagePool(stagePool) { + VulkanDisposer& disposer, VulkanDisposer::Key key, VkBufferUsageFlags usage, + uint32_t numBytes) : mContext(context), mStagePool(stagePool), mDisposer(disposer), + mDisposerKey(key) { // Create the VkBuffer. VkBufferCreateInfo bufferInfo { .sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO, @@ -51,6 +53,7 @@ void VulkanBuffer::loadFromCpu(const void* cpuData, uint32_t byteOffset, uint32_ auto copyToDevice = [this, numBytes, stage] (VulkanCommandBuffer& commands) { VkBufferCopy region { .size = numBytes }; vkCmdCopyBuffer(commands.cmdbuffer, stage->buffer, mGpuBuffer, 1, ®ion); + mDisposer.acquire(mDisposerKey, commands.resources); // Ensure that the copy finishes before the next draw call. VkBufferMemoryBarrier barrier { diff --git a/filament/backend/src/vulkan/VulkanBuffer.h b/filament/backend/src/vulkan/VulkanBuffer.h index e76521a891..ca166c5829 100644 --- a/filament/backend/src/vulkan/VulkanBuffer.h +++ b/filament/backend/src/vulkan/VulkanBuffer.h @@ -26,14 +26,16 @@ namespace backend { // Encapsulates a Vulkan buffer, its attached DeviceMemory and a staging area. class VulkanBuffer { public: - VulkanBuffer(VulkanContext& context, VulkanStagePool& stagePool, VkBufferUsageFlags usage, - uint32_t numBytes); + VulkanBuffer(VulkanContext& context, VulkanStagePool& stagePool, VulkanDisposer& disposer, + VulkanDisposer::Key mDisposerKey, VkBufferUsageFlags usage, uint32_t numBytes); ~VulkanBuffer(); void loadFromCpu(const void* cpuData, uint32_t byteOffset, uint32_t numBytes); VkBuffer getGpuBuffer() const { return mGpuBuffer; } private: VulkanContext& mContext; VulkanStagePool& mStagePool; + VulkanDisposer& mDisposer; + VulkanDisposer::Key mDisposerKey; VmaAllocation mGpuMemory = VK_NULL_HANDLE; VkBuffer mGpuBuffer = VK_NULL_HANDLE; }; diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 3e4a00438d..f87c6c5c20 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -379,7 +379,7 @@ void VulkanDriver::createSamplerGroupR(Handle sbh, size_t count) void VulkanDriver::createUniformBufferR(Handle ubh, size_t size, BufferUsage usage) { auto uniformBuffer = construct_handle(mHandleMap, ubh, mContext, - mStagePool, size, usage); + mStagePool, mDisposer, size, usage); mDisposer.createDisposable(uniformBuffer, [this, ubh] () { destruct_handle(mHandleMap, ubh); }); @@ -394,16 +394,12 @@ void VulkanDriver::destroyUniformBuffer(Handle ubh) { } void VulkanDriver::createRenderPrimitiveR(Handle rph, int) { - auto renderPrimitive = construct_handle(mHandleMap, rph, mContext); - mDisposer.createDisposable(renderPrimitive, [this, rph] () { - destruct_handle(mHandleMap, rph); - }); + construct_handle(mHandleMap, rph, mContext); } void VulkanDriver::destroyRenderPrimitive(Handle rph) { if (rph) { - auto renderPrimitive = handle_cast(mHandleMap, rph); - mDisposer.removeReference(renderPrimitive); + destruct_handle(mHandleMap, rph); } } @@ -411,7 +407,7 @@ void VulkanDriver::createVertexBufferR(Handle vbh, uint8_t buffe uint8_t attributeCount, uint32_t elementCount, AttributeArray attributes, BufferUsage usage) { auto vertexBuffer = construct_handle(mHandleMap, vbh, mContext, mStagePool, - bufferCount, attributeCount, elementCount, attributes); + mDisposer, bufferCount, attributeCount, elementCount, attributes); mDisposer.createDisposable(vertexBuffer, [this, vbh] () { destruct_handle(mHandleMap, vbh); }); @@ -428,7 +424,7 @@ void VulkanDriver::createIndexBufferR(Handle ibh, ElementType elementType, uint32_t indexCount, BufferUsage usage) { auto elementSize = (uint8_t) getElementTypeSize(elementType); auto indexBuffer = construct_handle(mHandleMap, ibh, mContext, mStagePool, - elementSize, indexCount); + mDisposer, elementSize, indexCount); mDisposer.createDisposable(indexBuffer, [this, ibh] () { destruct_handle(mHandleMap, ibh); }); @@ -1579,6 +1575,8 @@ void VulkanDriver::draw(PipelineState pipelineState, Handle r auto* program = handle_cast(mHandleMap, programHandle); mDisposer.acquire(program, commands->resources); + mDisposer.acquire(prim.indexBuffer, commands->resources); + mDisposer.acquire(prim.vertexBuffer, commands->resources); // If this is a debug build, validate the current shader. #if !defined(NDEBUG) diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index 2df78b52f9..976cd53f3a 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -396,8 +396,8 @@ bool VulkanRenderTarget::invalidate() { } VulkanVertexBuffer::VulkanVertexBuffer(VulkanContext& context, VulkanStagePool& stagePool, - uint8_t bufferCount, uint8_t attributeCount, uint32_t elementCount, - AttributeArray const& attributes) : + VulkanDisposer& disposer, uint8_t bufferCount, uint8_t attributeCount, + uint32_t elementCount, AttributeArray const& attributes) : HwVertexBuffer(bufferCount, attributeCount, elementCount, attributes) { buffers.reserve(bufferCount); for (uint8_t bufferIndex = 0; bufferIndex < bufferCount; ++bufferIndex) { @@ -408,14 +408,14 @@ VulkanVertexBuffer::VulkanVertexBuffer(VulkanContext& context, VulkanStagePool& size = std::max(size, end); } } - buffers.emplace_back(new VulkanBuffer(context, stagePool, VK_BUFFER_USAGE_VERTEX_BUFFER_BIT, - size)); + buffers.emplace_back(new VulkanBuffer(context, stagePool, disposer, this, + VK_BUFFER_USAGE_VERTEX_BUFFER_BIT, size)); } } VulkanUniformBuffer::VulkanUniformBuffer(VulkanContext& context, VulkanStagePool& stagePool, - uint32_t numBytes, backend::BufferUsage usage) - : mContext(context), mStagePool(stagePool) { + VulkanDisposer& disposer, uint32_t numBytes, backend::BufferUsage usage) + : mContext(context), mStagePool(stagePool), mDisposer(disposer) { // Create the VkBuffer. VkBufferCreateInfo bufferInfo { .sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO, @@ -439,6 +439,7 @@ void VulkanUniformBuffer::loadFromCpu(const void* cpuData, uint32_t numBytes) { auto copyToDevice = [this, numBytes, stage] (VulkanCommandBuffer& commands) { VkBufferCopy region { .size = numBytes }; vkCmdCopyBuffer(commands.cmdbuffer, stage->buffer, mGpuBuffer, 1, ®ion); + mDisposer.acquire(this, commands.resources); // Ensure that the copy finishes before the next draw call. VkBufferMemoryBarrier barrier { diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index 966337fec1..adbd9aea26 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -78,31 +78,32 @@ struct VulkanSwapChain : public HwSwapChain { }; struct VulkanVertexBuffer : public HwVertexBuffer { - VulkanVertexBuffer(VulkanContext& context, VulkanStagePool& stagePool, uint8_t bufferCount, - uint8_t attributeCount, uint32_t elementCount, + VulkanVertexBuffer(VulkanContext& context, VulkanStagePool& stagePool, VulkanDisposer& disposer, + uint8_t bufferCount, uint8_t attributeCount, uint32_t elementCount, AttributeArray const& attributes); std::vector> buffers; }; struct VulkanIndexBuffer : public HwIndexBuffer { - VulkanIndexBuffer(VulkanContext& context, VulkanStagePool& stagePool, uint8_t elementSize, - uint32_t indexCount) : HwIndexBuffer(elementSize, indexCount), + VulkanIndexBuffer(VulkanContext& context, VulkanStagePool& stagePool, VulkanDisposer& disposer, + uint8_t elementSize, uint32_t indexCount) : HwIndexBuffer(elementSize, indexCount), indexType(elementSize == 2 ? VK_INDEX_TYPE_UINT16 : VK_INDEX_TYPE_UINT32), - buffer(new VulkanBuffer(context, stagePool, VK_BUFFER_USAGE_INDEX_BUFFER_BIT, - elementSize * indexCount)) {} + buffer(new VulkanBuffer(context, stagePool, disposer, this, + VK_BUFFER_USAGE_INDEX_BUFFER_BIT, elementSize * indexCount)) {} const VkIndexType indexType; const std::unique_ptr buffer; }; struct VulkanUniformBuffer : public HwUniformBuffer { - VulkanUniformBuffer(VulkanContext& context, VulkanStagePool& stagePool, uint32_t numBytes, - backend::BufferUsage usage); + VulkanUniformBuffer(VulkanContext& context, VulkanStagePool& stagePool, + VulkanDisposer& disposer, uint32_t numBytes, backend::BufferUsage usage); ~VulkanUniformBuffer(); void loadFromCpu(const void* cpuData, uint32_t numBytes); VkBuffer getGpuBuffer() const { return mGpuBuffer; } private: VulkanContext& mContext; VulkanStagePool& mStagePool; + VulkanDisposer& mDisposer; VkBuffer mGpuBuffer; VmaAllocation mGpuMemory; }; From 8de19cb5825abd401ddac271d82381f3f44a1f30 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 22 Oct 2020 09:39:30 -0700 Subject: [PATCH 12/24] Vulkan: fix leak with headless swap chain. --- filament/backend/src/vulkan/VulkanContext.cpp | 23 ++++++++++++------- filament/backend/src/vulkan/VulkanHandles.cpp | 2 +- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanContext.cpp b/filament/backend/src/vulkan/VulkanContext.cpp index ca428e1950..16fbc746e4 100644 --- a/filament/backend/src/vulkan/VulkanContext.cpp +++ b/filament/backend/src/vulkan/VulkanContext.cpp @@ -474,9 +474,10 @@ void createSwapChain(VulkanContext& context, VulkanSurfaceContext& surfaceContex void destroySwapChain(VulkanContext& context, VulkanSurfaceContext& surfaceContext, VulkanDisposer& disposer) { waitForIdle(context); + const VkDevice device = context.device; for (SwapContext& swapContext : surfaceContext.swapContexts) { disposer.release(swapContext.commands.resources); - vkFreeCommandBuffers(context.device, context.commandPool, 1, + vkFreeCommandBuffers(device, context.commandPool, 1, &swapContext.commands.cmdbuffer); // The wrapper object for the submission fence has shared ownership semantics, so here @@ -487,17 +488,23 @@ void destroySwapChain(VulkanContext& context, VulkanSurfaceContext& surfaceConte swapContext.commands.fence.reset(); } - vkDestroyImageView(context.device, swapContext.attachment.view, VKALLOC); + // If this is headless, then we own the image and need to explicitly destroy it. + if (!surfaceContext.swapchain) { + vkDestroyImage(device, swapContext.attachment.image, VKALLOC); + vkFreeMemory(device, swapContext.attachment.memory, VKALLOC); + } + + vkDestroyImageView(device, swapContext.attachment.view, VKALLOC); swapContext.commands.fence = VK_NULL_HANDLE; swapContext.attachment.view = VK_NULL_HANDLE; } - vkDestroySwapchainKHR(context.device, surfaceContext.swapchain, VKALLOC); - vkDestroySemaphore(context.device, surfaceContext.imageAvailable, VKALLOC); - vkDestroySemaphore(context.device, surfaceContext.renderingFinished, VKALLOC); + vkDestroySwapchainKHR(device, surfaceContext.swapchain, VKALLOC); + vkDestroySemaphore(device, surfaceContext.imageAvailable, VKALLOC); + vkDestroySemaphore(device, surfaceContext.renderingFinished, VKALLOC); - vkDestroyImageView(context.device, surfaceContext.depth.view, VKALLOC); - vkDestroyImage(context.device, surfaceContext.depth.image, VKALLOC); - vkFreeMemory(context.device, surfaceContext.depth.memory, VKALLOC); + vkDestroyImageView(device, surfaceContext.depth.view, VKALLOC); + vkDestroyImage(device, surfaceContext.depth.image, VKALLOC); + vkFreeMemory(device, surfaceContext.depth.memory, VKALLOC); } // makeSwapChainPresentable() diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index 976cd53f3a..d7d0946ffc 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -261,7 +261,7 @@ VulkanSwapChain::VulkanSwapChain(VulkanContext& context, uint32_t width, uint32_ surfaceContext.swapContexts[i].attachment = { .format = surfaceContext.surfaceFormat.format, .image = image, - .view = {}, .memory = {}, .texture = {}, .layout = VK_IMAGE_LAYOUT_GENERAL + .view = {}, .memory = imageMemory, .texture = {}, .layout = VK_IMAGE_LAYOUT_GENERAL }; VkImageViewCreateInfo ivCreateInfo = { .sType = VK_STRUCTURE_TYPE_IMAGE_VIEW_CREATE_INFO, From 06e639ae91d4ab3a5c5b2ec123d5585a0738562b Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 22 Oct 2020 10:32:15 -0700 Subject: [PATCH 13/24] Vulkan: fix destroy-while-used for UBOs. --- filament/backend/src/vulkan/VulkanDriver.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index f87c6c5c20..8c032195cf 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -389,6 +389,15 @@ void VulkanDriver::destroyUniformBuffer(Handle ubh) { if (ubh) { auto buffer = handle_cast(mHandleMap, ubh); mBinder.unbindUniformBuffer(buffer->getGpuBuffer()); + + // We do not know if any pending draw calls are making use of this uniform buffer, + // so assume the worst: that all command buffers are all using it. + if (mContext.currentSurface) { + for (auto& swapContext : mContext.currentSurface->swapContexts) { + mDisposer.acquire(buffer, swapContext.commands.resources); + } + } + mDisposer.removeReference(buffer); } } From f55c9ee4377bd9c26388e10ce77001160ee178cc Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 22 Oct 2020 13:08:09 -0700 Subject: [PATCH 14/24] Fix Windows build. --- filament/backend/src/vulkan/VulkanDisposer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/backend/src/vulkan/VulkanDisposer.h b/filament/backend/src/vulkan/VulkanDisposer.h index c2d6b0e3bc..86972e15b9 100644 --- a/filament/backend/src/vulkan/VulkanDisposer.h +++ b/filament/backend/src/vulkan/VulkanDisposer.h @@ -61,7 +61,7 @@ public: private: struct Disposable { - ssize_t refcount = 1; + int refcount = 1; std::function destructor; }; tsl::robin_map mDisposables; From 2e10fe6680ccd384b78d41d95b757741dacfd7cb Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 22 Oct 2020 13:18:33 -0700 Subject: [PATCH 15/24] PlatformVkLinux now supports all combos of XLIB and XCB. You can now build Filament with support for both X11 APIs, or neither. If both are supported, run-time selection is achieved using a SwapChain flag. Supporting only one API at build time (or neither) is useful because our list of "required" VkInstance extensions can vary according to which API's are supported. During VkInstance creation, we do not have a priori knowledge about what kinds of swap chains will be created. (headless vs non-headless, XCB vs XLIB, etc) Note that some Vulkan implementation (e.g. some builds of SwiftShader) only support XCB. --- CMakeLists.txt | 13 ++++ .../google/android/filament/SwapChain.java | 6 ++ .../backend/include/backend/DriverEnums.h | 1 + .../backend/src/vulkan/PlatformVkAndroid.cpp | 2 +- .../backend/src/vulkan/PlatformVkAndroid.h | 2 +- filament/backend/src/vulkan/PlatformVkCocoa.h | 2 +- .../backend/src/vulkan/PlatformVkCocoa.mm | 2 +- .../backend/src/vulkan/PlatformVkCocoaTouch.h | 2 +- .../src/vulkan/PlatformVkCocoaTouch.mm | 2 +- .../backend/src/vulkan/PlatformVkLinux.cpp | 64 ++++++++++++------- filament/backend/src/vulkan/PlatformVkLinux.h | 9 ++- .../backend/src/vulkan/PlatformVkWindows.cpp | 2 +- .../backend/src/vulkan/PlatformVkWindows.h | 2 +- filament/backend/src/vulkan/VulkanDriver.cpp | 3 +- filament/backend/src/vulkan/VulkanPlatform.h | 2 +- filament/include/filament/SwapChain.h | 6 ++ libs/bluevk/include/vulkan/vk_platform.h | 3 +- 17 files changed, 87 insertions(+), 36 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 11345d35e5..7dd3c71ba8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,6 +21,10 @@ option(FILAMENT_ENABLE_LTO "Enable link-time optimizations if supported by the c option(FILAMENT_SKIP_SAMPLES "Don't build samples" OFF) +option(FILAMENT_SUPPORTS_XCB "Include XCB support in Linux builds" ON) + +option(FILAMENT_SUPPORTS_XLIB "Include XLIB support in Linux builds" ON) + set(FILAMENT_PER_RENDER_PASS_ARENA_SIZE_IN_MB "2" CACHE STRING "Per render pass arena size. Must be roughly 1 MB larger than FILAMENT_PER_FRAME_COMMANDS_SIZE_IN_MB, default 2." ) @@ -84,6 +88,15 @@ if (UNIX AND NOT APPLE AND NOT ANDROID AND NOT WEBGL) endif() if (LINUX) + + if (FILAMENT_SUPPORTS_XCB) + add_definitions(-DFILAMENT_SUPPORTS_XCB) + endif() + + if (FILAMENT_SUPPORTS_XLIB) + add_definitions(-DFILAMENT_SUPPORTS_XLIB) + endif() + execute_process(COMMAND uname -p OUTPUT_VARIABLE PROCESSOR_ARCH OUTPUT_STRIP_TRAILING_WHITESPACE diff --git a/android/filament-android/src/main/java/com/google/android/filament/SwapChain.java b/android/filament-android/src/main/java/com/google/android/filament/SwapChain.java index 3f16a81172..96821515fc 100644 --- a/android/filament-android/src/main/java/com/google/android/filament/SwapChain.java +++ b/android/filament-android/src/main/java/com/google/android/filament/SwapChain.java @@ -85,6 +85,12 @@ public class SwapChain { */ public static final long CONFIG_READABLE = 0x2; + /** + * Indicates that the native X11 window is an XCB window rather than an XLIB window. + * This is ignored on non-Linux platforms and in builds that support only one X11 API. + */ + public static final long CONFIG_ENABLE_XCB = 0x4; + SwapChain(long nativeSwapChain, Object surface) { mNativeObject = nativeSwapChain; mSurface = surface; diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index cb17bdf010..b951756650 100644 --- a/filament/backend/include/backend/DriverEnums.h +++ b/filament/backend/include/backend/DriverEnums.h @@ -41,6 +41,7 @@ namespace backend { static constexpr uint64_t SWAP_CHAIN_CONFIG_TRANSPARENT = 0x1; static constexpr uint64_t SWAP_CHAIN_CONFIG_READABLE = 0x2; +static constexpr uint64_t SWAP_CHAIN_CONFIG_ENABLE_XCB = 0x4; static constexpr size_t MAX_VERTEX_ATTRIBUTE_COUNT = 16; // This is guaranteed by OpenGL ES. static constexpr size_t MAX_SAMPLER_COUNT = 16; // Matches the Adreno Vulkan driver. diff --git a/filament/backend/src/vulkan/PlatformVkAndroid.cpp b/filament/backend/src/vulkan/PlatformVkAndroid.cpp index 273e32dc31..409199232a 100644 --- a/filament/backend/src/vulkan/PlatformVkAndroid.cpp +++ b/filament/backend/src/vulkan/PlatformVkAndroid.cpp @@ -47,7 +47,7 @@ Driver* PlatformVkAndroid::createDriver(void* const sharedContext) noexcept { sizeof(requiredInstanceExtensions) / sizeof(requiredInstanceExtensions[0])); } -void* PlatformVkAndroid::createVkSurfaceKHR(void* nativeWindow, void* vkinstance) noexcept { +void* PlatformVkAndroid::createVkSurfaceKHR(void* nativeWindow, void* vkinstance, uint64_t flags) noexcept { const VkInstance instance = (VkInstance) vkinstance; ANativeWindow* aNativeWindow = (ANativeWindow*) nativeWindow; VkAndroidSurfaceCreateInfoKHR createInfo { diff --git a/filament/backend/src/vulkan/PlatformVkAndroid.h b/filament/backend/src/vulkan/PlatformVkAndroid.h index c8fa312740..a7ee6de47c 100644 --- a/filament/backend/src/vulkan/PlatformVkAndroid.h +++ b/filament/backend/src/vulkan/PlatformVkAndroid.h @@ -29,7 +29,7 @@ public: backend::Driver* createDriver(void* const sharedContext) noexcept override; - void* createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept override; + void* createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept override; int getOSVersion() const noexcept override { return 0; } }; diff --git a/filament/backend/src/vulkan/PlatformVkCocoa.h b/filament/backend/src/vulkan/PlatformVkCocoa.h index 3309e2121e..ea5956e050 100644 --- a/filament/backend/src/vulkan/PlatformVkCocoa.h +++ b/filament/backend/src/vulkan/PlatformVkCocoa.h @@ -27,7 +27,7 @@ namespace filament { class PlatformVkCocoa final : public backend::VulkanPlatform { public: backend::Driver* createDriver(void* sharedContext) noexcept override; - void* createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept override; + void* createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept override; int getOSVersion() const noexcept override { return 0; } }; diff --git a/filament/backend/src/vulkan/PlatformVkCocoa.mm b/filament/backend/src/vulkan/PlatformVkCocoa.mm index 91a3ff9118..b488df2117 100644 --- a/filament/backend/src/vulkan/PlatformVkCocoa.mm +++ b/filament/backend/src/vulkan/PlatformVkCocoa.mm @@ -51,7 +51,7 @@ Driver* PlatformVkCocoa::createDriver(void* sharedContext) noexcept { sizeof(requiredInstanceExtensions) / sizeof(requiredInstanceExtensions[0])); } -void* PlatformVkCocoa::createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept { +void* PlatformVkCocoa::createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept { // Obtain the CAMetalLayer-backed view. NSView* nsview = (__bridge NSView*) nativeWindow; ASSERT_POSTCONDITION(nsview, "Unable to obtain Metal-backed NSView."); diff --git a/filament/backend/src/vulkan/PlatformVkCocoaTouch.h b/filament/backend/src/vulkan/PlatformVkCocoaTouch.h index d2242b6276..b4be989420 100644 --- a/filament/backend/src/vulkan/PlatformVkCocoaTouch.h +++ b/filament/backend/src/vulkan/PlatformVkCocoaTouch.h @@ -27,7 +27,7 @@ namespace filament { class PlatformVkCocoaTouch final : public backend::VulkanPlatform { public: backend::Driver* createDriver(void* const sharedContext) noexcept override; - void* createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept override; + void* createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept override; int getOSVersion() const noexcept override { return 0; } }; diff --git a/filament/backend/src/vulkan/PlatformVkCocoaTouch.mm b/filament/backend/src/vulkan/PlatformVkCocoaTouch.mm index f73ac70ece..d32c2a6872 100644 --- a/filament/backend/src/vulkan/PlatformVkCocoaTouch.mm +++ b/filament/backend/src/vulkan/PlatformVkCocoaTouch.mm @@ -46,7 +46,7 @@ Driver* PlatformVkCocoaTouch::createDriver(void* const sharedContext) noexcept { sizeof(requestedExtensions) / sizeof(requestedExtensions[0])); } -void* PlatformVkCocoaTouch::createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept { +void* PlatformVkCocoaTouch::createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept { #if METAL_AVAILABLE CAMetalLayer* metalLayer = (CAMetalLayer*) nativeWindow; diff --git a/filament/backend/src/vulkan/PlatformVkLinux.cpp b/filament/backend/src/vulkan/PlatformVkLinux.cpp index 94e89b3e5a..f1d61f7ea8 100644 --- a/filament/backend/src/vulkan/PlatformVkLinux.cpp +++ b/filament/backend/src/vulkan/PlatformVkLinux.cpp @@ -36,14 +36,18 @@ static constexpr const char* LIBRARY_X11 = "libX11.so.6"; #ifdef FILAMENT_SUPPORTS_XCB typedef xcb_connection_t* (*XCB_CONNECT)(const char *displayname, int *screenp); -#else +#endif + +#ifdef FILAMENT_SUPPORTS_XLIB typedef Display* (*X11_OPEN_DISPLAY)(const char*); #endif struct X11Functions { #ifdef FILAMENT_SUPPORTS_XCB XCB_CONNECT xcbConnect; -#else +#endif + +#ifdef FILAMENT_SUPPORTS_XLIB X11_OPEN_DISPLAY openDisplay; #endif void* library = nullptr; @@ -55,7 +59,8 @@ Driver* PlatformVkLinux::createDriver(void* const sharedContext) noexcept { "VK_KHR_surface", #ifdef FILAMENT_SUPPORTS_XCB "VK_KHR_xcb_surface", -#else +#endif +#ifdef FILAMENT_SUPPORTS_XLIB "VK_KHR_xlib_surface", #endif "VK_KHR_get_physical_device_properties2", @@ -67,42 +72,57 @@ Driver* PlatformVkLinux::createDriver(void* const sharedContext) noexcept { sizeof(requiredInstanceExtensions) / sizeof(requiredInstanceExtensions[0])); } -void* PlatformVkLinux::createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept { -#ifdef FILAMENT_SUPPORTS_XCB +void* PlatformVkLinux::createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept { if (g_x11.library == nullptr) { g_x11.library = dlopen(LIBRARY_X11, RTLD_LOCAL | RTLD_NOW); ASSERT_PRECONDITION(g_x11.library, "Unable to open X11 library."); + +#ifdef FILAMENT_SUPPORTS_XCB g_x11.xcbConnect = (XCB_CONNECT) dlsym(g_x11.library, "xcb_connect"); int screen; mConnection = g_x11.xcbConnect(nullptr, &screen); - } - ASSERT_POSTCONDITION(vkCreateXcbSurfaceKHR, "Unable to load vkCreateXcbSurfaceKHR function."); - VkSurfaceKHR surface = nullptr; - const uint64_t ptrval = reinterpret_cast(nativeWindow); - VkXcbSurfaceCreateInfoKHR createInfo = { - .sType = VK_STRUCTURE_TYPE_XCB_SURFACE_CREATE_INFO_KHR, - .connection = mConnection, - .window = (xcb_window_t) ptrval, - }; - VkResult result = vkCreateXcbSurfaceKHR((VkInstance) instance, &createInfo, VKALLOC, &surface); -#else - if (g_x11.library == nullptr) { - g_x11.library = dlopen(LIBRARY_X11, RTLD_LOCAL | RTLD_NOW); - ASSERT_PRECONDITION(g_x11.library, "Unable to open X11 library."); + ASSERT_POSTCONDITION(vkCreateXcbSurfaceKHR, "Unable to load vkCreateXcbSurfaceKHR function."); +#endif + +#ifdef FILAMENT_SUPPORTS_XLIB g_x11.openDisplay = (X11_OPEN_DISPLAY) dlsym(g_x11.library, "XOpenDisplay"); mDisplay = g_x11.openDisplay(NULL); ASSERT_PRECONDITION(mDisplay, "Unable to open X11 display."); + ASSERT_POSTCONDITION(vkCreateXlibSurfaceKHR, "Unable to load vkCreateXlibSurfaceKHR function."); +#endif + } - ASSERT_POSTCONDITION(vkCreateXlibSurfaceKHR, "Unable to load vkCreateXlibSurfaceKHR function."); + VkSurfaceKHR surface = nullptr; + +#ifdef FILAMENT_SUPPORTS_XCB +#ifdef FILAMENT_SUPPORTS_XLIB +const bool windowIsXCB = flags & SWAP_CHAIN_CONFIG_ENABLE_XCB; +#else +const bool windowIsXCB = true; +#endif + + if (windowIsXCB) { + const uint64_t ptrval = reinterpret_cast(nativeWindow); + VkXcbSurfaceCreateInfoKHR createInfo = { + .sType = VK_STRUCTURE_TYPE_XCB_SURFACE_CREATE_INFO_KHR, + .connection = mConnection, + .window = (xcb_window_t) ptrval, + }; + vkCreateXcbSurfaceKHR((VkInstance) instance, &createInfo, VKALLOC, &surface); + return surface; + } +#endif + +#ifdef FILAMENT_SUPPORTS_XLIB VkXlibSurfaceCreateInfoKHR createInfo = { .sType = VK_STRUCTURE_TYPE_XLIB_SURFACE_CREATE_INFO_KHR, .dpy = mDisplay, .window = (Window) nativeWindow, }; - VkResult result = vkCreateXlibSurfaceKHR((VkInstance) instance, &createInfo, VKALLOC, &surface); + vkCreateXlibSurfaceKHR((VkInstance) instance, &createInfo, VKALLOC, &surface); #endif - ASSERT_POSTCONDITION(result == VK_SUCCESS, "vkCreateXlibSurfaceKHR error."); + return surface; } diff --git a/filament/backend/src/vulkan/PlatformVkLinux.h b/filament/backend/src/vulkan/PlatformVkLinux.h index c112ecd628..8dde97bc73 100644 --- a/filament/backend/src/vulkan/PlatformVkLinux.h +++ b/filament/backend/src/vulkan/PlatformVkLinux.h @@ -24,7 +24,9 @@ #ifdef FILAMENT_SUPPORTS_XCB #include -#else +#endif + +#ifdef FILAMENT_SUPPORTS_XLIB #include #endif @@ -35,14 +37,15 @@ public: backend::Driver* createDriver(void* const sharedContext) noexcept override; - void* createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept override; + void* createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept override; int getOSVersion() const noexcept override { return 0; } private: #ifdef FILAMENT_SUPPORTS_XCB xcb_connection_t* mConnection; -#else +#endif +#ifdef FILAMENT_SUPPORTS_XLIB Display* mDisplay; #endif }; diff --git a/filament/backend/src/vulkan/PlatformVkWindows.cpp b/filament/backend/src/vulkan/PlatformVkWindows.cpp index e5a168304f..a38b05075d 100644 --- a/filament/backend/src/vulkan/PlatformVkWindows.cpp +++ b/filament/backend/src/vulkan/PlatformVkWindows.cpp @@ -40,7 +40,7 @@ Driver* PlatformVkWindows::createDriver(void* const sharedContext) noexcept { sizeof(requiredInstanceExtensions) / sizeof(requiredInstanceExtensions[0])); } -void* PlatformVkWindows::createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept { +void* PlatformVkWindows::createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept { VkSurfaceKHR surface = nullptr; HWND window = (HWND) nativeWindow; diff --git a/filament/backend/src/vulkan/PlatformVkWindows.h b/filament/backend/src/vulkan/PlatformVkWindows.h index dfc7f84493..335bda8327 100644 --- a/filament/backend/src/vulkan/PlatformVkWindows.h +++ b/filament/backend/src/vulkan/PlatformVkWindows.h @@ -29,7 +29,7 @@ public: backend::Driver* createDriver(void* const sharedContext) noexcept override; - void* createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept override; + void* createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept override; int getOSVersion() const noexcept override { return 0; } diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 8c032195cf..419246b81c 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -559,7 +559,8 @@ void VulkanDriver::createSyncR(Handle sh, int) { void VulkanDriver::createSwapChainR(Handle sch, void* nativeWindow, uint64_t flags) { const VkInstance instance = mContext.instance; - auto vksurface = (VkSurfaceKHR) mContextManager.createVkSurfaceKHR(nativeWindow, instance); + auto vksurface = (VkSurfaceKHR) mContextManager.createVkSurfaceKHR(nativeWindow, instance, + flags); auto* swapChain = construct_handle(mHandleMap, sch, mContext, vksurface); // TODO: move the following line into makeCurrent. diff --git a/filament/backend/src/vulkan/VulkanPlatform.h b/filament/backend/src/vulkan/VulkanPlatform.h index daa32da03f..0d560dcffa 100644 --- a/filament/backend/src/vulkan/VulkanPlatform.h +++ b/filament/backend/src/vulkan/VulkanPlatform.h @@ -43,7 +43,7 @@ namespace backend { class VulkanPlatform : public DefaultPlatform { public: // Given a Vulkan instance and native window handle, creates the platform-specific surface. - virtual void* createVkSurfaceKHR(void* nativeWindow, void* instance) noexcept = 0; + virtual void* createVkSurfaceKHR(void* nativeWindow, void* instance, uint64_t flags) noexcept = 0; ~VulkanPlatform() override; }; diff --git a/filament/include/filament/SwapChain.h b/filament/include/filament/SwapChain.h index 61470c0c5d..6ebd331134 100644 --- a/filament/include/filament/SwapChain.h +++ b/filament/include/filament/SwapChain.h @@ -153,6 +153,12 @@ public: */ static const uint64_t CONFIG_READABLE = backend::SWAP_CHAIN_CONFIG_READABLE; + /** + * Indicates that the native X11 window is an XCB window rather than an XLIB window. + * This is ignored on non-Linux platforms and in builds that support only one X11 API. + */ + static const uint64_t CONFIG_ENABLE_XCB = backend::SWAP_CHAIN_CONFIG_ENABLE_XCB; + void* getNativeWindow() const noexcept; }; diff --git a/libs/bluevk/include/vulkan/vk_platform.h b/libs/bluevk/include/vulkan/vk_platform.h index 4bfd1f0c1a..e8dc718721 100644 --- a/libs/bluevk/include/vulkan/vk_platform.h +++ b/libs/bluevk/include/vulkan/vk_platform.h @@ -28,7 +28,8 @@ #elif defined(__linux__) #if defined(FILAMENT_SUPPORTS_XCB) #define VK_USE_PLATFORM_XCB_KHR 1 -#else +#endif +#if defined(FILAMENT_SUPPORTS_XLIB) #define VK_USE_PLATFORM_XLIB_KHR 1 #endif #elif defined(__APPLE__) From 5746532b7cd0c5ec9f297ccd6eac5de158a7fafd Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Tue, 20 Oct 2020 14:26:54 -0700 Subject: [PATCH 16/24] Add PNG output to test_ReadPixels. --- filament/backend/CMakeLists.txt | 4 +++ filament/backend/test/test_ReadPixels.cpp | 44 +++++++++++++++++++++-- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/filament/backend/CMakeLists.txt b/filament/backend/CMakeLists.txt index 8c1bf68110..a4fd099661 100644 --- a/filament/backend/CMakeLists.txt +++ b/filament/backend/CMakeLists.txt @@ -307,6 +307,8 @@ if (APPLE) filabridge getopt gtest + image + imageio SPIRV spirv-cross-glsl) @@ -321,6 +323,8 @@ if (APPLE) getopt gtest glslang + image + imageio spirv-cross-core spirv-cross-glsl spirv-cross-msl diff --git a/filament/backend/test/test_ReadPixels.cpp b/filament/backend/test/test_ReadPixels.cpp index de8172c6f8..57a79b3644 100644 --- a/filament/backend/test/test_ReadPixels.cpp +++ b/filament/backend/test/test_ReadPixels.cpp @@ -16,6 +16,10 @@ #include "BackendTest.h" +#include + +#include + #include "ShaderGenerator.h" #include "TrianglePrimitive.h" @@ -23,8 +27,27 @@ #include +using namespace filament; +using namespace filament::backend; +using namespace image; + namespace { +template +static LinearImage toLinear(size_t w, size_t h, size_t bpr, const uint8_t* src) { + LinearImage result(w, h, 4); + math::float4* d = reinterpret_cast(result.getPixelRef(0, 0)); + for (size_t y = 0; y < h; ++y) { + T const* p = reinterpret_cast(src + y * bpr); + for (size_t x = 0; x < w; ++x, p += 4) { + math::float3 sRGB(p[0], p[1], p[2]); + sRGB /= std::numeric_limits::max(); + *d++ = math::float4(sRGBToLinear(sRGB), 1.0f); + } + } + return result; +} + //////////////////////////////////////////////////////////////////////////////////////////////////// // Shaders //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -52,9 +75,6 @@ void main() { namespace test { -using namespace filament; -using namespace filament::backend; - TEST_F(BackendTest, ReadPixels) { // These test scenarios use a known hash of the result pixel buffer to decide pass / fail, // asserting an exact pixel-for-pixel match. So far, rendering on macOS and iPhone have had @@ -117,6 +137,21 @@ TEST_F(BackendTest, ReadPixels) { return bufferDimension; } + void exportScreenshot(void* pixelData) const { + const size_t width = readRect.width, height = readRect.height; + LinearImage image(width, height, 4); + if (format == PixelDataFormat::RGBA && type == PixelDataType::UBYTE) { + image = toLinear(width, height, width * 4, (uint8_t*) pixelData); + } + if (format == PixelDataFormat::RGBA && type == PixelDataType::FLOAT) { + memcpy(image.getPixelRef(), pixelData, width * height * sizeof(math::float4)); + } + std::string png = std::string(testName) + ".png"; + std::ofstream outputStream(png.c_str(), std::ios::binary | std::ios::trunc); + ImageEncoder::encode(outputStream, ImageEncoder::Format::PNG, image, "", + png.c_str()); + } + PixelDataFormat format = PixelDataFormat::RGBA; PixelDataType type = PixelDataType::UBYTE; }; @@ -256,6 +291,8 @@ TEST_F(BackendTest, ReadPixels) { const TestCase* test = (const TestCase*) user; assert(test); + test->exportScreenshot(buffer); + // Hash the contents of the buffer and check that they match. uint32_t hash = utils::hash::murmur3((const uint32_t*) buffer, size / 4, 0); @@ -264,6 +301,7 @@ TEST_F(BackendTest, ReadPixels) { free(buffer); }, (void*) &t); + getDriverApi().readPixels(renderTarget, t.readRect.x, t.readRect.y, t.readRect.width, t.readRect.height, std::move(descriptor)); From d84bb82e442d57abe65d83c31f56b323e6afb4df Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Tue, 20 Oct 2020 21:28:10 -0700 Subject: [PATCH 17/24] Merge duplicated implementations of toLinear(). --- filament/backend/test/test_ReadPixels.cpp | 15 --------------- libs/image/include/image/ColorTransform.h | 15 +++++++++++++++ libs/viewer/src/AutomationEngine.cpp | 15 --------------- samples/frame_generator.cpp | 15 --------------- 4 files changed, 15 insertions(+), 45 deletions(-) diff --git a/filament/backend/test/test_ReadPixels.cpp b/filament/backend/test/test_ReadPixels.cpp index 57a79b3644..cdf9a38a52 100644 --- a/filament/backend/test/test_ReadPixels.cpp +++ b/filament/backend/test/test_ReadPixels.cpp @@ -33,21 +33,6 @@ using namespace image; namespace { -template -static LinearImage toLinear(size_t w, size_t h, size_t bpr, const uint8_t* src) { - LinearImage result(w, h, 4); - math::float4* d = reinterpret_cast(result.getPixelRef(0, 0)); - for (size_t y = 0; y < h; ++y) { - T const* p = reinterpret_cast(src + y * bpr); - for (size_t x = 0; x < w; ++x, p += 4) { - math::float3 sRGB(p[0], p[1], p[2]); - sRGB /= std::numeric_limits::max(); - *d++ = math::float4(sRGBToLinear(sRGB), 1.0f); - } - } - return result; -} - //////////////////////////////////////////////////////////////////////////////////////////////////// // Shaders //////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/libs/image/include/image/ColorTransform.h b/libs/image/include/image/ColorTransform.h index 15e2658f02..61ebaed86a 100644 --- a/libs/image/include/image/ColorTransform.h +++ b/libs/image/include/image/ColorTransform.h @@ -346,6 +346,21 @@ inline LinearImage fromLinearToRGBM(const LinearImage& image) { return result; } +template +static LinearImage toLinear(size_t w, size_t h, size_t bpr, const uint8_t* src) { + LinearImage result(w, h, 4); + filament::math::float4* d = reinterpret_cast(result.getPixelRef(0, 0)); + for (size_t y = 0; y < h; ++y) { + T const* p = reinterpret_cast(src + y * bpr); + for (size_t x = 0; x < w; ++x, p += 4) { + filament::math::float3 sRGB(p[0], p[1], p[2]); + sRGB /= std::numeric_limits::max(); + *d++ = filament::math::float4(sRGBToLinear(sRGB), 1.0f); + } + } + return result; +} + } // namespace Image #endif // IMAGE_COLORTRANSFORM_H_ diff --git a/libs/viewer/src/AutomationEngine.cpp b/libs/viewer/src/AutomationEngine.cpp index 87a143be66..b314b04b2c 100644 --- a/libs/viewer/src/AutomationEngine.cpp +++ b/libs/viewer/src/AutomationEngine.cpp @@ -40,21 +40,6 @@ namespace viewer { static std::string gStatus; -template -static LinearImage toLinear(size_t w, size_t h, size_t bpr, const uint8_t* src) { - LinearImage result(w, h, 3); - filament::math::float3* d = reinterpret_cast(result.getPixelRef(0, 0)); - for (size_t y = 0; y < h; ++y) { - T const* p = reinterpret_cast(src + y * bpr); - for (size_t x = 0; x < w; ++x, p += 3) { - filament::math::float3 sRGB(p[0], p[1], p[2]); - sRGB /= std::numeric_limits::max(); - *d++ = sRGBToLinear(sRGB); - } - } - return result; -} - struct ScreenshotState { View* view; std::string filename; diff --git a/samples/frame_generator.cpp b/samples/frame_generator.cpp index 09335a5ec7..a6a2164fbc 100644 --- a/samples/frame_generator.cpp +++ b/samples/frame_generator.cpp @@ -345,21 +345,6 @@ static void setup(Engine* engine, View* view, Scene* scene) { } } -template -static LinearImage toLinear(size_t w, size_t h, size_t bpr, const uint8_t* src) { - LinearImage result(w, h, 3); - filament::math::float3* d = reinterpret_cast(result.getPixelRef(0, 0)); - for (size_t y = 0; y < h; ++y) { - T const* p = reinterpret_cast(src + y * bpr); - for (size_t x = 0; x < w; ++x, p += 3) { - filament::math::float3 sRGB(p[0], p[1], p[2]); - sRGB /= std::numeric_limits::max(); - *d++ = sRGBToLinear(sRGB); - } - } - return result; -} - static void render(Engine*, View*, Scene*, Renderer*) { int frame = g_currentFrame - FRAME_TO_SKIP - 1; if (frame >= 0 && frame < g_materialVariantCount) { From 2849dc6a2adf1cff016b3cf73a57fcac1eaf3787 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 21 Oct 2020 18:38:52 -0700 Subject: [PATCH 18/24] Fix iOS build break. --- filament/backend/CMakeLists.txt | 9 +++++---- filament/backend/test/test_ReadPixels.cpp | 12 ++++++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/filament/backend/CMakeLists.txt b/filament/backend/CMakeLists.txt index a4fd099661..8221845337 100644 --- a/filament/backend/CMakeLists.txt +++ b/filament/backend/CMakeLists.txt @@ -307,8 +307,6 @@ if (APPLE) filabridge getopt gtest - image - imageio SPIRV spirv-cross-glsl) @@ -323,13 +321,16 @@ if (APPLE) getopt gtest glslang - image - imageio spirv-cross-core spirv-cross-glsl spirv-cross-msl ) + if (NOT IOS) + target_link_libraries(backend_test PRIVATE image imageio) + list(APPEND BACKEND_TEST_DEPS image imageio) + endif() + set(BACKEND_TEST_COMBINED_OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/libbackendtest_combined.a") combine_static_libs(backend_test "${BACKEND_TEST_COMBINED_OUTPUT}" "${BACKEND_TEST_DEPS}") diff --git a/filament/backend/test/test_ReadPixels.cpp b/filament/backend/test/test_ReadPixels.cpp index cdf9a38a52..59b947b924 100644 --- a/filament/backend/test/test_ReadPixels.cpp +++ b/filament/backend/test/test_ReadPixels.cpp @@ -16,10 +16,6 @@ #include "BackendTest.h" -#include - -#include - #include "ShaderGenerator.h" #include "TrianglePrimitive.h" @@ -29,7 +25,13 @@ using namespace filament; using namespace filament::backend; + +#ifndef IOS +#include +#include + using namespace image; +#endif namespace { @@ -123,6 +125,7 @@ TEST_F(BackendTest, ReadPixels) { } void exportScreenshot(void* pixelData) const { + #ifndef IOS const size_t width = readRect.width, height = readRect.height; LinearImage image(width, height, 4); if (format == PixelDataFormat::RGBA && type == PixelDataType::UBYTE) { @@ -135,6 +138,7 @@ TEST_F(BackendTest, ReadPixels) { std::ofstream outputStream(png.c_str(), std::ios::binary | std::ios::trunc); ImageEncoder::encode(outputStream, ImageEncoder::Format::PNG, image, "", png.c_str()); + #endif } PixelDataFormat format = PixelDataFormat::RGBA; From 212b64ea5f1856b390cdf7629801243f76a4466d Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 22 Oct 2020 16:38:11 -0700 Subject: [PATCH 19/24] Vulkan: make ReadPixels synchronous. This makes it so that the PixelBufferDescriptor callback triggers at a time consistent with other backends. ReadPixels is still asynchronous in the sense that the callback is triggered on the main thread. However, it is now guaranteed to trigger during (or before) flushAndWait(), which is less surprising behavior. --- filament/backend/src/vulkan/VulkanDriver.cpp | 54 ++++++++------------ 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 419246b81c..0a28e2d443 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -1420,46 +1420,36 @@ void VulkanDriver::readPixels(Handle src, uint32_t x, uint32_t y vkCmdPipelineBarrier(mContext.work.cmdbuffer, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, 0, 0, nullptr, 0, nullptr, 1, &barrier); + // Flush and wait. + flushWorkCommandBuffer(mContext); + acquireWorkCommandBuffer(mContext); - // Create a closure-friendly pointer that holds the rvalue reference. + VkImageSubresource subResource { .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT }; + VkSubresourceLayout subResourceLayout; + vkGetImageSubresourceLayout(device, stagingImage, &subResource, &subResourceLayout); - PixelBufferDescriptor* closure = new PixelBufferDescriptor(); - *closure = std::move(pbd); + // Map image memory so we can start copying from it. - // Create a disposable to defer execution of the following code until after - // the work command buffer has completed. + const uint8_t* srcPixels; + vkMapMemory(device, stagingMemory, 0, VK_WHOLE_SIZE, 0, (void**) &srcPixels); + srcPixels += subResourceLayout.offset; - mDisposer.createDisposable((VulkanDisposer::Key) stagingImage, [=] () { + // TODO: investigate why this Y-flip exists. This conditional seems to work with both + // test_ReadPixels.cpp (readpixels from a normal render target with texture attachment) and + // viewer_basic_test.cc (readpixels from an offscreen swap chain) + const bool flipY = srcTexture ? true : false; - VkImageSubresource subResource { .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT }; - VkSubresourceLayout subResourceLayout; - vkGetImageSubresourceLayout(device, stagingImage, &subResource, &subResourceLayout); + if (!DataReshaper::reshapeImage(&pbd, getComponentType(srcFormat), srcPixels, + subResourceLayout.rowPitch, width, height, swizzle, flipY)) { + utils::slog.e << "Unsupported PixelDataFormat or PixelDataType" << utils::io::endl; + } - // Map image memory so we can start copying from it. + vkUnmapMemory(device, stagingMemory); + vkFreeMemory(device, stagingMemory, nullptr); + vkDestroyImage(device, stagingImage, nullptr); - const uint8_t* srcPixels; - vkMapMemory(device, stagingMemory, 0, VK_WHOLE_SIZE, 0, (void**) &srcPixels); - srcPixels += subResourceLayout.offset; - - // TODO: investigate why this Y-flip exists. - constexpr bool flipY = true; - if (!DataReshaper::reshapeImage(closure, getComponentType(srcFormat), srcPixels, - subResourceLayout.rowPitch, width, height, swizzle, flipY)) { - utils::slog.e << "Unsupported PixelDataFormat or PixelDataType" << utils::io::endl; - } - - vkUnmapMemory(device, stagingMemory); - vkFreeMemory(device, stagingMemory, nullptr); - vkDestroyImage(device, stagingImage, nullptr); - - scheduleDestroy(std::move(*closure)); - delete closure; - }); - - // Next we reduce the ref count of the image to zero, which schedules the above callback to be - // executed on the next beginFrame(), after the work command buffer is completed. - mDisposer.removeReference((VulkanDisposer::Key) stagingImage); + scheduleDestroy(std::move(pbd)); } void VulkanDriver::readStreamPixels(Handle sh, uint32_t x, uint32_t y, uint32_t width, From 82cb13936c02abed4f394b9fc58ef836c43b03a9 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Fri, 23 Oct 2020 09:36:47 -0700 Subject: [PATCH 20/24] Vulkan: warn instead of panic for sampler overflow. This makes behavior in release builds similar to the OpenGL backend. In debug builds, we will still assert. --- filament/backend/src/vulkan/VulkanBinder.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanBinder.cpp b/filament/backend/src/vulkan/VulkanBinder.cpp index 51115ee74f..3ff630b67e 100644 --- a/filament/backend/src/vulkan/VulkanBinder.cpp +++ b/filament/backend/src/vulkan/VulkanBinder.cpp @@ -16,6 +16,7 @@ #include "vulkan/VulkanBinder.h" +#include #include #include @@ -478,9 +479,12 @@ void VulkanBinder::bindUniformBuffer(uint32_t bindingIndex, VkBuffer uniformBuff } void VulkanBinder::bindSampler(uint32_t bindingIndex, VkDescriptorImageInfo samplerInfo) noexcept { - ASSERT_POSTCONDITION(bindingIndex < SAMPLER_BINDING_COUNT, - "Sampler bindings overflow: index = %d, capacity = %d.", - bindingIndex, SAMPLER_BINDING_COUNT); + assert(bindingIndex < SAMPLER_BINDING_COUNT); + if (bindingIndex >= SAMPLER_BINDING_COUNT) { + utils::slog.w << "Sampler bindings overflow: " << bindingIndex << " / " + << SAMPLER_BINDING_COUNT << utils::io::endl; + return; + } VkDescriptorImageInfo& imageInfo = mDescriptorKey.samplers[bindingIndex]; if (imageInfo.sampler != samplerInfo.sampler || imageInfo.imageView != samplerInfo.imageView || imageInfo.imageLayout != samplerInfo.imageLayout) { From 4270384661a4240d5065ccd302d925d5379c190b Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Mon, 26 Oct 2020 11:27:08 -0600 Subject: [PATCH 21/24] Bump version to 1.9.6 --- README.md | 4 ++-- android/gradle.properties | 2 +- ios/CocoaPods/Filament.podspec | 4 ++-- web/filament-js/package.json | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index a5d46b834d..7da889b316 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.9.5' + implementation 'com.google.android.filament:filament-android:1.9.6' } ``` @@ -63,7 +63,7 @@ A much smaller alternative to `filamat-android` that can only generate OpenGL sh iOS projects can use CocoaPods to install the latest release: ``` -pod 'Filament', '~> 1.9.5' +pod 'Filament', '~> 1.9.6' ``` ### Snapshots diff --git a/android/gradle.properties b/android/gradle.properties index 0ad0816b06..cb256dbd8d 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.9.5 +VERSION_NAME=1.9.6 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index f5d1d41b7d..72a38fdf8d 100644 --- a/ios/CocoaPods/Filament.podspec +++ b/ios/CocoaPods/Filament.podspec @@ -1,12 +1,12 @@ Pod::Spec.new do |spec| spec.name = "Filament" - spec.version = "1.9.5" + spec.version = "1.9.6" spec.license = { :type => "Apache 2.0", :file => "LICENSE" } spec.homepage = "https://google.github.io/filament" spec.authors = "Google LLC." spec.summary = "Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WASM/WebGL." spec.platform = :ios, "11.0" - spec.source = { :http => "https://github.com/google/filament/releases/download/v1.9.5/filament-v1.9.5-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.9.6/filament-v1.9.6-ios.tgz" } # Fix linking error with Xcode 12; we do not yet support the simulator on Apple silicon. spec.pod_target_xcconfig = { diff --git a/web/filament-js/package.json b/web/filament-js/package.json index 8e26fc510c..3c9364a33a 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.9.5", + "version": "1.9.6", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From 6971b3b6b041bbfaa38bbd18065c9b0429404853 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Mon, 26 Oct 2020 11:25:31 -0600 Subject: [PATCH 22/24] Update RELEASE_NOTES for 1.9.6 --- RELEASE_NOTES.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 47f49bea7c..eef4d4e265 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -5,11 +5,16 @@ A new header is inserted each time a *tag* is created. ## Next release (main branch) +## v1.9.7 + ## v1.9.6 - Added View::setVsmShadowOptions (experimental) - Add anisotropic shadow map sampling with VSM (experimental) - matc: fixed bug where some compilation failures still exited with code 0 +- Vulkan + Android: fix build break +- Add optional XCB support to PlatformVkLinux +- Fix Vulkan black screen on Windows with NVIDIA hardware ## v1.9.5 From 65394f6301d2c43d235a107b30f4564cf4eb931b Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Mon, 26 Oct 2020 11:34:20 -0600 Subject: [PATCH 23/24] Bump version to 1.9.7 --- README.md | 4 ++-- android/gradle.properties | 2 +- ios/CocoaPods/Filament.podspec | 4 ++-- web/filament-js/package.json | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 7da889b316..70491bcd62 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.9.6' + implementation 'com.google.android.filament:filament-android:1.9.7' } ``` @@ -63,7 +63,7 @@ A much smaller alternative to `filamat-android` that can only generate OpenGL sh iOS projects can use CocoaPods to install the latest release: ``` -pod 'Filament', '~> 1.9.6' +pod 'Filament', '~> 1.9.7' ``` ### Snapshots diff --git a/android/gradle.properties b/android/gradle.properties index cb256dbd8d..b0fe81f4ab 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.9.6 +VERSION_NAME=1.9.7 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 72a38fdf8d..764c667677 100644 --- a/ios/CocoaPods/Filament.podspec +++ b/ios/CocoaPods/Filament.podspec @@ -1,12 +1,12 @@ Pod::Spec.new do |spec| spec.name = "Filament" - spec.version = "1.9.6" + spec.version = "1.9.7" spec.license = { :type => "Apache 2.0", :file => "LICENSE" } spec.homepage = "https://google.github.io/filament" spec.authors = "Google LLC." spec.summary = "Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WASM/WebGL." spec.platform = :ios, "11.0" - spec.source = { :http => "https://github.com/google/filament/releases/download/v1.9.6/filament-v1.9.6-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.9.7/filament-v1.9.7-ios.tgz" } # Fix linking error with Xcode 12; we do not yet support the simulator on Apple silicon. spec.pod_target_xcconfig = { diff --git a/web/filament-js/package.json b/web/filament-js/package.json index 3c9364a33a..36a7e1df4d 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.9.6", + "version": "1.9.7", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From be4fb4fdbbaead746892206f3b5088fdd25dff68 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Mon, 2 Nov 2020 10:59:19 -0700 Subject: [PATCH 24/24] Update RELEASE_NOTES for 1.9.7 --- RELEASE_NOTES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index eef4d4e265..32966665d0 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,6 +7,12 @@ A new header is inserted each time a *tag* is created. ## v1.9.7 +- Vulkan: improvements to the ReadPixels implementation. +- Vulkan: warn instead of panic for sampler overflow. +- Vulkan: fix leak with headless swap chain. +- PlatformVkLinux now supports all combos of XLIB and XCB. +- Fix TypeScript binding for TextureUsage. + ## v1.9.6 - Added View::setVsmShadowOptions (experimental)