Scarfski/filament
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

IBLPrefilter relied on finalizer which could cause crashes

Browse Source 
The finalizer can't be used to dispose of filament resource, because
it runs on its own thread.

Added a destroy() method to all IBLPrefilter related classes that must
be called once the object is no longer needed.

note: Resources will be freed eventually anyways when the Engine itself is
destroyed.


fixes #4633
This commit is contained in:
Mathias Agopian
2021-09-17 11:04:53 -07:00
committed by Mathias Agopian
parent 552a3ab103
commit 82623c8fd4
4 changed files with 52 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

83
android/filament-utils-android/src/main/java/com/google/android/filament/utils/IBLPrefilterContext.java
View File

@@ -32,9 +32,13 @@ import com.google.android.filament.Texture;
* Texture equirect = HDRLoader.createTexture("foo.hdr");
* Texture skyboxTexture = equirectangularToCubemap.run(equirect);
* engine.destroy(equirect);
* equirectangularToCubemap.destroy();
*
* specularFilter = new IBLPrefilterContext.SpecularFilter(context);
* Texture reflections = specularFilter.run(skyboxTexture);
* specularFilter.destroy();
*
* context.destroy();
*
* IndirectLight ibl = IndirectLight.Builder()
* .reflections(reflections)
@@ -43,81 +47,74 @@ import com.google.android.filament.Texture;
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*/
public class IBLPrefilterContext {
private final long mNativeObject;
private long mNativeObject;
public IBLPrefilterContext(Engine engine) {
mNativeObject = nCreate(engine.getNativeObject());
if (mNativeObject == 0) throw new IllegalStateException("Couldn't create IBLPrefilterContext");
}
@Override
protected void finalize() throws Throwable {
nDestroy(mNativeObject);
super.finalize();
public void destroy() {
nDestroy(getNativeObject());
mNativeObject = 0;
}
public static class EquirectangularToCubemap {
@SuppressWarnings({"FieldCanBeLocal", "UnusedDeclaration"}) // Keep to finalize native resources
private final HelperFinalizer mFinalizer;
private final long mNativeHelper;
private long mNativeObject;
public EquirectangularToCubemap(IBLPrefilterContext context) {
mNativeHelper = nCreateEquirectHelper(context.mNativeObject);
mFinalizer = new HelperFinalizer(mNativeHelper);
mNativeObject = nCreateEquirectHelper(context.getNativeObject());
}
public Texture run(Texture equirect) {
long nativeTexture = nEquirectHelperRun(mNativeHelper, equirect.getNativeObject());
long nativeTexture = nEquirectHelperRun(getNativeObject(), equirect.getNativeObject());
return new Texture(nativeTexture);
}
private static class HelperFinalizer {
private final long mNativeObject;
public void destroy() {
nDestroyEquirectHelper(getNativeObject());
mNativeObject = 0;
}
HelperFinalizer(long nativeObject) { mNativeObject = nativeObject; }
@Override
public void finalize() {
try {
super.finalize();
} catch (Throwable t) { // Ignore
} finally {
nDestroyEquirectHelper(mNativeObject);
}
protected long getNativeObject() {
if (mNativeObject == 0) {
throw new IllegalStateException("Calling method on destroyed EquirectangularToCubemap");
}
return mNativeObject;
}
}
public static class SpecularFilter {
@SuppressWarnings({"FieldCanBeLocal", "UnusedDeclaration"}) // Keep to finalize native resources
private final HelperFinalizer mFinalizer;
private final long mNativeHelper;
private long mNativeObject;
public SpecularFilter(IBLPrefilterContext context) {
mNativeHelper = nCreateSpecularFilter(context.mNativeObject);
mFinalizer = new HelperFinalizer(mNativeHelper);
mNativeObject = nCreateSpecularFilter(context.getNativeObject());
}
public Texture run(Texture skybox) {
long nativeTexture = nSpecularFilterRun(mNativeHelper, skybox.getNativeObject());
long nativeTexture = nSpecularFilterRun(getNativeObject(), skybox.getNativeObject());
return new Texture(nativeTexture);
}
private static class HelperFinalizer {
private final long mNativeObject;
HelperFinalizer(long nativeObject) { mNativeObject = nativeObject; }
@Override
public void finalize() {
try {
super.finalize();
} catch (Throwable t) { // Ignore
} finally {
nDestroySpecularFilter(mNativeObject);
}
}
public void destroy() {
nDestroySpecularFilter(getNativeObject());
mNativeObject = 0;
}
protected long getNativeObject() {
if (mNativeObject == 0) {
throw new IllegalStateException("Calling method on destroyed SpecularFilter");
}
return mNativeObject;
}
}
protected long getNativeObject() {
if (mNativeObject == 0) {
throw new IllegalStateException("Calling method on destroyed IBLPrefilterContext");
}
return mNativeObject;
}
private static native long nCreate(long nativeEngine);

4
android/samples/sample-gltf-viewer/src/main/java/com/google/android/filament/gltf/MainActivity.kt
View File

@@ -196,6 +196,10 @@ class MainActivity : Activity() {
val sky = Skybox.Builder().environment(skyboxTexture).build(engine)
specularFilter.destroy();
equirectToCubemap.destroy();
context.destroy();
modelViewer.scene.skybox = sky
modelViewer.scene.indirectLight = ibl
}

8
libs/iblprefilter/include/filament-iblprefilter/IBLPrefilterContext.h
View File

@@ -62,7 +62,7 @@ public:
* Creates an IBLPrefilter context.
* @param engine filament engine to use
*/
IBLPrefilterContext(filament::Engine& engine);
explicit IBLPrefilterContext(filament::Engine& engine);
/**
* Destroys all GPU resources created during initialization.
@@ -75,7 +75,7 @@ public:
// movable
IBLPrefilterContext(IBLPrefilterContext&& rhs) noexcept;
IBLPrefilterContext& operator=(IBLPrefilterContext&& rhs);
IBLPrefilterContext& operator=(IBLPrefilterContext&& rhs) noexcept;
// -------------------------------------------------------------------------------------------
@@ -98,7 +98,7 @@ public:
EquirectangularToCubemap(EquirectangularToCubemap const&) = delete;
EquirectangularToCubemap& operator=(EquirectangularToCubemap const&) = delete;
EquirectangularToCubemap(EquirectangularToCubemap&& rhs) noexcept;
EquirectangularToCubemap& operator=(EquirectangularToCubemap&& rhs);
EquirectangularToCubemap& operator=(EquirectangularToCubemap&& rhs) noexcept;
/**
* Converts an equirectangular image to a cubemap.
@@ -175,7 +175,7 @@ public:
SpecularFilter(SpecularFilter const&) = delete;
SpecularFilter& operator=(SpecularFilter const&) = delete;
SpecularFilter(SpecularFilter&& rhs) noexcept;
SpecularFilter& operator=(SpecularFilter&& rhs);
SpecularFilter& operator=(SpecularFilter&& rhs) noexcept;
/**
* Generates a prefiltered cubemap.

7
libs/iblprefilter/src/IBLPrefilterContext.cpp
View File

@@ -142,7 +142,7 @@ IBLPrefilterContext::IBLPrefilterContext(IBLPrefilterContext&& rhs) noexcept
this->operator=(std::move(rhs));
}
IBLPrefilterContext& IBLPrefilterContext::operator=(IBLPrefilterContext&& rhs) {
IBLPrefilterContext& IBLPrefilterContext::operator=(IBLPrefilterContext&& rhs) noexcept {
using std::swap;
if (this != & rhs) {
swap(mRenderer, rhs.mRenderer);
@@ -182,7 +182,7 @@ IBLPrefilterContext::EquirectangularToCubemap::EquirectangularToCubemap(
IBLPrefilterContext::EquirectangularToCubemap&
IBLPrefilterContext::EquirectangularToCubemap::operator=(
IBLPrefilterContext::EquirectangularToCubemap&& rhs) {
IBLPrefilterContext::EquirectangularToCubemap&& rhs) noexcept {
using std::swap;
if (this != &rhs) {
swap(mEquirectMaterial, rhs.mEquirectMaterial);
@@ -332,7 +332,8 @@ IBLPrefilterContext::SpecularFilter::SpecularFilter(SpecularFilter&& rhs) noexce
this->operator=(std::move(rhs));
}
IBLPrefilterContext::SpecularFilter& IBLPrefilterContext::SpecularFilter::operator=(SpecularFilter&& rhs) {
IBLPrefilterContext::SpecularFilter&
IBLPrefilterContext::SpecularFilter::operator=(SpecularFilter&& rhs) noexcept {
using std::swap;
if (this != & rhs) {
swap(mKernelMaterial, rhs.mKernelMaterial);