diff --git a/android/filament-android/src/main/java/com/google/android/filament/Texture.java b/android/filament-android/src/main/java/com/google/android/filament/Texture.java
index 723d9c782d..85642839fc 100644
--- a/android/filament-android/src/main/java/com/google/android/filament/Texture.java
+++ b/android/filament-android/src/main/java/com/google/android/filament/Texture.java
@@ -322,13 +322,26 @@ public class Texture {
public CompressedFormat compressedFormat;
@Nullable public Object handler;
- @Nullable public Runnable callback;
/**
- * Valid handler types:
- * - Android: Handler, Executor
- * - Other: Executor
+ * Callback used to destroy the buffer data.
+ *
+ * Guarantees:
+ *
+ * - Called on the main filament thread.
+ *
+ *
+ *
+ *
+ * Limitations:
+ *
+ * - Must be lightweight.
+ * - Must not call filament APIs.
+ *
+ *
*/
+ @Nullable public Runnable callback;
+
/**
* Creates a PixelBufferDescriptor
diff --git a/filament/backend/include/backend/BufferDescriptor.h b/filament/backend/include/backend/BufferDescriptor.h
index 75b21a4020..f6c7c69b84 100644
--- a/filament/backend/include/backend/BufferDescriptor.h
+++ b/filament/backend/include/backend/BufferDescriptor.h
@@ -39,7 +39,12 @@ class UTILS_PUBLIC BufferDescriptor {
public:
/**
* Callback used to destroy the buffer data.
- * It is guaranteed to be called on the main filament thread.
+ * Guarantees:
+ * Called on the main filament thread.
+ *
+ * Limitations:
+ * Must be lightweight.
+ * Must not call filament APIs.
*/
using Callback = void(*)(void* buffer, size_t size, void* user);
diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp
index f62901ca7d..ffda0bbc7a 100644
--- a/filament/backend/src/opengl/OpenGLDriver.cpp
+++ b/filament/backend/src/opengl/OpenGLDriver.cpp
@@ -163,6 +163,15 @@ OpenGLDriver::~OpenGLDriver() noexcept {
// ------------------------------------------------------------------------------------------------
void OpenGLDriver::terminate() {
+ // wait for the GPU to finish executing all commands
+ glFinish();
+
+ // and make sure to execute all the GpuCommandCompleteOps callbacks
+ executeGpuCommandsCompleteOps();
+
+ // because we called glFinish(), all callbacks should have been executed
+ assert(!mGpuCommandCompleteOps.size());
+
for (auto& item : mSamplerMap) {
mContext.unbindSampler(item.second);
glDeleteSamplers(1, &item.second);
@@ -2428,7 +2437,7 @@ void OpenGLDriver::readPixels(Handle src,
glReadPixels(GLint(x), GLint(y), GLint(width), GLint(height), glFormat, glType, nullptr);
gl.bindBuffer(GL_PIXEL_PACK_BUFFER, 0);
- // we're forced to make a copy on the stack because otherwise it deletes std::function<> copy
+ // we're forced to make a copy on the heap because otherwise it deletes std::function<> copy
// constructor.
auto* pUserBuffer = new PixelBufferDescriptor(std::move(p));
whenGpuCommandsComplete([this, width, height, pbo, pUserBuffer]() mutable {
@@ -2443,7 +2452,7 @@ void OpenGLDriver::readPixels(Handle src,
p.format, p.type, 1, 1, 1);
size_t bpr = PixelBufferDescriptor::computeDataSize(
p.format, p.type, stride, 1, p.alignment);
- char* head = (char*)vaddr + p.left * bpp + bpr * p.top;
+ char const* head = (char const*)vaddr + p.left * bpp + bpr * p.top;
char* tail = (char*)p.buffer + p.left * bpp + bpr * (p.top + height - 1);
for (size_t i = 0; i < height; ++i) {
memcpy(tail, head, bpp * width);
@@ -2534,6 +2543,7 @@ void OpenGLDriver::flush(int) {
void OpenGLDriver::finish(int) {
DEBUG_MARKER()
glFinish();
+ executeGpuCommandsCompleteOps();
}
UTILS_NOINLINE
diff --git a/filament/backend/src/opengl/OpenGLDriver.h b/filament/backend/src/opengl/OpenGLDriver.h
index f6fcf86996..7b96ef6187 100644
--- a/filament/backend/src/opengl/OpenGLDriver.h
+++ b/filament/backend/src/opengl/OpenGLDriver.h
@@ -379,7 +379,7 @@ private:
void whenGpuCommandsComplete(std::function fn) noexcept;
void executeGpuCommandsCompleteOps() noexcept;
- std::vector< std::pair> > mGpuCommandCompleteOps;
+ std::vector>> mGpuCommandCompleteOps;
};
// ------------------------------------------------------------------------------------------------
diff --git a/filament/src/Engine.cpp b/filament/src/Engine.cpp
index 0fd55e1328..a71d0b8f33 100644
--- a/filament/src/Engine.cpp
+++ b/filament/src/Engine.cpp
@@ -315,21 +315,33 @@ void FEngine::shutdown() {
}
cleanupResourceList(mFences);
- // There might be commands added by the terminate() calls
- flushCommandBuffer(mCommandBufferQueue);
- if (!UTILS_HAS_THREADING) {
- execute();
- }
-
/*
- * terminate the rendering engine
+ * Shutdown the backend...
*/
+ // There might be commands added by the terminate() calls, so we need to flush all commands
+ // up to this point. After flushCommandBuffer() is called, all pending commands are guaranteed
+ // to be executed before the driver thread exits.
+ flushCommandBuffer(mCommandBufferQueue);
+
+ // now wait for all pending commands to be executed and the thread to exit
mCommandBufferQueue.requestExit();
- if (UTILS_HAS_THREADING) {
+ if (!UTILS_HAS_THREADING) {
+ execute();
+ getDriverApi().terminate();
+ } else {
mDriverThread.join();
+
}
+ // Finally, call user callbacks that might have been scheduled.
+ // These callbacks CANNOT call driver APIs.
+ getDriver().purge();
+
+ /*
+ * Terminate the JobSystem...
+ */
+
// detach this thread from the jobsystem
mJobSystem.emancipate();
@@ -446,6 +458,9 @@ int FEngine::loop() {
}
#endif
+ JobSystem::setThreadName("FEngine::loop");
+ JobSystem::setThreadPriority(JobSystem::Priority::DISPLAY);
+
mDriver = platform->createDriver(mSharedGLContext);
mDriverBarrier.latch();
if (UTILS_UNLIKELY(!mDriver)) {
@@ -454,9 +469,6 @@ int FEngine::loop() {
return 0;
}
- JobSystem::setThreadName("FEngine::loop");
- JobSystem::setThreadPriority(JobSystem::Priority::DISPLAY);
-
// We use the highest affinity bit, assuming this is a Big core in a big.little
// configuration. This is also a core not used by the JobSystem.
// Either way the main reason to do this is to avoid this thread jumping from core to core