Skip to content

Commit

Permalink
Fix a crash for IBL resource loading (#8001)
Browse files Browse the repository at this point in the history
This fixes a crash introduced by a8ace28

The refactored FrameInfoManager can cause a crash when IBL resource loading
happens because now the getLastFrameInfo() references an invalid value via the
`front` method. Return the default FrameInfo to resolve this.

Also fix a null pointer reference bug for OpenGLTimer::State, which
happenes when the renderer for IBLPrefilterContext is destroyed.
  • Loading branch information
z3moon committed Jul 29, 2024
1 parent 1fef82a commit 6b43762
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
32 changes: 18 additions & 14 deletions filament/backend/src/opengl/OpenGLTimerQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,21 +143,25 @@ void TimerQueryNativeFactory::endTimeElapsedQuery(OpenGLDriver& driver, GLTimerQ

driver.runEveryNowAndThen([&context = mContext, weak]() -> bool {
auto state = weak.lock();
if (state) {
GLuint available = 0;
context.procs.getQueryObjectuiv(state->gl.query, GL_QUERY_RESULT_AVAILABLE, &available);
CHECK_GL_ERROR(utils::slog.e)
if (!available) {
// we need to try this one again later
return false;
}
GLuint64 elapsedTime = 0;
// we won't end-up here if we're on ES and don't have GL_EXT_disjoint_timer_query
context.procs.getQueryObjectui64v(state->gl.query, GL_QUERY_RESULT, &elapsedTime);
state->elapsed.store((int64_t)elapsedTime, std::memory_order_relaxed);
} else {
state->elapsed.store(int64_t(TimerQueryResult::ERROR), std::memory_order_relaxed);
if (!state) {
// The timer query state has been destroyed on the way, very likely due to the IBL
// prefilter context destruction. We still return true to get this element removed from
// the query list.
return true;
}

GLuint available = 0;
context.procs.getQueryObjectuiv(state->gl.query, GL_QUERY_RESULT_AVAILABLE, &available);
CHECK_GL_ERROR(utils::slog.e)
if (!available) {
// we need to try this one again later
return false;
}
GLuint64 elapsedTime = 0;
// we won't end-up here if we're on ES and don't have GL_EXT_disjoint_timer_query
context.procs.getQueryObjectui64v(state->gl.query, GL_QUERY_RESULT, &elapsedTime);
state->elapsed.store((int64_t)elapsedTime, std::memory_order_relaxed);

return true;
});
}
Expand Down
6 changes: 3 additions & 3 deletions filament/src/FrameInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ class FrameInfoManager {
// call this immediately before "swap buffers"
void endFrame(backend::DriverApi& driver) noexcept;

details::FrameInfo const& getLastFrameInfo() const noexcept {
// if pFront is not set yet, return front() but in this case front().valid will be false
return pFront ? *pFront : mFrameTimeHistory.front();
details::FrameInfo getLastFrameInfo() const noexcept {
// if pFront is not set yet, return FrameInfo(). But the `valid` field will be false in this case.
return pFront ? *pFront : details::FrameInfo{};
}

utils::FixedCapacityVector<Renderer::FrameInfo> getFrameInfoHistory(size_t historySize) const noexcept;
Expand Down

0 comments on commit 6b43762

Please sign in to comment.