We are not correctly handling gltf models with multiple color
attributes. Filament doesn't allow for more than one color
attribute. So we just set the additional attributes as
VertexAttribute::CUSOM(N).
Fixes#8396
* validate MaterialInstance references when destroyed
With this change we now enforce two things:
- All MaterialInstance of a Material must be destroyed when
destroying said Material. This has always been a documented
requirement of the public API, but wasn't enforced (only
a warning was printed).
This new assertion is unconditional.
- A MaterialInstance, when destroyed is no longer in use by any
Renderable.
So before destroying a MaterialInstance, the user of API needs to
ensure that either all Renderable using that MaterialInstance in one
of their Render Primitives are destroyed, or, that these Renderable
using that MaterialInstance are reset to another one or to null.
There is a new RenderableManager::clearMaterialInstanceAt() that can
be used to clear a MaterialInstance on a Render Primitive.
Additionally, a Render Primitive with a null MaterialInstance is now
silently skipped during rendering, instead of a null-dereference.
Finally, that second assert is protected by a new feature flag:
"features.engine.debug.assert_material_instance_in_use". This flag is
enabled on DEBUG builds and disabled on RELEASE builds by default.
The flag can be changed at any time using `Engine::setFeatureFlag()`.
BUGS=[333907416]
* Update filament/src/components/RenderableManager.cpp
Co-authored-by: Powei Feng <powei@google.com>
---------
Co-authored-by: Powei Feng <powei@google.com>
default parameters were not initialized which could cause them to
be incorrectly evaluated in the shader. this is actually a pretty
crazy bug that has been around since forever and which we were
"lucky" to not run into sooner.
this was exposed with the specular extension that was added recently.
Value of mCgltfBuffersLoaded is sometimes retained across creation of FAssetLoader which skips loading the buffer in AssetLoaderExtended#createPrimitive leading to null pointer crash
if it can't find a name for a node, it will revert to the config's
defaultNodeName, however, if that is nullptr also, a crash will occur.
so we provide a last-resort hardcoded name in that case.
- remove deprecated morphing APIs
- repair gltfio, samples and tests
The new API doesn't allow a MorphTargetBuffer per RenderPrimitive,
instead the MorphTargetBuffer is specified per Renderable.
gltfio separates RenderPrimitives from Renderables, in particular all
RenderPrimitives are created before their Renderable; this was
problematic for this change because all primitives must share
a single MorphTargetBuffer living in the Renderable.
To fix this, we're no longer initializing the morphing paramters
at RenderPrimitive creation, instead we store a reference to the
BufferSlot in the Primtive structure, so that later, when the Renderable
is created we can finally retrieve the BufferSlot and initialize its
morphing paramters, which are not available. The "morphing parameters"
are now expanded to contain the MorphTargetBuffer as before (except now
it's always the same for all the primitives of a Rendrable), as well
as the offset within the buffer and the vertex count.
This change will enable proper flat-shading and MikkTSpace.
Caveats:
- Only for disk-local glTF resources
- iOS, Web, Android do not work as of now
Fixes#6358, #7444
* gltfio: add Asset/Resource extended implementations
- Add gltfio/src/extended to implement an alternate loader for
primitives. This is largely based on the implementation in
AssetLoader/ResourceLoader
- Able to correctly produce flat shading from gltf that only have
vertex positions and indices.
- This is not hooked into current code and should have no
practical effect on gltfio.
- TangentsJobExtended extracts data from cgltf accessor and
runs geometry::TangentSpaceMesh on the attributes and computes
the tangent space.
- The /extended folder is meant for running this process. Note that
this API might remesh the input and will require corresponding
changes that might break previous assumptions.
- The general flow of the code is modeled after src/TangentsJob.h
- This is not hooked into current code and should have no
practical effect on gltfio.
- gltfio: Enable -Wall -Werror for gltfio_core
- gltfio: Fix various errors that were missed warnings
- matdbg: switch from std::atomic_uint64_t to
std::atomic<uint64_t> for older clang
we recently added calls to Material::compile in gltfio to precompile
materials are they are discovered. that wasn't a good call, because
this should be the responsibility of the app, not of gltfio, at least
not without an option.
This is now done in gltf_viewer. We need something similar for
Android.
Bugs #7318, #7336
Drag and dropping a gltf folder was broken:
- the handle didn't find the gltf file on drag&drop
- the ResourceLoader cached the asset path
- don't exit(1) when drag&dropping an invalid file
* prevent public classes from being created on the stack
- we used to to this by deleting operator delete, but this prevented
the internal "F" classes from being virtual; which can be useful
when using EntityManger::Listener.
now we just make the destructor protected in each class.
- EntityManger::Listener now has a virtual destructor so that
objects could be correctly destroyed from Listener*
* improve EntityManger and Component managers
- all component managers now have the same "base" API
- getComponentCount()
- empty()
- getEntity()
- getEntities()
- Scene now has getEntityCount()
- EntityManager now has getEntityCount()
- all component manager implement gc() the same way, by calling destroy()
- SingleInstanceComponentManager::gc() that calls removeComponent() has
been removed because it's dangerous. removeComponent() is often
not enough, some additional cleanup might be needed.
The transient property `mRootNotes` in FAssetLoader is built when a new
root asset is created and referenced whenever a new instance is created.
So it incurs an undefined behavior when a previously created asset tries
creating a new instance after a newly created asset has already created
via the same asset loader.
Move this transient property to each asset so that they can reference it
when a new instance is created.
This partially fixes#7269
There's no functional change in this commit.
Make some parameter names more legible by renaming them and put output
parameters to the right of their function.
The temporary variable has been used to store the current instance of
FFilamentAsset being loaded for easy access from internal methods. This
causes a crash as to a complex scenario as follows.
val asset1 = assetLoader.createAsset(assetBuffer1)
val instance1 = assetLoader.createInstance(asset1)
val asset2 = assetLoader.createAsset(assetBuffer2)
val instance2 = assetLoader.createInstance(asset1)
As the first step of fixing this issue, remove the transient property
`mAsset` from FAssetLoader. This commit alone doesn't resolve the issue,
and more commits are following.
Consolidate the low level version of createInstance, which takes a
pointer to cgltf_data type, into the high level version as the latter
one uses a parameter for FFilamentAsset instead of referencing mAsset.
Update all other relevant methods to take a FFilamentAsset pointer
instead of cgltf_data.
This partially fixes#7269
* Don't force masked blending for transmission/volume materials
glTF lets you choose your own alpha mode when using the transmission
and volume material extensions. We were forcing the masked mode which
was incorrect, except to pass the standard tests.
* Update release notes
Powei Feng
Mathias AgopianandPowei Feng
Ben Doherty
seker
Evan Mezeske
Guilhem Saurel
Balaji MandPowei Feng
Balaji M
Minjae Kim
Nimrod Gileadi
Hanno J. Gödecke
Sungun Park
mackongandMathias Agopian
Jacob Su
Romain Guy