From c65dc99771127ad0901d815a80f04633296a7ee8 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Mon, 26 Oct 2020 14:52:46 -0700 Subject: [PATCH 01/21] Fix colorGradingAsSubpass when dithering is off. This fixes the blank screen seen with Vulkan when all view options are disabled. --- filament/src/materials/colorGrading/colorGradingAsSubpass.mat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/src/materials/colorGrading/colorGradingAsSubpass.mat b/filament/src/materials/colorGrading/colorGradingAsSubpass.mat index b44ad79fa8..2e85f16685 100644 --- a/filament/src/materials/colorGrading/colorGradingAsSubpass.mat +++ b/filament/src/materials/colorGrading/colorGradingAsSubpass.mat @@ -119,8 +119,8 @@ fragment { #else postProcess.color = dithered; #endif - postProcess.tonemappedOutput = postProcess.color; } + postProcess.tonemappedOutput = postProcess.color; } } From c6942a52e6852c6e7b18e0e833c86551c87432bf Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Mon, 26 Oct 2020 17:04:53 -0700 Subject: [PATCH 02/21] gltfio: refactor the instancing cache. This prepares for #3137 by moving the mesh cache and material instance cache out of the loader (where they were transient anyway) and into the actual asset. This paves the way for a `createInstance()` API. --- libs/gltfio/include/gltfio/AssetLoader.h | 12 +++--- libs/gltfio/src/AssetLoader.cpp | 52 +++++------------------- libs/gltfio/src/FFilamentAsset.h | 31 ++++++++++++++ libs/gltfio/src/FilamentAsset.cpp | 2 + 4 files changed, 49 insertions(+), 48 deletions(-) diff --git a/libs/gltfio/include/gltfio/AssetLoader.h b/libs/gltfio/include/gltfio/AssetLoader.h index a276ef5a95..e9ee733b64 100644 --- a/libs/gltfio/include/gltfio/AssetLoader.h +++ b/libs/gltfio/include/gltfio/AssetLoader.h @@ -154,19 +154,19 @@ public: /** * Consumes the contents of a glTF 2.0 file and produces a primary asset with one or more - * instances. + * instances. The primary asset has ownership over the instances. * * The returned instances share their textures, material instances, and vertex buffers with the - * primary asset. However each instance has its own unique set of entities, transform components, - * and renderable components. Instances are automatically freed when the primary asset is freed. + * primary asset. However each instance has its own unique set of entities, transform + * components, and renderable components. Instances are freed when the primary asset is freed. * * Light components are not instanced, they belong only to the primary asset. * * Clients must use ResourceLoader to load resources on the primary asset. * - * The entity accessors and renderable stack in the returned FilamentAsset represent the union - * of all entities across all instances. Use the individual FilamentInstance objects to access - * each partition of entities. Similarly, the Animator in the primary asset controls all + * The entity accessor and renderable stack API in the primary asset can be used to control the + * union of all instances. The individual FilamentInstance objects can be used to access each + * instance's partition of entities. Similarly, the Animator in the primary asset controls all * instances. To animate instances individually, use FilamentInstance::getAnimator(). * * @param bytes the contents of a glTF 2.0 file (JSON or GLB) diff --git a/libs/gltfio/src/AssetLoader.cpp b/libs/gltfio/src/AssetLoader.cpp index 960619bb79..90aac5e19a 100644 --- a/libs/gltfio/src/AssetLoader.cpp +++ b/libs/gltfio/src/AssetLoader.cpp @@ -60,33 +60,6 @@ namespace gltfio { static const auto FREE_CALLBACK = [](void* mem, size_t, void*) { free(mem); }; -// MeshCache -// --------- -// If a given glTF mesh is referenced by multiple glTF nodes, then it generates a separate Filament -// renderable for each of those nodes. All renderables generated by a given mesh share a common set -// of VertexBuffer and IndexBuffer objects. To achieve the sharing behavior, the loader maintains a -// small cache. The cache keys are glTF mesh definitions and the cache entries are lists of -// primitives, where a "primitive" is a reference to a Filament VertexBuffer and IndexBuffer. -struct Primitive { - VertexBuffer* vertices = nullptr; - IndexBuffer* indices = nullptr; - Aabb aabb; // object-space bounding box -}; -using MeshCache = tsl::robin_map>; - -// MatInstanceCache -// ---------------- -// Each glTF material definition corresponds to a single filament::MaterialInstance, which are -// cached here in the loader. The filament::Material objects that are used to create instances are -// cached in MaterialProvider. If a given glTF material is referenced by multiple glTF meshes, then -// their corresponding filament primitives will share the same Filament MaterialInstance and UvMap. -// The UvMap is a mapping from each texcoord slot in glTF to one of Filament's 2 texcoord sets. -struct MaterialEntry { - MaterialInstance* instance; - UvMap uvmap; -}; -using MatInstanceCache = tsl::robin_map; - // Sometimes a glTF bufferview includes unused data at the end (e.g. in skinning.gltf) so we need to // compute the correct size of the vertex buffer. Filament automatically infers the size of // driver-level vertex buffers from the attribute data (stride, count, offset) and clients are @@ -171,10 +144,8 @@ struct FAssetLoader : public AssetLoader { MaterialProvider* mMaterials; Engine* mEngine; - // The loader owns a few transient mappings used only for the current asset being loaded. + // Transient state used only for the asset currently being loaded: FFilamentAsset* mResult; - MatInstanceCache mMatInstanceCache; - MeshCache mMeshCache; const char* mDefaultNodeName; bool mError = false; bool mDiagnosticsEnabled = false; @@ -280,8 +251,8 @@ void FAssetLoader::createAsset(const cgltf_data* srcAsset, size_t numInstances) createEntity(nodes[i], mResult->mRoot, true, nullptr); } } else { - // Create a separate entity hierarchy for each instance. Note that mMeshCache (vertex - // buffers and index buffers) and mMatInstanceCache (materials and textures) help avoid + // Create a separate entity hierarchy for each instance. Note that MeshCache (vertex + // buffers and index buffers) and MatInstanceCache (materials and textures) help avoid // needless duplication of resources. for (size_t index = 0; index < numInstances; ++index) { // Create a root node within each instance that is a child of the primary root. @@ -329,9 +300,6 @@ void FAssetLoader::createAsset(const cgltf_data* srcAsset, size_t numInstances) mResult->mResourceUris.push_back(pair.second); } - // We're done with the import, so free up transient bookkeeping resources. - mMatInstanceCache.clear(); - mMeshCache.clear(); mError = false; } @@ -406,11 +374,11 @@ void FAssetLoader::createRenderable(const cgltf_node* node, Entity entity, const // If the mesh is already loaded, obtain the list of Filament VertexBuffer / IndexBuffer objects // that were already generated (one for each primitive), otherwise allocate a new list of // pointers for the primitives. - auto iter = mMeshCache.find(mesh); - if (iter == mMeshCache.end()) { - mMeshCache[mesh].resize(nprims); + auto iter = mResult->mMeshCache.find(mesh); + if (iter == mResult->mMeshCache.end()) { + mResult->mMeshCache[mesh].resize(nprims); } - Primitive* outputPrim = mMeshCache[mesh].data(); + Primitive* outputPrim = mResult->mMeshCache[mesh].data(); const cgltf_primitive* inputPrim = &mesh->primitives[0]; Aabb aabb; @@ -849,8 +817,8 @@ void FAssetLoader::createCamera(const cgltf_camera* camera, Entity entity) { MaterialInstance* FAssetLoader::createMaterialInstance(const cgltf_material* inputMat, UvMap* uvmap, bool vertexColor) { intptr_t key = ((intptr_t) inputMat) ^ (vertexColor ? 1 : 0); - auto iter = mMatInstanceCache.find(key); - if (iter != mMatInstanceCache.end()) { + auto iter = mResult->mMatInstanceCache.find(key); + if (iter != mResult->mMatInstanceCache.end()) { *uvmap = iter->second.uvmap; return iter->second.instance; } @@ -1074,7 +1042,7 @@ MaterialInstance* FAssetLoader::createMaterialInstance(const cgltf_material* inp } } - mMatInstanceCache[key] = {mi, *uvmap}; + mResult->mMatInstanceCache[key] = {mi, *uvmap}; return mi; } diff --git a/libs/gltfio/src/FFilamentAsset.h b/libs/gltfio/src/FFilamentAsset.h index 2d9891a226..0ffec28f91 100644 --- a/libs/gltfio/src/FFilamentAsset.h +++ b/libs/gltfio/src/FFilamentAsset.h @@ -28,6 +28,8 @@ #include #include +#include + #include #include @@ -73,6 +75,33 @@ struct TextureSlot { bool srgb; }; +// MeshCache +// --------- +// If a given glTF mesh is referenced by multiple glTF nodes, then it generates a separate Filament +// renderable for each of those nodes. All renderables generated by a given mesh share a common set +// of VertexBuffer and IndexBuffer objects. To achieve the sharing behavior, the loader maintains a +// small cache. The cache keys are glTF mesh definitions and the cache entries are lists of +// primitives, where a "primitive" is a reference to a Filament VertexBuffer and IndexBuffer. +struct Primitive { + filament::VertexBuffer* vertices = nullptr; + filament::IndexBuffer* indices = nullptr; + filament::Aabb aabb; // object-space bounding box +}; +using MeshCache = tsl::robin_map>; + +// MatInstanceCache +// ---------------- +// Each glTF material definition corresponds to a single filament::MaterialInstance, which are +// temporarily cached during loading. The filament::Material objects that are used to create instances are +// cached in MaterialProvider. If a given glTF material is referenced by multiple glTF meshes, then +// their corresponding filament primitives will share the same Filament MaterialInstance and UvMap. +// The UvMap is a mapping from each texcoord slot in glTF to one of Filament's 2 texcoord sets. +struct MaterialEntry { + filament::MaterialInstance* instance; + UvMap uvmap; +}; +using MatInstanceCache = tsl::robin_map; + struct FFilamentAsset : public FilamentAsset { FFilamentAsset(filament::Engine* engine, utils::NameComponentManager* names, utils::EntityManager* entityManager) : @@ -218,6 +247,8 @@ struct FFilamentAsset : public FilamentAsset { const cgltf_data* mSourceAsset = nullptr; NodeMap mNodeMap; // unused for instanced assets std::vector > mPrimitives; + MatInstanceCache mMatInstanceCache; + MeshCache mMeshCache; }; FILAMENT_UPCAST(FilamentAsset) diff --git a/libs/gltfio/src/FilamentAsset.cpp b/libs/gltfio/src/FilamentAsset.cpp index 87886e7463..4dc3dfa608 100644 --- a/libs/gltfio/src/FilamentAsset.cpp +++ b/libs/gltfio/src/FilamentAsset.cpp @@ -86,6 +86,8 @@ void FFilamentAsset::releaseSourceData() noexcept { // To ensure that all possible memory is freed, we reassign to new containers rather than // calling clear(). With many container types (such as robin_map), clearing is a fast // operation that merely frees the storage for the items. + mMatInstanceCache = {}; + mMeshCache = {}; mResourceUris = {}; mNodeMap = {}; mPrimitives = {}; From a6f364e0f395c410ddbcd12fe298d06e492bb8ed Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 28 Oct 2020 11:17:20 -0700 Subject: [PATCH 03/21] gltfio: fix segfault when consuming invalid file. --- libs/gltfio/src/AssetLoader.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/libs/gltfio/src/AssetLoader.cpp b/libs/gltfio/src/AssetLoader.cpp index 90aac5e19a..5285a454ab 100644 --- a/libs/gltfio/src/AssetLoader.cpp +++ b/libs/gltfio/src/AssetLoader.cpp @@ -276,11 +276,6 @@ void FAssetLoader::createAsset(const cgltf_data* srcAsset, size_t numInstances) } } - if (mError) { - delete mResult; - mResult = nullptr; - } - // Find every unique resource URI and store a pointer to any of the cgltf-owned cstrings // that match the URI. These strings get freed during releaseSourceData(). tsl::robin_map resourceUris; @@ -300,7 +295,11 @@ void FAssetLoader::createAsset(const cgltf_data* srcAsset, size_t numInstances) mResult->mResourceUris.push_back(pair.second); } - mError = false; + if (mError) { + delete mResult; + mResult = nullptr; + mError = false; + } } void FAssetLoader::createEntity(const cgltf_node* node, Entity parent, bool enableLight, From 1159435d47600f5a58a5aa6db8bea1295fadddd7 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 28 Oct 2020 11:12:41 -0700 Subject: [PATCH 04/21] gltfio: add safety checks to getAnimator. --- libs/gltfio/src/Animator.cpp | 1 + libs/gltfio/src/FFilamentAsset.h | 1 + libs/gltfio/src/FFilamentInstance.h | 10 ++-------- libs/gltfio/src/FilamentAsset.cpp | 10 ++++++++++ libs/gltfio/src/FilamentInstance.cpp | 18 ++++++++++++++++++ libs/gltfio/src/ResourceLoader.cpp | 6 +++--- libs/viewer/src/SimpleViewer.cpp | 3 ++- 7 files changed, 37 insertions(+), 12 deletions(-) diff --git a/libs/gltfio/src/Animator.cpp b/libs/gltfio/src/Animator.cpp index e2004cb334..92de0a7798 100644 --- a/libs/gltfio/src/Animator.cpp +++ b/libs/gltfio/src/Animator.cpp @@ -140,6 +140,7 @@ static void setTransformType(const cgltf_animation_channel& src, Channel& dst) { } Animator::Animator(FFilamentAsset* asset, FFilamentInstance* instance) { + assert(asset->mResourcesLoaded && !asset->mIsReleased); mImpl = new AnimatorImpl(); mImpl->asset = asset; mImpl->instance = instance; diff --git a/libs/gltfio/src/FFilamentAsset.h b/libs/gltfio/src/FFilamentAsset.h index 0ffec28f91..c73b56f913 100644 --- a/libs/gltfio/src/FFilamentAsset.h +++ b/libs/gltfio/src/FFilamentAsset.h @@ -249,6 +249,7 @@ struct FFilamentAsset : public FilamentAsset { std::vector > mPrimitives; MatInstanceCache mMatInstanceCache; MeshCache mMeshCache; + bool mIsReleased = false; }; FILAMENT_UPCAST(FilamentAsset) diff --git a/libs/gltfio/src/FFilamentInstance.h b/libs/gltfio/src/FFilamentInstance.h index 3fa6cd9b82..6487f6d1d9 100644 --- a/libs/gltfio/src/FFilamentInstance.h +++ b/libs/gltfio/src/FFilamentInstance.h @@ -18,7 +18,6 @@ #define GLTFIO_FFILAMENTINSTANCE_H #include -#include #include @@ -36,6 +35,7 @@ struct cgltf_node; namespace gltfio { struct FFilamentAsset; +class Animator; struct Skin { std::string name; @@ -54,13 +54,7 @@ struct FFilamentInstance : public FilamentInstance { FFilamentAsset* owner; SkinVector skins; NodeMap nodeMap; - - Animator* getAnimator() noexcept { - if (!animator) { - animator = new Animator(owner, this); - } - return animator; - } + Animator* getAnimator() noexcept; }; FILAMENT_UPCAST(FilamentInstance) diff --git a/libs/gltfio/src/FilamentAsset.cpp b/libs/gltfio/src/FilamentAsset.cpp index 4dc3dfa608..1b4a2e4d82 100644 --- a/libs/gltfio/src/FilamentAsset.cpp +++ b/libs/gltfio/src/FilamentAsset.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include "Wireframe.h" @@ -70,6 +71,14 @@ FFilamentAsset::~FFilamentAsset() { Animator* FFilamentAsset::getAnimator() noexcept { if (!mAnimator) { + if (!mResourcesLoaded) { + slog.e << "Cannot create animator before resource loading." << io::endl; + return nullptr; + } + if (mIsReleased) { + slog.e << "Cannot create animator from frozen asset." << io::endl; + return nullptr; + } mAnimator = new Animator(this, nullptr); } return mAnimator; @@ -83,6 +92,7 @@ Entity FFilamentAsset::getWireframe() noexcept { } void FFilamentAsset::releaseSourceData() noexcept { + mIsReleased = true; // To ensure that all possible memory is freed, we reassign to new containers rather than // calling clear(). With many container types (such as robin_map), clearing is a fast // operation that merely frees the storage for the items. diff --git a/libs/gltfio/src/FilamentInstance.cpp b/libs/gltfio/src/FilamentInstance.cpp index 9effbacf89..de91a4dcf5 100644 --- a/libs/gltfio/src/FilamentInstance.cpp +++ b/libs/gltfio/src/FilamentInstance.cpp @@ -15,14 +15,32 @@ */ #include "FFilamentInstance.h" +#include "FFilamentAsset.h" #include +#include + using namespace filament; using namespace utils; namespace gltfio { +Animator* FFilamentInstance::getAnimator() noexcept { + if (!animator) { + if (!owner->mResourcesLoaded) { + slog.e << "Cannot create animator before resource loading." << io::endl; + return nullptr; + } + if (owner->mIsReleased) { + slog.e << "Cannot create animator from frozen asset." << io::endl; + return nullptr; + } + animator = new Animator(owner, this); + } + return animator; +} + size_t FilamentInstance::getEntityCount() const noexcept { return upcast(this)->entities.size(); } diff --git a/libs/gltfio/src/ResourceLoader.cpp b/libs/gltfio/src/ResourceLoader.cpp index 4a54a6e9aa..16b4364a4d 100644 --- a/libs/gltfio/src/ResourceLoader.cpp +++ b/libs/gltfio/src/ResourceLoader.cpp @@ -296,7 +296,6 @@ bool ResourceLoader::loadResources(FFilamentAsset* asset, bool async) { if (asset->mResourcesLoaded) { return false; } - asset->mResourcesLoaded = true; mPool->addAsset(asset); const cgltf_data* gltf = asset->mSourceAsset; cgltf_options options {}; @@ -444,8 +443,9 @@ bool ResourceLoader::loadResources(FFilamentAsset* asset, bool async) { asset->mDependencyGraph.finalize(); pImpl->mCurrentAsset = asset; - // Finally, load image files and create Filament Textures. - return pImpl->createTextures(async); + // Finally, create Filament Textures and begin loading image files. + asset->mResourcesLoaded = pImpl->createTextures(async); + return asset->mResourcesLoaded; } bool ResourceLoader::asyncBeginLoad(FilamentAsset* asset) { diff --git a/libs/viewer/src/SimpleViewer.cpp b/libs/viewer/src/SimpleViewer.cpp index 051a328edc..69362c4fc1 100644 --- a/libs/viewer/src/SimpleViewer.cpp +++ b/libs/viewer/src/SimpleViewer.cpp @@ -410,7 +410,8 @@ void SimpleViewer::updateUserInterface() { ImGui::Unindent(); } - if (mAnimator->getAnimationCount() > 0 && ImGui::CollapsingHeader("Animation")) { + if (mAnimator && mAnimator->getAnimationCount() > 0 && + ImGui::CollapsingHeader("Animation")) { ImGui::Indent(); int selectedAnimation = mCurrentAnimation; ImGui::RadioButton("Disable", &selectedAnimation, 0); From 2fe4f6cbe70d4fc93f80f2a29c35ffd51a493b70 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 28 Oct 2020 12:50:51 -0700 Subject: [PATCH 05/21] AutomationEngine: fix regression with screenshots. --- filament/backend/test/test_ReadPixels.cpp | 2 +- libs/image/include/image/ColorTransform.h | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/filament/backend/test/test_ReadPixels.cpp b/filament/backend/test/test_ReadPixels.cpp index 59b947b924..837e62e263 100644 --- a/filament/backend/test/test_ReadPixels.cpp +++ b/filament/backend/test/test_ReadPixels.cpp @@ -129,7 +129,7 @@ TEST_F(BackendTest, ReadPixels) { 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); + image = toLinearWithAlpha(width, height, width * 4, (uint8_t*) pixelData); } if (format == PixelDataFormat::RGBA && type == PixelDataType::FLOAT) { memcpy(image.getPixelRef(), pixelData, width * height * sizeof(math::float4)); diff --git a/libs/image/include/image/ColorTransform.h b/libs/image/include/image/ColorTransform.h index 61ebaed86a..334c21b014 100644 --- a/libs/image/include/image/ColorTransform.h +++ b/libs/image/include/image/ColorTransform.h @@ -347,7 +347,7 @@ inline LinearImage fromLinearToRGBM(const LinearImage& image) { } template -static LinearImage toLinear(size_t w, size_t h, size_t bpr, const uint8_t* src) { +static LinearImage toLinearWithAlpha(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) { @@ -361,6 +361,21 @@ static LinearImage toLinear(size_t w, size_t h, size_t bpr, const uint8_t* src) return result; } +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; +} + } // namespace Image #endif // IMAGE_COLORTRANSFORM_H_ From a9754273b0b061a87a1915b77e5bfdfc10d9c2a8 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 28 Oct 2020 15:38:38 -0700 Subject: [PATCH 06/21] Vulkan: minor fix for diagnostic output. --- filament/backend/src/vulkan/VulkanHandles.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index d7d0946ffc..bf5bc5f84b 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -77,7 +77,7 @@ VulkanProgram::VulkanProgram(VulkanContext& context, const Program& builder) noe #if FILAMENT_VULKAN_VERBOSE utils::slog.d << "Created VulkanProgram " << builder.getName().c_str() << ", variant = (" << utils::io::hex - << builder.getVariant() << utils::io::dec << "), " + << (int) builder.getVariant() << utils::io::dec << "), " << "shaders = (" << bundle.vertex << ", " << bundle.fragment << ")" << utils::io::endl; #endif From 94a1eb27b448eca738f3e5a8205a465b640be0c5 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 28 Oct 2020 15:35:46 -0700 Subject: [PATCH 07/21] Do not declare unused fragColor in GLSL. This fixes a Vulkan validation warning when building a pipeline that does not have a color attachment. --- shaders/src/depth_main.fs | 4 ++++ shaders/src/inputs.fs | 2 +- shaders/src/main.fs | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/shaders/src/depth_main.fs b/shaders/src/depth_main.fs index 739e797564..8ac7f75d1d 100644 --- a/shaders/src/depth_main.fs +++ b/shaders/src/depth_main.fs @@ -1,3 +1,7 @@ +#if defined(HAS_VSM) +layout(location = 0) out vec4 fragColor; +#endif + //------------------------------------------------------------------------------ // Depth //------------------------------------------------------------------------------ diff --git a/shaders/src/inputs.fs b/shaders/src/inputs.fs index f442f6b66e..1b29d70426 100644 --- a/shaders/src/inputs.fs +++ b/shaders/src/inputs.fs @@ -31,4 +31,4 @@ LAYOUT_LOCATION(11) in highp vec4 vertex_lightSpacePosition; LAYOUT_LOCATION(12) in highp vec4 vertex_spotLightSpacePosition[MAX_SHADOW_CASTING_SPOTS]; #endif -layout(location = 0) out vec4 fragColor; +// Note that fragColor is an output and is not declared here; see main.fs and depth_main.fs diff --git a/shaders/src/main.fs b/shaders/src/main.fs index fe7dcae8e5..8aab527c3f 100644 --- a/shaders/src/main.fs +++ b/shaders/src/main.fs @@ -1,3 +1,5 @@ +layout(location = 0) out vec4 fragColor; + #if defined(MATERIAL_HAS_POST_LIGHTING_COLOR) void blendPostLightingColor(const MaterialInputs material, inout vec4 color) { #if defined(POST_LIGHTING_BLEND_MODE_OPAQUE) From 4b1c557bb1ab3ec056a1134f618df7aa7b14a8b0 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Wed, 28 Oct 2020 17:00:42 -0700 Subject: [PATCH 08/21] Vulkan: fix color attachment list in first subpass. This fixes a slew of validation warnings and errors seen when multiple subpasses are enabled, starting with: Attachment 1 not written by fragment shader; undefined values will be written to attachment The Vulkan driver was using the same color attachment list for both subpasses, so the first subpass had unused attachments. Note that Vulkan makes a distinction between color attachments and input attachments and requires separate lists to be supplied for each subpass, but our Driver API consolidates everything into a single list. This should perhaps be refactored at a later date. --- filament/backend/src/vulkan/VulkanBinder.cpp | 4 +- filament/backend/src/vulkan/VulkanBinder.h | 2 +- filament/backend/src/vulkan/VulkanDriver.cpp | 2 +- .../backend/src/vulkan/VulkanFboCache.cpp | 86 +++++++++++-------- filament/backend/src/vulkan/VulkanHandles.cpp | 10 ++- filament/backend/src/vulkan/VulkanHandles.h | 2 +- 6 files changed, 63 insertions(+), 43 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanBinder.cpp b/filament/backend/src/vulkan/VulkanBinder.cpp index 3ff630b67e..61065e0777 100644 --- a/filament/backend/src/vulkan/VulkanBinder.cpp +++ b/filament/backend/src/vulkan/VulkanBinder.cpp @@ -281,7 +281,7 @@ bool VulkanBinder::getOrCreatePipeline(VkPipeline* pipeline) noexcept { pipelineCreateInfo.pDynamicState = &dynamicState; // Filament assumes consistent blend state across all color attachments. - mColorBlendState.attachmentCount = mPipelineKey.rasterState.getColorTargetCount; + mColorBlendState.attachmentCount = mPipelineKey.rasterState.colorTargetCount; for (auto& target : mColorBlendAttachments) { target = mPipelineKey.rasterState.blending; } @@ -333,7 +333,7 @@ void VulkanBinder::bindRasterState(const RasterState& rasterState) noexcept { VkPipelineMultisampleStateCreateInfo& ms0 = mPipelineKey.rasterState.multisampling; const VkPipelineMultisampleStateCreateInfo& ms1 = rasterState.multisampling; if ( - mPipelineKey.rasterState.getColorTargetCount != rasterState.getColorTargetCount || + mPipelineKey.rasterState.colorTargetCount != rasterState.colorTargetCount || raster0.polygonMode != raster1.polygonMode || raster0.cullMode != raster1.cullMode || raster0.frontFace != raster1.frontFace || diff --git a/filament/backend/src/vulkan/VulkanBinder.h b/filament/backend/src/vulkan/VulkanBinder.h index 53c1581df6..2b91ec3d4f 100644 --- a/filament/backend/src/vulkan/VulkanBinder.h +++ b/filament/backend/src/vulkan/VulkanBinder.h @@ -103,7 +103,7 @@ public: VkPipelineColorBlendAttachmentState blending; VkPipelineDepthStencilStateCreateInfo depthStencil; VkPipelineMultisampleStateCreateInfo multisampling; - uint32_t getColorTargetCount; + uint32_t colorTargetCount; }; static_assert(std::is_pod::value, "RasterState must be a POD for fast hashing."); diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 0a28e2d443..3a81fc2b07 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -1622,7 +1622,7 @@ void VulkanDriver::draw(PipelineState pipelineState, Handle r vkraster.depthBiasConstantFactor = depthOffset.constant; vkraster.depthBiasSlopeFactor = depthOffset.slope; - mContext.rasterState.getColorTargetCount = rt->getColorTargetCount(); + mContext.rasterState.colorTargetCount = rt->getColorTargetCount(mContext.currentRenderPass); VulkanBinder::ProgramBundle shaderHandles = program->bundle; diff --git a/filament/backend/src/vulkan/VulkanFboCache.cpp b/filament/backend/src/vulkan/VulkanFboCache.cpp index e74fd26eca..ba02c930f4 100644 --- a/filament/backend/src/vulkan/VulkanFboCache.cpp +++ b/filament/backend/src/vulkan/VulkanFboCache.cpp @@ -120,6 +120,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { return iter->second.handle; } const bool isSwapChain = config.colorLayout[0] == VK_IMAGE_LAYOUT_PRESENT_SRC_KHR; + const bool hasSubpasses = config.subpassMask != 0; // Set up some const aliases for terseness. const VkAttachmentLoadOp kClear = VK_ATTACHMENT_LOAD_OP_CLEAR; @@ -149,7 +150,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { } VkAttachmentReference inputAttachmentRef[MRT::TARGET_COUNT] = {}; - VkAttachmentReference colorAttachmentRef[MRT::TARGET_COUNT] = {}; + VkAttachmentReference colorAttachmentRefs[2][MRT::TARGET_COUNT] = {}; VkAttachmentReference resolveAttachmentRef[MRT::TARGET_COUNT] = {}; VkAttachmentReference depthAttachmentRef = {}; @@ -157,14 +158,15 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { VkSubpassDescription subpasses[2] = {{ .pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS, - .pColorAttachments = colorAttachmentRef, + .pInputAttachments = nullptr, + .pColorAttachments = colorAttachmentRefs[0], .pResolveAttachments = resolveAttachmentRef, .pDepthStencilAttachment = hasDepth ? &depthAttachmentRef : nullptr }, { .pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS, .pInputAttachments = inputAttachmentRef, - .pColorAttachments = colorAttachmentRef, + .pColorAttachments = colorAttachmentRefs[1], .pResolveAttachments = resolveAttachmentRef, .pDepthStencilAttachment = hasDepth ? &depthAttachmentRef : nullptr }}; @@ -174,24 +176,6 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { // Note that this needs to have the same ordering as the corollary array in getFramebuffer. VkAttachmentDescription attachments[MRT::TARGET_COUNT + MRT::TARGET_COUNT + 1] = {}; - // Determine the number of color attachments based on whether the format has been initialized. - int colorAttachmentCount = 0; - for (VkFormat format : config.colorFormat) { - if (format != VK_FORMAT_UNDEFINED) { - ++colorAttachmentCount; - } - } - subpasses[0].colorAttachmentCount = colorAttachmentCount; - subpasses[1].colorAttachmentCount = colorAttachmentCount; - - // Nulling out the zero-sized lists is necessary to avoid VK_ERROR_OUT_OF_HOST_MEMORY on Adreno. - if (colorAttachmentCount == 0) { - subpasses[0].pColorAttachments = nullptr; - subpasses[0].pResolveAttachments = nullptr; - subpasses[1].pColorAttachments = nullptr; - subpasses[1].pResolveAttachments = nullptr; - } - // We support 2 subpasses, which means we need to supply 1 dependency struct. VkSubpassDependency dependencies[1] = {{ .srcSubpass = 0, @@ -207,32 +191,56 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { .sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO, .attachmentCount = 0u, .pAttachments = attachments, - .subpassCount = config.subpassMask ? 2u : 1u, + .subpassCount = hasSubpasses ? 2u : 1u, .pSubpasses = subpasses, - .dependencyCount = config.subpassMask ? 1u : 0u, + .dependencyCount = hasSubpasses ? 1u : 0u, .pDependencies = dependencies }; int attachmentIndex = 0; // Populate the Color Attachments. - VkAttachmentReference* pColorAttachment = colorAttachmentRef; for (int i = 0; i < MRT::TARGET_COUNT; i++) { if (config.colorFormat[i] == VK_FORMAT_UNDEFINED) { continue; } - TargetBufferFlags flag = TargetBufferFlags(int(TargetBufferFlags::COLOR0) << i); - bool clear = any(config.clear & flag); - bool discard = any(config.discardStart & flag); - if (config.subpassMask & (1 << i)) { - int subpassInputIndex = subpasses[1].inputAttachmentCount++; - inputAttachmentRef[subpassInputIndex].layout = colorLayouts[i].subpass; - inputAttachmentRef[subpassInputIndex].attachment = attachmentIndex; + const VkImageLayout subpassLayout = colorLayouts[i].subpass; + uint32_t index; + + if (!hasSubpasses) { + index = subpasses[0].colorAttachmentCount++; + colorAttachmentRefs[0][index].layout = subpassLayout; + colorAttachmentRefs[0][index].attachment = attachmentIndex; + } else { + + // The Driver API consolidates all color attachments from the first and second subpasses + // into a single list, and uses a bitmask to mark attachments that belong only to the + // second subpass and should be available as inputs. All color attachments in the first + // subpass are automatically made available to the second subpass. + + // If there are subpasses, we require the input attachment to be the first attachment. + // Breaking this assumption would likely require enhancements to the Driver API in order + // to supply Vulkan with all the information needed. + assert(config.subpassMask == 1); + + if (config.subpassMask & (1 << i)) { + index = subpasses[0].colorAttachmentCount++; + colorAttachmentRefs[0][index].layout = subpassLayout; + colorAttachmentRefs[0][index].attachment = attachmentIndex; + + index = subpasses[1].inputAttachmentCount++; + inputAttachmentRef[index].layout = subpassLayout; + inputAttachmentRef[index].attachment = attachmentIndex; + } + + index = subpasses[1].colorAttachmentCount++; + colorAttachmentRefs[1][index].layout = subpassLayout; + colorAttachmentRefs[1][index].attachment = attachmentIndex; } - pColorAttachment->layout = colorLayouts[i].subpass; - pColorAttachment->attachment = attachmentIndex; - ++pColorAttachment; + const TargetBufferFlags flag = TargetBufferFlags(int(TargetBufferFlags::COLOR0) << i); + const bool clear = any(config.clear & flag); + const bool discard = any(config.discardStart & flag); attachments[attachmentIndex++] = { .format = config.colorFormat[i], @@ -246,6 +254,14 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { }; } + // Nulling out the zero-sized lists is necessary to avoid VK_ERROR_OUT_OF_HOST_MEMORY on Adreno. + if (subpasses[0].colorAttachmentCount == 0) { + subpasses[0].pColorAttachments = nullptr; + subpasses[0].pResolveAttachments = nullptr; + subpasses[1].pColorAttachments = nullptr; + subpasses[1].pResolveAttachments = nullptr; + } + // Populate the Resolve Attachments. VkAttachmentReference* pResolveAttachment = resolveAttachmentRef; for (int i = 0; i < MRT::TARGET_COUNT; i++) { @@ -304,7 +320,7 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { utils::slog.d << "Created render pass " << renderPass << " with " << "samples = " << int(config.samples) << ", " << "depth = " << (hasDepth ? 1 : 0) << ", " - << "colorAttachmentCount = " << colorAttachmentCount + << "colorAttachmentCount[0] = " << subpasses[0].colorAttachmentCount << utils::io::endl; #endif diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index bf5bc5f84b..ea83f841e0 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -374,14 +374,18 @@ VulkanAttachment VulkanRenderTarget::getMsaaDepth() const { return mMsaaDepthAttachment; } -int VulkanRenderTarget::getColorTargetCount() const { +int VulkanRenderTarget::getColorTargetCount(const VulkanRenderPass& pass) const { if (!mOffscreen) { return 1; } int count = 0; for (int i = 0; i < MRT::TARGET_COUNT; i++) { - if (mColor[i].format != VK_FORMAT_UNDEFINED) { - ++count; + if (mColor[i].format == VK_FORMAT_UNDEFINED) { + continue; + } + // NOTE: This must be consistent with VkRenderPass construction (see VulkanFboCache). + if (!(pass.subpassMask & (1 << i)) || pass.currentSubpass == 1) { + count++; } } return count; diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index adbd9aea26..ebaf55636c 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -58,7 +58,7 @@ struct VulkanRenderTarget : private HwRenderTarget { VulkanAttachment getMsaaColor(int target) const; VulkanAttachment getDepth() const; VulkanAttachment getMsaaDepth() const; - int getColorTargetCount() const; + int getColorTargetCount(const VulkanRenderPass& pass) const; bool invalidate(); uint8_t getSamples() const { return mSamples; } private: From 6abef94d95d90d583bb04ed25894a10866b20b72 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 29 Oct 2020 15:33:58 -0700 Subject: [PATCH 09/21] mathio: add ostream operator for quaternions. --- libs/mathio/include/mathio/ostream.h | 5 +++++ libs/mathio/src/ostream.cpp | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/libs/mathio/include/mathio/ostream.h b/libs/mathio/include/mathio/ostream.h index 58c1bf3a1d..101c84deb5 100644 --- a/libs/mathio/include/mathio/ostream.h +++ b/libs/mathio/include/mathio/ostream.h @@ -20,6 +20,8 @@ namespace filament { namespace math { +namespace details { template class TQuaternion; } + template std::ostream& operator<<(std::ostream& out, const details::TVec2& v) noexcept; @@ -38,5 +40,8 @@ std::ostream& operator<<(std::ostream& out, const details::TMat33& v) noexcep template std::ostream& operator<<(std::ostream& out, const details::TMat44& v) noexcept; +template +std::ostream& operator<<(std::ostream& out, const details::TQuaternion& v) noexcept; + } // namespace math } // namespace filament diff --git a/libs/mathio/src/ostream.cpp b/libs/mathio/src/ostream.cpp index f66e04b24c..a3e442424a 100644 --- a/libs/mathio/src/ostream.cpp +++ b/libs/mathio/src/ostream.cpp @@ -112,6 +112,11 @@ std::ostream& operator<<(std::ostream& out, const details::TMat44& v) noexcep return printMatrix(out, v.asArray(), 4, 4); } +template +std::ostream& operator<<(std::ostream& out, const details::TQuaternion& v) noexcept { + return printQuat(out, v); +} + template std::ostream& operator<<(std::ostream& out, const details::TVec2& v) noexcept; template std::ostream& operator<<(std::ostream& out, const details::TVec2& v) noexcept; template std::ostream& operator<<(std::ostream& out, const details::TVec2& v) noexcept; @@ -154,5 +159,9 @@ template std::ostream& operator<<(std::ostream& out, const details::TMat33& v) noexcept; template std::ostream& operator<<(std::ostream& out, const details::TMat44& v) noexcept; +template std::ostream& operator<<(std::ostream& out, const details::TQuaternion& v) noexcept; +template std::ostream& operator<<(std::ostream& out, const details::TQuaternion& v) noexcept; +template std::ostream& operator<<(std::ostream& out, const details::TQuaternion& v) noexcept; + } // namespace math } // namespace filament From 4e4d4f9ee3d420a01dd84e52ac00eaf9aa9d58f8 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Thu, 29 Oct 2020 17:03:24 -0700 Subject: [PATCH 10/21] Vulkan: add blit support for depth and MRT. --- filament/backend/src/vulkan/VulkanContext.cpp | 118 ++++++++++++++++++ filament/backend/src/vulkan/VulkanContext.h | 9 ++ filament/backend/src/vulkan/VulkanDriver.cpp | 99 +++------------ filament/backend/src/vulkan/VulkanHandles.h | 1 + 4 files changed, 146 insertions(+), 81 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanContext.cpp b/filament/backend/src/vulkan/VulkanContext.cpp index 16fbc746e4..16fb685db7 100644 --- a/filament/backend/src/vulkan/VulkanContext.cpp +++ b/filament/backend/src/vulkan/VulkanContext.cpp @@ -31,6 +31,7 @@ #pragma clang diagnostic pop #include "VulkanContext.h" +#include "VulkanHandles.h" #include "VulkanUtility.h" #include @@ -831,5 +832,122 @@ VkImageLayout getTextureLayout(TextureUsage usage) { return VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; } +static void blit(VkImageAspectFlags aspect, VkFilter filter, VulkanContext* context, + const VulkanRenderTarget* srcTarget, VulkanAttachment src, VulkanAttachment dst, + const VkOffset3D srcRect[2], const VkOffset3D dstRect[2], VkCommandBuffer cmdbuffer) { + const VkImageBlit blitRegions[1] = {{ + .srcSubresource = { aspect, src.level, src.layer, 1 }, + .srcOffsets = { srcRect[0], srcRect[1] }, + .dstSubresource = { aspect, dst.level, dst.layer, 1 }, + .dstOffsets = { dstRect[0], dstRect[1] } + }}; + + const VkExtent2D srcExtent = srcTarget->getExtent(); + + const VkImageResolve resolveRegions[1] = {{ + .srcSubresource = { aspect, src.level, src.layer, 1 }, + .srcOffset = srcRect[0], + .dstSubresource = { aspect, dst.level, dst.layer, 1 }, + .dstOffset = dstRect[0], + .extent = { srcExtent.width, srcExtent.height, 1 } + }}; + + VulkanTexture::transitionImageLayout(cmdbuffer, src.image, VK_IMAGE_LAYOUT_UNDEFINED, + VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, src.level, 1, 1, aspect); + + VulkanTexture::transitionImageLayout(cmdbuffer, dst.image, VK_IMAGE_LAYOUT_UNDEFINED, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, dst.level, 1, 1, aspect); + + if (src.texture && src.texture->samples > 1 && dst.texture && dst.texture->samples == 1) { + vkCmdResolveImage(cmdbuffer, src.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, dst.image, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, resolveRegions); + } else { + vkCmdBlitImage(cmdbuffer, src.image, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, dst.image, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, blitRegions, filter); + } + + if (src.texture) { + VulkanTexture::transitionImageLayout(cmdbuffer, src.image, VK_IMAGE_LAYOUT_UNDEFINED, + getTextureLayout(src.texture->usage), src.level, 1, 1, aspect); + } else if (!context->currentSurface->headlessQueue) { + VulkanTexture::transitionImageLayout(cmdbuffer, src.image, VK_IMAGE_LAYOUT_UNDEFINED, + VK_IMAGE_LAYOUT_PRESENT_SRC_KHR, src.level, 1, 1, aspect); + } + + // Determine the desired texture layout for the destination while ensuring that the default + // render target is supported, which has no associated texture. + const VkImageLayout desiredLayout = dst.texture ? getTextureLayout(dst.texture->usage) : + getSwapContext(*context).attachment.layout; + + VulkanTexture::transitionImageLayout(cmdbuffer, dst.image, VK_IMAGE_LAYOUT_UNDEFINED, + desiredLayout, dst.level, 1, 1, aspect); +} + +void blitDepth(VulkanContext* context, const VulkanRenderTarget* dstTarget, + const VkOffset3D dstRect[2], const VulkanRenderTarget* srcTarget, + const VkOffset3D srcRect[2]) { + const VulkanAttachment src = srcTarget->getDepth(); + const VulkanAttachment dst = dstTarget->getDepth(); + const VkImageAspectFlags aspect = VK_IMAGE_ASPECT_DEPTH_BIT; + + // In debug builds, verify that the two render targets have blittable formats. +#ifndef NDEBUG + const VkPhysicalDevice gpu = context->physicalDevice; + VkFormatProperties info; + vkGetPhysicalDeviceFormatProperties(gpu, src.format, &info); + if (!ASSERT_POSTCONDITION_NON_FATAL(info.optimalTilingFeatures & VK_FORMAT_FEATURE_BLIT_SRC_BIT, + "Depth format is not blittable")) { + return; + } + vkGetPhysicalDeviceFormatProperties(gpu, dst.format, &info); + if (!ASSERT_POSTCONDITION_NON_FATAL(info.optimalTilingFeatures & VK_FORMAT_FEATURE_BLIT_DST_BIT, + "Depth format is not blittable")) { + return; + } +#endif + + if (!context->currentCommands) { + VkCommandBuffer cmdbuf = acquireWorkCommandBuffer(*context); + blit(aspect, VK_FILTER_NEAREST, context, srcTarget, src, dst, srcRect, dstRect, cmdbuf); + flushWorkCommandBuffer(*context); + } else { + VkCommandBuffer cmdbuf = context->currentCommands->cmdbuffer; + blit(aspect, VK_FILTER_NEAREST, context, srcTarget, src, dst, srcRect, dstRect, cmdbuf); + } +} + +void blitColor(VulkanContext* context, const VulkanRenderTarget* dstTarget, + const VkOffset3D dstRect[2], const VulkanRenderTarget* srcTarget, + const VkOffset3D srcRect[2], VkFilter filter, int targetIndex) { + const VulkanAttachment src = srcTarget->getColor(targetIndex); + const VulkanAttachment dst = dstTarget->getColor(0); + const VkImageAspectFlags aspect = VK_IMAGE_ASPECT_COLOR_BIT; + + // In debug builds, verify that the two render targets have blittable formats. +#ifndef NDEBUG + const VkPhysicalDevice gpu = context->physicalDevice; + VkFormatProperties info; + vkGetPhysicalDeviceFormatProperties(gpu, src.format, &info); + if (!ASSERT_POSTCONDITION_NON_FATAL(info.optimalTilingFeatures & VK_FORMAT_FEATURE_BLIT_SRC_BIT, + "Source format is not blittable")) { + return; + } + vkGetPhysicalDeviceFormatProperties(gpu, dst.format, &info); + if (!ASSERT_POSTCONDITION_NON_FATAL(info.optimalTilingFeatures & VK_FORMAT_FEATURE_BLIT_DST_BIT, + "Destination format is not blittable")) { + return; + } +#endif + + if (!context->currentCommands) { + VkCommandBuffer cmdbuf = acquireWorkCommandBuffer(*context); + blit(aspect, filter, context, srcTarget, src, dst, srcRect, dstRect, cmdbuf); + flushWorkCommandBuffer(*context); + } else { + VkCommandBuffer cmdbuf = context->currentCommands->cmdbuffer; + blit(aspect, filter, context, srcTarget, src, dst, srcRect, dstRect, cmdbuf); + } +} + } // namespace filament } // namespace backend diff --git a/filament/backend/src/vulkan/VulkanContext.h b/filament/backend/src/vulkan/VulkanContext.h index 0e011b0f35..34cca2daef 100644 --- a/filament/backend/src/vulkan/VulkanContext.h +++ b/filament/backend/src/vulkan/VulkanContext.h @@ -47,6 +47,7 @@ constexpr VkAllocationCallbacks* VKALLOC = nullptr; constexpr static const int VK_REQUIRED_VERSION_MAJOR = 1; constexpr static const int VK_REQUIRED_VERSION_MINOR = 0; +struct VulkanRenderTarget; struct VulkanSurfaceContext; struct VulkanTexture; @@ -176,6 +177,14 @@ void flushWorkCommandBuffer(VulkanContext& context); void createFinalDepthBuffer(VulkanContext& context, VulkanSurfaceContext& sc, VkFormat depthFormat); VkImageLayout getTextureLayout(TextureUsage usage); +void blitDepth(VulkanContext* context, const VulkanRenderTarget* dstTarget, + const VkOffset3D dstRect[2], const VulkanRenderTarget* srcTarget, + const VkOffset3D srcRect[2]); + +void blitColor(VulkanContext* context, const VulkanRenderTarget* dstTarget, + const VkOffset3D dstRect[2], const VulkanRenderTarget* srcTarget, + const VkOffset3D srcRect[2], VkFilter filter, int index); + } // namespace filament } // namespace backend diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 3a81fc2b07..67a4233acb 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -1461,104 +1461,41 @@ void VulkanDriver::blit(TargetBufferFlags buffers, Handle dst, V Handle src, Viewport srcRect, SamplerMagFilter filter) { VulkanRenderTarget* dstTarget = handle_cast(mHandleMap, dst); VulkanRenderTarget* srcTarget = handle_cast(mHandleMap, src); - const int targetIndex = 0; // TODO: support MRT in blit - // In debug builds, verify that the two render targets have blittable formats. -#ifndef NDEBUG - const VkPhysicalDevice gpu = mContext.physicalDevice; - VkFormatProperties info; - vkGetPhysicalDeviceFormatProperties(gpu, srcTarget->getColor(targetIndex).format, &info); - if (!ASSERT_POSTCONDITION_NON_FATAL(info.optimalTilingFeatures & VK_FORMAT_FEATURE_BLIT_SRC_BIT, - "Source format is not blittable")) { - return; - } - vkGetPhysicalDeviceFormatProperties(gpu, dstTarget->getColor(targetIndex).format, &info); - if (!ASSERT_POSTCONDITION_NON_FATAL(info.optimalTilingFeatures & VK_FORMAT_FEATURE_BLIT_DST_BIT, - "Destination format is not blittable")) { - return; - } - if (any(buffers & TargetBufferFlags::DEPTH)) { - utils::slog.w << "Depth blits are not yet supported." << utils::io::endl; - } -#endif + VkFilter vkfilter = filter == SamplerMagFilter::NEAREST ? VK_FILTER_NEAREST : VK_FILTER_LINEAR; const VkExtent2D srcExtent = srcTarget->getExtent(); - const VkExtent2D dstExtent = dstTarget->getExtent(); - const int32_t srcLeft = std::min(srcRect.left, (int32_t) srcExtent.width); const int32_t srcBottom = std::min(srcRect.bottom, (int32_t) srcExtent.height); const int32_t srcRight = std::min(srcRect.left + srcRect.width, srcExtent.width); const int32_t srcTop = std::min(srcRect.bottom + srcRect.height, srcExtent.height); - const uint32_t srcLevel = srcTarget->getColor(targetIndex).level; - const uint32_t srcLayer = srcTarget->getColor(targetIndex).layer; + const VkOffset3D srcOffsets[2] = { { srcLeft, srcBottom, 0 }, { srcRight, srcTop, 1 }}; + const VkExtent2D dstExtent = dstTarget->getExtent(); const int32_t dstLeft = std::min(dstRect.left, (int32_t) dstExtent.width); const int32_t dstBottom = std::min(dstRect.bottom, (int32_t) dstExtent.height); const int32_t dstRight = std::min(dstRect.left + dstRect.width, dstExtent.width); const int32_t dstTop = std::min(dstRect.bottom + dstRect.height, dstExtent.height); - const uint32_t dstLevel = dstTarget->getColor(targetIndex).level; - const uint32_t dstLayer = dstTarget->getColor(targetIndex).layer; + const VkOffset3D dstOffsets[2] = { { dstLeft, dstBottom, 0 }, { dstRight, dstTop, 1 }}; - const VkImageAspectFlags aspect = VK_IMAGE_ASPECT_COLOR_BIT; + if (any(buffers & TargetBufferFlags::DEPTH) && srcTarget->hasDepth() && dstTarget->hasDepth()) { + blitDepth(&mContext, dstTarget, dstOffsets, srcTarget, srcOffsets); + } - const VkImageBlit blitRegions[1] = {{ - .srcSubresource = { aspect, srcLevel, srcLayer, 1 }, - .srcOffsets = { { srcLeft, srcBottom, 0 }, { srcRight, srcTop, 1 }}, - .dstSubresource = { aspect, dstLevel, dstLayer, 1 }, - .dstOffsets = { { dstLeft, dstBottom, 0 }, { dstRight, dstTop, 1 }} - }}; + if (any(buffers & TargetBufferFlags::COLOR0)) { + blitColor(&mContext, dstTarget, dstOffsets, srcTarget, srcOffsets, vkfilter, 0); + } - const VkImageResolve resolveRegions[1] = {{ - .srcSubresource = { aspect, srcLevel, srcLayer, 1 }, - .srcOffset = { srcLeft, srcBottom, 0 }, - .dstSubresource = { aspect, dstLevel, dstLayer, 1 }, - .dstOffset = { dstLeft, dstBottom, 0 }, - .extent = { srcExtent.width, srcExtent.height, 1 } - }}; + if (any(buffers & TargetBufferFlags::COLOR1)) { + blitColor(&mContext, dstTarget, dstOffsets, srcTarget, srcOffsets, vkfilter, 1); + } - const VulkanTexture* srcTexture = srcTarget->getColor(targetIndex).texture; - const VulkanTexture* dstTexture = dstTarget->getColor(targetIndex).texture; + if (any(buffers & TargetBufferFlags::COLOR2)) { + blitColor(&mContext, dstTarget, dstOffsets, srcTarget, srcOffsets, vkfilter, 2); + } - auto vkblit = [=](VkCommandBuffer cmdbuffer) { - VkImage srcImage = srcTarget->getColor(targetIndex).image; - VulkanTexture::transitionImageLayout(cmdbuffer, srcImage, VK_IMAGE_LAYOUT_UNDEFINED, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, srcLevel, 1, 1, aspect); - - VkImage dstImage = dstTarget->getColor(targetIndex).image; - VulkanTexture::transitionImageLayout(cmdbuffer, dstImage, VK_IMAGE_LAYOUT_UNDEFINED, - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, dstLevel, 1, 1, aspect); - - if (srcTexture && srcTexture->samples > 1 && dstTexture && dstTexture->samples == 1) { - vkCmdResolveImage(cmdbuffer, srcImage, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, dstImage, - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, resolveRegions); - } else { - vkCmdBlitImage(cmdbuffer, srcImage, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, dstImage, - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, blitRegions, - filter == SamplerMagFilter::NEAREST ? VK_FILTER_NEAREST : VK_FILTER_LINEAR); - } - - if (srcTexture) { - VulkanTexture::transitionImageLayout(cmdbuffer, srcImage, VK_IMAGE_LAYOUT_UNDEFINED, - getTextureLayout(srcTexture->usage), srcLevel, 1, 1, aspect); - } else if (!mContext.currentSurface->headlessQueue) { - VulkanTexture::transitionImageLayout(cmdbuffer, srcImage, VK_IMAGE_LAYOUT_UNDEFINED, - VK_IMAGE_LAYOUT_PRESENT_SRC_KHR, srcLevel, 1, 1, aspect); - } - - // Determine the desired texture layout for the destination while ensuring that the default - // render target is supported, which has no associated texture. - const VkImageLayout desiredLayout = dstTexture ? getTextureLayout(dstTexture->usage) : - getSwapContext(mContext).attachment.layout; - - VulkanTexture::transitionImageLayout(cmdbuffer, dstImage, VK_IMAGE_LAYOUT_UNDEFINED, - desiredLayout, dstLevel, 1, 1, aspect); - }; - - if (!mContext.currentCommands) { - vkblit(acquireWorkCommandBuffer(mContext)); - flushWorkCommandBuffer(mContext); - } else { - vkblit(mContext.currentCommands->cmdbuffer); + if (any(buffers & TargetBufferFlags::COLOR3)) { + blitColor(&mContext, dstTarget, dstOffsets, srcTarget, srcOffsets, vkfilter, 3); } } diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index ebaf55636c..3d2697b1e3 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -61,6 +61,7 @@ struct VulkanRenderTarget : private HwRenderTarget { int getColorTargetCount(const VulkanRenderPass& pass) const; bool invalidate(); uint8_t getSamples() const { return mSamples; } + bool hasDepth() const { return mDepth.format != VK_FORMAT_UNDEFINED; } private: VulkanAttachment mColor[MRT::TARGET_COUNT] = {}; VulkanAttachment mDepth = {}; From 8a638346076adbb378b34539e2433742c1da7363 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Fri, 30 Oct 2020 14:06:43 -0700 Subject: [PATCH 11/21] Vulkan: fix layout mismatch after blitting to swap chain. --- filament/backend/src/vulkan/VulkanContext.cpp | 2 +- filament/backend/src/vulkan/VulkanFboCache.cpp | 7 ++++++- filament/backend/src/vulkan/VulkanHandles.cpp | 2 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanContext.cpp b/filament/backend/src/vulkan/VulkanContext.cpp index 16fb685db7..c263f49636 100644 --- a/filament/backend/src/vulkan/VulkanContext.cpp +++ b/filament/backend/src/vulkan/VulkanContext.cpp @@ -871,7 +871,7 @@ static void blit(VkImageAspectFlags aspect, VkFilter filter, VulkanContext* cont getTextureLayout(src.texture->usage), src.level, 1, 1, aspect); } else if (!context->currentSurface->headlessQueue) { VulkanTexture::transitionImageLayout(cmdbuffer, src.image, VK_IMAGE_LAYOUT_UNDEFINED, - VK_IMAGE_LAYOUT_PRESENT_SRC_KHR, src.level, 1, 1, aspect); + VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, src.level, 1, 1, aspect); } // Determine the desired texture layout for the destination while ensuring that the default diff --git a/filament/backend/src/vulkan/VulkanFboCache.cpp b/filament/backend/src/vulkan/VulkanFboCache.cpp index ba02c930f4..21f56cd67a 100644 --- a/filament/backend/src/vulkan/VulkanFboCache.cpp +++ b/filament/backend/src/vulkan/VulkanFboCache.cpp @@ -139,7 +139,12 @@ VkRenderPass VulkanFboCache::getRenderPass(RenderPassKey config) noexcept { struct { VkImageLayout subpass, initial, final; } colorLayouts[MRT::TARGET_COUNT]; if (isSwapChain) { colorLayouts[0].subpass = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; - colorLayouts[0].initial = discard ? VK_IMAGE_LAYOUT_UNDEFINED : colorLayouts[0].subpass; + + // It is legal to always use UNDEFINED for "initial", but we wish to avoid warnings + // when the load op is LOAD. + colorLayouts[0].initial = discard ? VK_IMAGE_LAYOUT_UNDEFINED : + VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; + colorLayouts[0].final = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; } else { for (int i = 0; i < MRT::TARGET_COUNT; i++) { diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index ea83f841e0..911643c7e3 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -773,6 +773,7 @@ VkImageView VulkanTexture::getImageView(int level, int layer, VkImageAspectFlags } // TODO: replace the last 4 args with VkImageSubresourceRange +// TODO: replace this function with a flexible thin wrapper over image barrier creation void VulkanTexture::transitionImageLayout(VkCommandBuffer cmd, VkImage image, VkImageLayout oldLayout, VkImageLayout newLayout, uint32_t miplevel, uint32_t layerCount, uint32_t levelCount, VkImageAspectFlags aspect) { @@ -816,6 +817,7 @@ void VulkanTexture::transitionImageLayout(VkCommandBuffer cmd, VkImage image, // We support PRESENT as a target layout to allow blitting from the swap chain. // See also makeSwapChainPresentable(). + case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: case VK_IMAGE_LAYOUT_PRESENT_SRC_KHR: barrier.srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; barrier.dstAccessMask = 0; From 0db70579830c2459d9b1d72834e1288e6242a1bb Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Fri, 30 Oct 2020 14:24:31 -0700 Subject: [PATCH 12/21] Vulkan: disable unreliable blit diagnostics. --- filament/backend/src/vulkan/VulkanContext.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanContext.cpp b/filament/backend/src/vulkan/VulkanContext.cpp index c263f49636..5ee0d3df79 100644 --- a/filament/backend/src/vulkan/VulkanContext.cpp +++ b/filament/backend/src/vulkan/VulkanContext.cpp @@ -36,6 +36,8 @@ #include +#define FILAMENT_VULKAN_CHECK_BLIT_FORMAT 0 + namespace filament { namespace backend { @@ -890,8 +892,7 @@ void blitDepth(VulkanContext* context, const VulkanRenderTarget* dstTarget, const VulkanAttachment dst = dstTarget->getDepth(); const VkImageAspectFlags aspect = VK_IMAGE_ASPECT_DEPTH_BIT; - // In debug builds, verify that the two render targets have blittable formats. -#ifndef NDEBUG +#if FILAMENT_VULKAN_CHECK_BLIT_FORMAT const VkPhysicalDevice gpu = context->physicalDevice; VkFormatProperties info; vkGetPhysicalDeviceFormatProperties(gpu, src.format, &info); @@ -923,8 +924,7 @@ void blitColor(VulkanContext* context, const VulkanRenderTarget* dstTarget, const VulkanAttachment dst = dstTarget->getColor(0); const VkImageAspectFlags aspect = VK_IMAGE_ASPECT_COLOR_BIT; - // In debug builds, verify that the two render targets have blittable formats. -#ifndef NDEBUG +#if FILAMENT_VULKAN_CHECK_BLIT_FORMAT const VkPhysicalDevice gpu = context->physicalDevice; VkFormatProperties info; vkGetPhysicalDeviceFormatProperties(gpu, src.format, &info); From 468992f4cdeae9f0fa7651a9f7284a4b7f4fe3e3 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Sat, 31 Oct 2020 12:54:30 -0700 Subject: [PATCH 13/21] gltfio: do not segfault on invalid primitives. --- libs/gltfio/src/AssetLoader.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libs/gltfio/src/AssetLoader.cpp b/libs/gltfio/src/AssetLoader.cpp index 5285a454ab..7dd6a96a0f 100644 --- a/libs/gltfio/src/AssetLoader.cpp +++ b/libs/gltfio/src/AssetLoader.cpp @@ -483,7 +483,7 @@ bool FAssetLoader::createPrimitive(const cgltf_primitive* inPrim, Primitive* out }; // In glTF, each primitive may or may not have an index buffer. - IndexBuffer* indices; + IndexBuffer* indices = nullptr; const cgltf_accessor* accessor = inPrim->indices; if (accessor) { IndexBuffer::IndexType indexType; @@ -500,7 +500,7 @@ bool FAssetLoader::createPrimitive(const cgltf_primitive* inPrim, Primitive* out BufferSlot slot = { accessor }; slot.indexBuffer = indices; addBufferSlot(slot); - } else { + } else if (inPrim->attributes_count > 0) { // If a primitive does not have an index buffer, generate a trivial one now. const uint32_t vertexCount = inPrim->attributes[0].data->count; @@ -677,6 +677,11 @@ bool FAssetLoader::createPrimitive(const cgltf_primitive* inPrim, Primitive* out } } + if (vertexCount == 0) { + slog.e << "Empty vertex buffer in " << name << io::endl; + return false; + } + vbb.vertexCount(vertexCount); // If an ubershader is used, then we provide a single dummy buffer for all unfulfilled vertex From 47521d70a1f6376cbeae58aa169133b42c262953 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Sat, 31 Oct 2020 12:54:30 -0700 Subject: [PATCH 14/21] gltfio: fix ASAN issue when consuming invalid animation. Release builds do not call cgltf_validate() so it was possible to read out-of-bounds animation data when encountering a badly formed glTF file with mismatched counts between sampler inputs and outputs. --- libs/gltfio/src/Animator.cpp | 37 ++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/libs/gltfio/src/Animator.cpp b/libs/gltfio/src/Animator.cpp index 92de0a7798..c762147f5b 100644 --- a/libs/gltfio/src/Animator.cpp +++ b/libs/gltfio/src/Animator.cpp @@ -139,6 +139,31 @@ static void setTransformType(const cgltf_animation_channel& src, Channel& dst) { } } +static bool validateAnimation(const cgltf_animation& anim) { + for (cgltf_size j = 0; j < anim.channels_count; ++j) { + const cgltf_animation_channel& channel = anim.channels[j]; + const cgltf_animation_sampler* sampler = channel.sampler; + if (!channel.target_node) { + continue; + } + if (!channel.sampler) { + return false; + } + cgltf_size components = 1; + if (channel.target_path == cgltf_animation_path_type_weights) { + if (!channel.target_node->mesh || !channel.target_node->mesh->primitives_count) { + return false; + } + components = channel.target_node->mesh->primitives[0].targets_count; + } + cgltf_size values = sampler->interpolation == cgltf_interpolation_type_cubic_spline ? 3 : 1; + if (sampler->input->count * components * values != sampler->output->count) { + return false; + } + } + return true; +} + Animator::Animator(FFilamentAsset* asset, FFilamentInstance* instance) { assert(asset->mResourcesLoaded && !asset->mIsReleased); mImpl = new AnimatorImpl(); @@ -147,6 +172,16 @@ Animator::Animator(FFilamentAsset* asset, FFilamentInstance* instance) { mImpl->renderableManager = &asset->mEngine->getRenderableManager(); mImpl->transformManager = &asset->mEngine->getTransformManager(); + const cgltf_data* srcAsset = asset->mSourceAsset; + const cgltf_animation* srcAnims = srcAsset->animations; + for (cgltf_size i = 0, len = srcAsset->animations_count; i < len; ++i) { + const cgltf_animation& anim = srcAnims[i]; + if (!validateAnimation(anim)) { + slog.e << "Disabling animation due to validation failure." << io::endl; + return; + } + } + auto addChannels = [](const NodeMap& nodeMap, const cgltf_animation& srcAnim, Animation& dst) { cgltf_animation_channel* srcChannels = srcAnim.channels; cgltf_animation_sampler* srcSamplers = srcAnim.samplers; @@ -163,8 +198,6 @@ Animator::Animator(FFilamentAsset* asset, FFilamentInstance* instance) { }; // Loop over the glTF animation definitions. - const cgltf_data* srcAsset = asset->mSourceAsset; - const cgltf_animation* srcAnims = srcAsset->animations; mImpl->animations.resize(srcAsset->animations_count); for (cgltf_size i = 0, len = srcAsset->animations_count; i < len; ++i) { const cgltf_animation& srcAnim = srcAnims[i]; From 70dba2398e37e1913031f55b07b402679e87812a Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Tue, 27 Oct 2020 12:46:16 -0700 Subject: [PATCH 15/21] gltfio: add createInstance() to AssetLoader. Note that this API is on the loader rather than the asset. This is because the loader knows how to create Filament entities by traversing a cgltf node hierarchy. Animation on dynamically added instances is not yet supported. We did not add destroyInstance() because gltfio favors flat arrays for long term storage of entity lists and instance lists, which would be slow to shift. We also wish to discourage create/destroy churn since it is more efficient to pre-allocate instances and selectively add them into the scene. Fixes #3137. --- .../android/filament/gltfio/AssetLoader.java | 27 ++++++++ .../filament/gltfio/FilamentAsset.java | 7 +- libs/gltfio/include/gltfio/AssetLoader.h | 18 +++++ libs/gltfio/include/gltfio/FilamentAsset.h | 1 + libs/gltfio/src/Animator.cpp | 4 +- libs/gltfio/src/AssetLoader.cpp | 67 ++++++++++++++----- libs/gltfio/src/DependencyGraph.cpp | 58 +++++++++++----- libs/gltfio/src/DependencyGraph.h | 8 ++- libs/gltfio/src/FFilamentAsset.h | 4 ++ libs/gltfio/src/ResourceLoader.cpp | 4 +- samples/gltf_instances.cpp | 58 +++++++++------- web/filament-js/filament.d.ts | 1 + web/filament-js/jsbindings.cpp | 4 ++ 13 files changed, 195 insertions(+), 66 deletions(-) diff --git a/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/AssetLoader.java b/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/AssetLoader.java index ba357257b3..a40db6fa3a 100644 --- a/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/AssetLoader.java +++ b/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/AssetLoader.java @@ -152,6 +152,32 @@ public class AssetLoader { return new FilamentAsset(mEngine, nativeAsset); } + /** + * Adds a new instance to an instanced asset. + * + * Use this with caution. It is more efficient to pre-allocate a max number of instances, and + * gradually add them to the scene as needed. Instances can also be "recycled" by removing and + * re-adding them to the scene. + * + * NOTE: destroyInstance() does not exist because gltfio favors flat arrays for storage of + * entity lists and instance lists, which would be slow to shift. We also wish to discourage + * create/destroy churn, as noted above. + * + * This cannot be called after FilamentAsset#releaseSourceData(). + * This cannot be called on a non-instanced asset. + * Animation is not supported in new instances. + * See also AssetLoader#createInstancedAsset(). + */ + @Nullable + @SuppressWarnings("unused") + public FilamentInstance createInstance(@NonNull FilamentAsset asset) { + long nativeInstance = nCreateInstance(mNativeObject, asset.getNativeObject()); + if (nativeInstance == 0) { + return null; + } + return new FilamentInstance(nativeInstance); + } + /** * Allows clients to enable diagnostic shading on newly-loaded assets. */ @@ -175,6 +201,7 @@ public class AssetLoader { private static native long nCreateAssetFromJson(long nativeLoader, Buffer buffer, int remaining); private static native long nCreateInstancedAsset(long nativeLoader, Buffer buffer, int remaining, long[] nativeInstances); + private static native long nCreateInstance(long nativeLoader, long nativeAsset); private static native void nEnableDiagnostics(long nativeLoader, boolean enable); private static native void nDestroyAsset(long nativeLoader, long nativeAsset); } diff --git a/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/FilamentAsset.java b/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/FilamentAsset.java index 7cc9eb3258..81b9560168 100644 --- a/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/FilamentAsset.java +++ b/android/gltfio-android/src/main/java/com/google/android/filament/gltfio/FilamentAsset.java @@ -197,7 +197,11 @@ public class FilamentAsset { if (mAnimator != null) { return mAnimator; } - mAnimator = new Animator(nGetAnimator(getNativeObject())); + long nativeAnimator = nGetAnimator(getNativeObject()); + if (nativeAnimator == 0) { + throw new IllegalStateException("Unable to create animator"); + } + mAnimator = new Animator(nativeAnimator); return mAnimator; } @@ -215,6 +219,7 @@ public class FilamentAsset { * * This should only be called after ResourceLoader#loadResources(). * If using Animator, this should be called after getAnimator(). + * If this is an instanced asset, this prevents creation of new instances. */ public void releaseSourceData() { nReleaseSourceData(mNativeObject); diff --git a/libs/gltfio/include/gltfio/AssetLoader.h b/libs/gltfio/include/gltfio/AssetLoader.h index e9ee733b64..b5e03226fb 100644 --- a/libs/gltfio/include/gltfio/AssetLoader.h +++ b/libs/gltfio/include/gltfio/AssetLoader.h @@ -178,6 +178,24 @@ public: FilamentAsset* createInstancedAsset(const uint8_t* bytes, uint32_t numBytes, FilamentInstance** instances, size_t numInstances); + /** + * Adds a new instance to an instanced asset. + * + * Use this with caution. It is more efficient to pre-allocate a max number of instances, and + * gradually add them to the scene as needed. Instances can also be "recycled" by removing and + * re-adding them to the scene. + * + * NOTE: destroyInstance() does not exist because gltfio favors flat arrays for storage of + * entity lists and instance lists, which would be slow to shift. We also wish to discourage + * create/destroy churn, as noted above. + * + * This cannot be called after FilamentAsset::releaseSourceData(). + * This cannot be called on a non-instanced asset. + * Animation is not supported in new instances. + * See also AssetLoader::createInstancedAsset(). + */ + FilamentInstance* createInstance(FilamentAsset* primary); + /** * Takes a pointer to an opaque pipeline object and returns a bundle of Filament objects. * diff --git a/libs/gltfio/include/gltfio/FilamentAsset.h b/libs/gltfio/include/gltfio/FilamentAsset.h index 3aea411af5..7cae8163ef 100644 --- a/libs/gltfio/include/gltfio/FilamentAsset.h +++ b/libs/gltfio/include/gltfio/FilamentAsset.h @@ -215,6 +215,7 @@ public: * * This should only be called after ResourceLoader::loadResources(). * If using Animator, this should be called after getAnimator(). + * If this is an instanced asset, this prevents creation of new instances. */ void releaseSourceData() noexcept; diff --git a/libs/gltfio/src/Animator.cpp b/libs/gltfio/src/Animator.cpp index c762147f5b..63e7a16410 100644 --- a/libs/gltfio/src/Animator.cpp +++ b/libs/gltfio/src/Animator.cpp @@ -223,7 +223,7 @@ Animator::Animator(FFilamentAsset* asset, FFilamentInstance* instance) { // Import each glTF channel into a custom data structure. if (instance) { addChannels(instance->nodeMap, srcAnim, dstAnim); - } else if (asset->mInstances.empty()) { + } else if (!asset->isInstanced()) { addChannels(asset->mNodeMap, srcAnim, dstAnim); } else { for (FFilamentInstance* instance : asset->mInstances) { @@ -413,7 +413,7 @@ void Animator::updateBoneMatrices() { if (mImpl->instance) { update(mImpl->instance->skins, mImpl->boneMatrices); - } else if (mImpl->asset->mInstances.empty()) { + } else if (!mImpl->asset->isInstanced()) { update(mImpl->asset->mSkins, mImpl->boneMatrices); } else { for (FFilamentInstance* instance : mImpl->asset->mInstances) { diff --git a/libs/gltfio/src/AssetLoader.cpp b/libs/gltfio/src/AssetLoader.cpp index 7dd6a96a0f..71adbe6696 100644 --- a/libs/gltfio/src/AssetLoader.cpp +++ b/libs/gltfio/src/AssetLoader.cpp @@ -97,6 +97,7 @@ struct FAssetLoader : public AssetLoader { FFilamentAsset* createAssetFromBinary(const uint8_t* bytes, uint32_t nbytes); FFilamentAsset* createInstancedAsset(const uint8_t* bytes, uint32_t numBytes, FilamentInstance** instances, size_t numInstances); + FilamentInstance* createInstance(FFilamentAsset* primary); bool createAssets(const uint8_t* bytes, uint32_t numBytes, FilamentAsset** assets, size_t numAssets); @@ -122,6 +123,7 @@ struct FAssetLoader : public AssetLoader { } void createAsset(const cgltf_data* srcAsset, size_t numInstances); + FilamentInstance* createInstance(FFilamentAsset* primary, const cgltf_scene* scene); void createEntity(const cgltf_node* node, Entity parent, bool enableLight, FFilamentInstance* instance); void createRenderable(const cgltf_node* node, Entity entity, const char* name); @@ -217,6 +219,26 @@ FFilamentAsset* FAssetLoader::createInstancedAsset(const uint8_t* bytes, uint32_ return mResult; } +FilamentInstance* FAssetLoader::createInstance(FFilamentAsset* primary) { + if (primary->mIsReleased) { + slog.e << "Source data has been released; asset is frozen." << io::endl; + return nullptr; + } + if (!primary->isInstanced()) { + slog.e << "Cannot add an instance to a non-instanced asset." << io::endl; + return nullptr; + } + const cgltf_data* srcAsset = primary->mSourceAsset; + const cgltf_scene* scene = srcAsset->scene ? srcAsset->scene : srcAsset->scenes; + if (!scene) { + slog.e << "There is no scene in the asset." << io::endl; + return nullptr; + } + FilamentInstance* instance = createInstance(primary, scene); + primary->mDependencyGraph.refinalize(); + return instance; +} + void FAssetLoader::createAsset(const cgltf_data* srcAsset, size_t numInstances) { SYSTRACE_CALL(); #if !GLTFIO_DRACO_SUPPORTED @@ -255,23 +277,9 @@ void FAssetLoader::createAsset(const cgltf_data* srcAsset, size_t numInstances) // buffers and index buffers) and MatInstanceCache (materials and textures) help avoid // needless duplication of resources. for (size_t index = 0; index < numInstances; ++index) { - // Create a root node within each instance that is a child of the primary root. - auto rootTransform = mTransformManager.getInstance(mResult->mRoot); - Entity instanceRoot = mEntityManager.create(); - mTransformManager.create(instanceRoot, rootTransform); - - // Create an instance object, which is a just a lightweight wrapper around a vector of - // entities and a lazily created animator. - FFilamentInstance* instance = new FFilamentInstance; - instance->root = instanceRoot; - instance->animator = nullptr; - instance->owner = mResult; - mResult->mInstances.push_back(instance); - - // For each scene root, recursively create all entities. - for (cgltf_size i = 0, len = scene->nodes_count; i < len; ++i) { - cgltf_node** nodes = scene->nodes; - createEntity(nodes[i], instanceRoot, index == 0, instance); + if (createInstance(mResult, scene) == nullptr) { + mError = true; + break; } } } @@ -302,6 +310,27 @@ void FAssetLoader::createAsset(const cgltf_data* srcAsset, size_t numInstances) } } +FilamentInstance* FAssetLoader::createInstance(FFilamentAsset* primary, const cgltf_scene* scene) { + auto rootTransform = mTransformManager.getInstance(primary->mRoot); + Entity instanceRoot = mEntityManager.create(); + mTransformManager.create(instanceRoot, rootTransform); + + // Create an instance object, which is a just a lightweight wrapper around a vector of + // entities and a lazily created animator. + FFilamentInstance* instance = new FFilamentInstance; + instance->root = instanceRoot; + instance->animator = nullptr; + instance->owner = primary; + primary->mInstances.push_back(instance); + + // For each scene root, recursively create all entities. + for (cgltf_size i = 0, len = scene->nodes_count; i < len; ++i) { + cgltf_node** nodes = scene->nodes; + createEntity(nodes[i], instanceRoot, false, instance); + } + return instance; +} + void FAssetLoader::createEntity(const cgltf_node* node, Entity parent, bool enableLight, FFilamentInstance* instance) { Entity entity = mEntityManager.create(); @@ -1129,6 +1158,10 @@ FilamentAsset* AssetLoader::createInstancedAsset(const uint8_t* bytes, uint32_t return upcast(this)->createInstancedAsset(bytes, numBytes, instances, numInstances); } +FilamentInstance* AssetLoader::createInstance(FilamentAsset* asset) { + return upcast(this)->createInstance(upcast(asset)); +} + FilamentAsset* AssetLoader::createAssetFromHandle(const void* handle) { const cgltf_data* sourceAsset = (const cgltf_data*) handle; upcast(this)->createAsset(sourceAsset, 0); diff --git a/libs/gltfio/src/DependencyGraph.cpp b/libs/gltfio/src/DependencyGraph.cpp index 1408449d28..24812b03c3 100644 --- a/libs/gltfio/src/DependencyGraph.cpp +++ b/libs/gltfio/src/DependencyGraph.cpp @@ -34,7 +34,12 @@ size_t DependencyGraph::popRenderables(Entity* result, size_t count) noexcept { } void DependencyGraph::addEdge(Entity entity, MaterialInstance* mi) { - assert(!mFinalized); + + // Permit adding an Entity-Material edge to a finalized graph as long as the material is already + // known. Since we already encountered this material instance, we already know what textures it + // is associated with. + assert(!mFinalized || mMaterialToEntity.find(mi) != mMaterialToEntity.end()); + mMaterialToEntity[mi].insert(entity); mEntityToMaterial[entity].materials.insert(mi); } @@ -57,12 +62,42 @@ void DependencyGraph::finalize() { mFinalized = true; } +void DependencyGraph::refinalize() { + assert(mFinalized); + for (auto pair : mMaterialToEntity) { + auto material = pair.first; + if (mMaterialToTexture.find(material) == mMaterialToTexture.end()) { + markAsReady(material); + } else { + checkReadiness(material); + } + } +} + void DependencyGraph::addEdge(Texture* texture, MaterialInstance* material, const char* parameter) { assert(mFinalized); mTextureToMaterial[texture].insert(material); mMaterialToTexture.at(material).params.at(parameter) = getStatus(texture); } +void DependencyGraph::checkReadiness(Material* material) { + auto& status = mMaterialToTexture.at(material); + + // Check this material's texture parameters, there are 5 in the worst case. + bool materialIsReady = true; + for (auto pair : status.params) { + if (!pair.second->ready) { + materialIsReady = false; + break; + } + } + + // If all of its textures are ready, then the material has become ready. + if (materialIsReady) { + markAsReady(material); + } +} + void DependencyGraph::markAsReady(Texture* texture) { assert(texture && mFinalized); mTextureNodes.at(texture)->ready = true; @@ -71,21 +106,7 @@ void DependencyGraph::markAsReady(Texture* texture) { // This is O(n2) but the inner loop is always small. auto& materials = mTextureToMaterial.at(texture); for (auto material : materials) { - auto& status = mMaterialToTexture.at(material); - - // Check this material's texture parameters, there are 5 in the worst case. - bool materialIsReady = true; - for (auto pair : status.params) { - if (!pair.second->ready) { - materialIsReady = false; - break; - } - } - - // If all of its textures are ready, then the material has become ready. - if (materialIsReady) { - markAsReady(material); - } + checkReadiness(material); } } @@ -93,7 +114,10 @@ void DependencyGraph::markAsReady(MaterialInstance* material) { auto& entities = mMaterialToEntity.at(material); for (auto entity : entities) { auto& status = mEntityToMaterial.at(entity); - assert(status.numReadyMaterials < status.materials.size()); + assert(status.numReadyMaterials <= status.materials.size()); + if (status.numReadyMaterials == status.materials.size()) { + continue; + } if (++status.numReadyMaterials == status.materials.size()) { mReadyRenderables.push(entity); } diff --git a/libs/gltfio/src/DependencyGraph.h b/libs/gltfio/src/DependencyGraph.h index 25ae276c11..854571c2f4 100644 --- a/libs/gltfio/src/DependencyGraph.h +++ b/libs/gltfio/src/DependencyGraph.h @@ -34,7 +34,7 @@ namespace gltfio { /** * Internal graph that enables FilamentAsset to discover "ready-to-render" entities by tracking - * the Texture objects that each entity depends on. + * the loading status of Texture objects that each entity depends on. * * Renderables connect to a set of material instances, which in turn connect to a set of parameter * names, which in turn connect to a set of texture objects. These relationships are not easily @@ -72,8 +72,13 @@ public: void addEdge(Material* material, const char* parameter); // This is called at the end of the initial asset loading phase. + // Makes a guarantee that no new material nodes or parameter nodes will be added to the graph. void finalize(); + // This can be called after finalization to allow for dynamic addition of entities. + // It is slower than finalize() because it checks the readiness of existing materials. + void refinalize(); + // These are called after textures have created and decoded. void addEdge(filament::Texture* texture, Material* material, const char* parameter); void markAsReady(filament::Texture* texture); @@ -93,6 +98,7 @@ private: size_t numReadyMaterials = 0; }; + void checkReadiness(Material* material); void markAsReady(Material* material); TextureNode* getStatus(filament::Texture* texture); diff --git a/libs/gltfio/src/FFilamentAsset.h b/libs/gltfio/src/FFilamentAsset.h index c73b56f913..668707aa67 100644 --- a/libs/gltfio/src/FFilamentAsset.h +++ b/libs/gltfio/src/FFilamentAsset.h @@ -212,6 +212,10 @@ struct FFilamentAsset : public FilamentAsset { mDependencyGraph.addEdge(texture, tb.materialInstance, tb.materialParameter); } + bool isInstanced() const { + return mInstances.size() > 0; + } + filament::Engine* mEngine; utils::NameComponentManager* mNameManager; utils::EntityManager* mEntityManager; diff --git a/libs/gltfio/src/ResourceLoader.cpp b/libs/gltfio/src/ResourceLoader.cpp index 16b4364a4d..c25307522f 100644 --- a/libs/gltfio/src/ResourceLoader.cpp +++ b/libs/gltfio/src/ResourceLoader.cpp @@ -388,7 +388,7 @@ bool ResourceLoader::loadResources(FFilamentAsset* asset, bool async) { if (pImpl->mNormalizeSkinningWeights) { normalizeSkinningWeights(asset); } - if (asset->mInstances.empty()) { + if (!asset->isInstanced()) { importSkins(gltf, asset->mNodeMap, asset->mSkins); } else { for (FFilamentInstance* instance : asset->mInstances) { @@ -996,7 +996,7 @@ void ResourceLoader::updateBoundingBoxes(FFilamentAsset* asset) const { SYSTRACE_CALL(); auto& rm = pImpl->mEngine->getRenderableManager(); auto& tm = pImpl->mEngine->getTransformManager(); - NodeMap& nodeMap = asset->mInstances.empty() ? asset->mNodeMap : asset->mInstances[0]->nodeMap; + NodeMap& nodeMap = asset->isInstanced() ? asset->mInstances[0]->nodeMap : asset->mNodeMap; // The purpose of the root node is to give the client a place for custom transforms. // Since it is not part of the source model, it should be ignored when computing the diff --git a/samples/gltf_instances.cpp b/samples/gltf_instances.cpp index 2540b9ce05..39e023d773 100644 --- a/samples/gltf_instances.cpp +++ b/samples/gltf_instances.cpp @@ -51,8 +51,6 @@ using namespace filament::viewer; using namespace gltfio; using namespace utils; -using InstanceHandle = FilamentInstance*; - struct App { Engine* engine; SimpleViewer* viewer; @@ -63,9 +61,8 @@ struct App { MaterialProvider* materials; MaterialSource materialSource = GENERATE_SHADERS; ResourceLoader* resourceLoader = nullptr; - int numInstances = 5; int instanceToAnimate = -1; - InstanceHandle* instances; + std::vector instances; }; static const char* DEFAULT_IBL = "default_env"; @@ -132,7 +129,7 @@ static int handleCommandLineArguments(int argc, char* argv[], App* app) { app->instanceToAnimate = atoi(arg.c_str()); break; case 'n': - app->numInstances = atoi(arg.c_str()); + app->instances.resize(atoi(arg.c_str())); break; case 'i': app->config.iblDirectory = arg; @@ -142,6 +139,9 @@ static int handleCommandLineArguments(int argc, char* argv[], App* app) { break; } } + if (app->instances.empty()) { + app->instances.resize(5); + } return optind; } @@ -184,8 +184,8 @@ int main(int argc, char** argv) { } // Parse the glTF file and create Filament entities. - app.asset = app.loader->createInstancedAsset(buffer.data(), buffer.size(), app.instances, - app.numInstances); + app.asset = app.loader->createInstancedAsset(buffer.data(), buffer.size(), + app.instances.data(), app.instances.size()); buffer.clear(); buffer.shrink_to_fit(); @@ -213,7 +213,6 @@ int main(int argc, char** argv) { if (app.instanceToAnimate > -1) { app.instances[app.instanceToAnimate]->getAnimator(); } - app.asset->releaseSourceData(); auto ibl = FilamentApp::get().getIBL(); if (ibl) { @@ -221,6 +220,20 @@ int main(int argc, char** argv) { } }; + auto arrangeIntoCircle = [&app]() { + auto& tcm = app.engine->getTransformManager(); + auto extent = app.asset->getBoundingBox().extent(); + float max_extent = std::max(std::max(extent.x, extent.y), extent.z); + auto translation = mat4f::translation(float3(max_extent, 0, 0)); + for (size_t inst = 0; inst < app.instances.size(); ++inst) { + FilamentInstance* instance = app.instances[inst]; + auto transformRoot = tcm.getInstance(instance->getRoot()); + float theta = inst * 2.0 * M_PI / app.instances.size(); + auto rotation = mat4f::rotation(theta, float3(0, 0, 1)); + tcm.setTransform(transformRoot, rotation * translation); + } + }; + auto setup = [&](Engine* engine, View* view, Scene* scene) { app.engine = engine; app.names = new NameComponentManager(EntityManager::get()); @@ -228,28 +241,15 @@ int main(int argc, char** argv) { app.materials = (app.materialSource == GENERATE_SHADERS) ? createMaterialGenerator(engine) : createUbershaderLoader(engine); app.loader = AssetLoader::create({engine, app.materials, app.names }); - app.instances = new InstanceHandle[app.numInstances]; if (filename.isEmpty()) { app.asset = app.loader->createInstancedAsset( GLTF_VIEWER_DAMAGEDHELMET_DATA, GLTF_VIEWER_DAMAGEDHELMET_SIZE, - app.instances, app.numInstances); + app.instances.data(), app.instances.size()); } else { loadAsset(filename); } - // Arrange all instances into a circle. - auto& tcm = engine->getTransformManager(); - auto extent = app.asset->getBoundingBox().extent(); - float max_extent = std::max(std::max(extent.x, extent.y), extent.z); - auto translation = mat4f::translation(float3(max_extent, 0, 0)); - for (size_t inst = 0; inst < app.numInstances; ++inst) { - FilamentInstance* instance = app.instances[inst]; - auto transformRoot = tcm.getInstance(instance->getRoot()); - float theta = inst * 2.0 * M_PI / app.numInstances; - auto rotation = mat4f::rotation(theta, float3(0, 0, 1)); - tcm.setTransform(transformRoot, rotation * translation); - } - + arrangeIntoCircle(); loadResources(filename); }; @@ -262,11 +262,9 @@ int main(int argc, char** argv) { delete app.names; AssetLoader::destroy(&app.loader); - - delete[] app.instances; }; - auto animate = [&app](Engine* engine, View* view, double now) { + auto animate = [&app, arrangeIntoCircle](Engine* engine, View* view, double now) { app.resourceLoader->asyncUpdateLoad(); FilamentInstance* instance = nullptr; if (app.instanceToAnimate > -1) { @@ -274,6 +272,14 @@ int main(int argc, char** argv) { } app.viewer->populateScene(app.asset, true, instance); app.viewer->applyAnimation(now); + + static double previous = 0.0; + if (now - previous > 1.0) { + FilamentInstance* instance = app.loader->createInstance(app.asset); + app.instances.push_back(instance); + arrangeIntoCircle(); + previous = now; + } }; auto gui = [&app](Engine* engine, View* view) { }; diff --git a/web/filament-js/filament.d.ts b/web/filament-js/filament.d.ts index 85513a3813..e4bdd3a897 100644 --- a/web/filament-js/filament.d.ts +++ b/web/filament-js/filament.d.ts @@ -568,6 +568,7 @@ export class gltfio$AssetLoader { public createInstancedAsset(urlOrBuffer: BufferReference, instances: (gltfio$FilamentInstance | null)[]): gltfio$FilamentAsset; public destroyAsset(asset: gltfio$FilamentAsset): void; + public createInstance(asset: gltfio$FilamentAsset): (gltfio$FilamentInstance | null); public delete(): void; } diff --git a/web/filament-js/jsbindings.cpp b/web/filament-js/jsbindings.cpp index 256c8547dc..56e149f54e 100644 --- a/web/filament-js/jsbindings.cpp +++ b/web/filament-js/jsbindings.cpp @@ -1782,6 +1782,10 @@ class_("gltfio$AssetLoader") buffer.bd->size, instances.data(), numInstances); }), allow_raw_pointers()) + // createInstance ::method:: + // Adds a new instance to an instanced asset. + .function("createInstance", &AssetLoader::createInstance, allow_raw_pointers()) + // destroyAsset ::method:: // Destroys the given asset and all of its associated Filament objects. This includes // components, material instances, vertex buffers, index buffers, and textures. From dcca236f6058b61a9a6aa07bfe4ac53b1cdd05e5 Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Mon, 2 Nov 2020 10:47:28 -0700 Subject: [PATCH 16/21] Fix bug with Fence timeouts (#3243) --- filament/backend/src/metal/MetalHandles.mm | 1 + filament/src/Fence.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/filament/backend/src/metal/MetalHandles.mm b/filament/backend/src/metal/MetalHandles.mm index 2d56e55ae4..9de34a5b24 100644 --- a/filament/backend/src/metal/MetalHandles.mm +++ b/filament/backend/src/metal/MetalHandles.mm @@ -684,6 +684,7 @@ void MetalFence::onSignal(MetalFenceSignalBlock block) { FenceStatus MetalFence::wait(uint64_t timeoutNs) { if (@available(macOS 10.14, iOS 12, *)) { std::unique_lock guard(state->mutex); + timeoutNs = std::min(timeoutNs, (uint64_t) std::chrono::nanoseconds::max().count()); while (state->status == FenceStatus::TIMEOUT_EXPIRED) { if (timeoutNs == 0 || state->cv.wait_for(guard, std::chrono::nanoseconds(timeoutNs)) == diff --git a/filament/src/Fence.cpp b/filament/src/Fence.cpp index f13a585a0c..912522ca43 100644 --- a/filament/src/Fence.cpp +++ b/filament/src/Fence.cpp @@ -71,6 +71,7 @@ FenceStatus FFence::waitAndDestroy(FFence* fence, Mode mode) noexcept { UTILS_NOINLINE FenceStatus FFence::wait(Mode mode, uint64_t timeout) noexcept { ASSERT_PRECONDITION(UTILS_HAS_THREADING || timeout == 0, "Non-zero timeout requires threads."); + timeout = std::min(timeout, (uint64_t) ns::max().count()); FEngine& engine = mEngine; From 8b8d563b0f5331f79558e5a61b1fbc964ea6456c Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Mon, 2 Nov 2020 11:00:09 -0700 Subject: [PATCH 17/21] 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 dc1c5529c0bbabb93e98342a0661faec00388ef5 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Mon, 2 Nov 2020 10:59:19 -0700 Subject: [PATCH 18/21] Update RELEASE_NOTES for 1.9.7 --- RELEASE_NOTES.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index eef4d4e265..bea3ffc4a7 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -5,8 +5,16 @@ A new header is inserted each time a *tag* is created. ## Next release (main branch) +## v1.9.8 + ## 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) From a3822f4af0b2f8504e5235e341df8270206c4bb5 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Mon, 2 Nov 2020 15:11:32 -0800 Subject: [PATCH 19/21] Fix FENCE_WAIT_FOR_EVER in Linux. The number of infinite nanoseconds was negative because we asked chrono for a signed integer, so "wait forever" really meant "do not wait at all". --- libs/utils/include/utils/linux/Condition.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/utils/include/utils/linux/Condition.h b/libs/utils/include/utils/linux/Condition.h index d70b477bfa..85a86b64be 100644 --- a/libs/utils/include/utils/linux/Condition.h +++ b/libs/utils/include/utils/linux/Condition.h @@ -68,7 +68,7 @@ public: std::cv_status wait_until(std::unique_lock& lock, const std::chrono::time_point& timeout_time) noexcept { // convert to nanoseconds - int64_t ns = std::chrono::duration(timeout_time.time_since_epoch()).count(); + uint64_t ns = std::chrono::duration(timeout_time.time_since_epoch()).count(); using sec_t = decltype(timespec::tv_sec); using nsec_t = decltype(timespec::tv_nsec); timespec ts{ sec_t(ns / 1000000000), nsec_t(ns % 1000000000) }; @@ -79,7 +79,7 @@ public: std::cv_status wait_until(std::unique_lock& lock, const std::chrono::time_point& timeout_time) noexcept { // convert to nanoseconds - int64_t ns = std::chrono::duration(timeout_time.time_since_epoch()).count(); + uint64_t ns = std::chrono::duration(timeout_time.time_since_epoch()).count(); using sec_t = decltype(timespec::tv_sec); using nsec_t = decltype(timespec::tv_nsec); timespec ts{ sec_t(ns / 1000000000), nsec_t(ns % 1000000000) }; From f6b90d2a3150a53b7929732fec125f8c6ed843c1 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Mon, 9 Nov 2020 09:21:36 -0800 Subject: [PATCH 20/21] Bump version to 1.9.8 --- 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 70491bcd62..52c336ed7d 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.9.7' + implementation 'com.google.android.filament:filament-android:1.9.8' } ``` @@ -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.7' +pod 'Filament', '~> 1.9.8' ``` ### Snapshots diff --git a/android/gradle.properties b/android/gradle.properties index b0fe81f4ab..3203eb7a92 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.9.7 +VERSION_NAME=1.9.8 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 764c667677..044bf84a95 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.7" + spec.version = "1.9.8" 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.7/filament-v1.9.7-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.9.8/filament-v1.9.8-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 36a7e1df4d..2b15ea3b28 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.9.7", + "version": "1.9.8", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From 75af25419d3138964d19fb2093d0a78e802064cd Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Mon, 9 Nov 2020 09:26:27 -0800 Subject: [PATCH 21/21] Update RELEASE_NOTES for 1.9.8 --- RELEASE_NOTES.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index bea3ffc4a7..90dc8e062d 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,6 +7,16 @@ A new header is inserted each time a *tag* is created. ## v1.9.8 +- Fix a few Fence-related bugs +- gltfio: add createInstance() to AssetLoader. +- gltfio: fix ASAN issue when consuming invalid animation. +- gltfio: do not segfault on invalid primitives. +- gltfio: add safety checks to getAnimator. +- gltfio: fix segfault when consuming invalid file. +- Vulkan: various internal refactoring and improvements +- mathio: add ostream operator for quaternions. +- Fix color grading not applied when dithering is off. + ## v1.9.7 - Vulkan: improvements to the ReadPixels implementation.