Skip to content

Commit

Permalink
Avoid calling any GL calls during shutdown on Android. Should help #1…
Browse files Browse the repository at this point in the history
…1063

The context is already lost and we're really running shutdown when the process is woken
up again. Additionally, orderly shutdown through the button doesn't happen
on the render thread so remove a couple of asserts that are wrong.
  • Loading branch information
hrydgard committed Oct 6, 2018
1 parent 80538b0 commit 77752c4
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 64 deletions.
2 changes: 1 addition & 1 deletion Common/GraphicsContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class GraphicsContext {

// Called from the render thread from threaded backends.
virtual void ThreadStart() {}
virtual bool ThreadFrame() { return true; }
virtual bool ThreadFrame(bool skipGLCalls) { return true; }
virtual void ThreadEnd() {}
virtual void StopThread() {}

Expand Down
4 changes: 2 additions & 2 deletions Windows/EmuThread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ void MainThreadFunc() {

if (useEmuThread) {
while (emuThreadState != (int)EmuThreadState::DISABLED) {
graphicsContext->ThreadFrame();
graphicsContext->ThreadFrame(false);
if (GetUIState() == UISTATE_EXIT) {
break;
}
Expand All @@ -264,7 +264,7 @@ void MainThreadFunc() {

if (useEmuThread) {
EmuThreadStop();
while (graphicsContext->ThreadFrame()) {
while (graphicsContext->ThreadFrame(false)) {
// Need to keep eating frames to allow the EmuThread to exit correctly.
continue;
}
Expand Down
4 changes: 2 additions & 2 deletions Windows/GPU/WindowsGLContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -443,8 +443,8 @@ void WindowsGLContext::ThreadStart() {
renderManager_->ThreadStart();
}

bool WindowsGLContext::ThreadFrame() {
return renderManager_->ThreadFrame();
bool WindowsGLContext::ThreadFrame(bool skipGLCalls) {
return renderManager_->ThreadFrame(skipGLCalls);
}

void WindowsGLContext::ThreadEnd() {
Expand Down
2 changes: 1 addition & 1 deletion Windows/GPU/WindowsGLContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class WindowsGLContext : public WindowsGraphicsContext {

void ThreadStart() override;
void ThreadEnd() override;
bool ThreadFrame() override;
bool ThreadFrame(bool skipGLCalls) override;
void StopThread() override;

Draw::DrawContext *GetDrawContext() override { return draw_; }
Expand Down
4 changes: 2 additions & 2 deletions android/jni/AndroidEGLContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ class AndroidEGLGraphicsContext : public AndroidGraphicsContext {
renderManager_->ThreadStart();
}

bool ThreadFrame() override {
return renderManager_->ThreadFrame();
bool ThreadFrame(bool skipGLCalls) override {
return renderManager_->ThreadFrame(skipGLCalls);
}

void ThreadEnd() override {
Expand Down
4 changes: 2 additions & 2 deletions android/jni/AndroidJavaGLContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ class AndroidJavaEGLGraphicsContext : public AndroidGraphicsContext {
renderManager_->ThreadStart();
}

bool ThreadFrame() override {
return renderManager_->ThreadFrame();
bool ThreadFrame(bool skipGLCalls) override {
return renderManager_->ThreadFrame(skipGLCalls);
}

void ThreadEnd() override {
Expand Down
15 changes: 10 additions & 5 deletions android/jni/app-android.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -528,13 +528,17 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_shutdown(JNIEnv *, jclass) {
if (renderer_inited && useCPUThread && graphicsContext) {
// Only used in Java EGL path.
EmuThreadStop();
while (graphicsContext->ThreadFrame()) {
// Skipping GL calls, the old context is gone.
while (graphicsContext->ThreadFrame(true)) {
ILOG("graphicsContext->ThreadFrame executed to clear buffers");
continue;
}
ILOG("Joining emuthread");
EmuThreadJoin();

graphicsContext->ThreadEnd();
graphicsContext->ShutdownFromRenderThread();
ILOG("Graphics context now shut down from NativeApp_shutdown");
}

ILOG("NativeApp.shutdown() -- begin");
Expand Down Expand Up @@ -565,7 +569,8 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeRenderer_displayInit(JNIEnv * env,
ILOG("NativeApp.displayInit() restoring");
if (useCPUThread) {
EmuThreadStop();
while (graphicsContext->ThreadFrame()) {
// Skipping GL calls here because the old context is lost.
while (graphicsContext->ThreadFrame(true)) {
continue;
}
EmuThreadJoin();
Expand Down Expand Up @@ -660,7 +665,7 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeRenderer_displayRender(JNIEnv *env,
if (useCPUThread) {
// This is the "GPU thread".
if (graphicsContext)
graphicsContext->ThreadFrame();
graphicsContext->ThreadFrame(false);
} else {
UpdateRunLoopAndroid(env);
}
Expand Down Expand Up @@ -992,7 +997,7 @@ extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runEGLRenderLoop(J
ELOG("Running graphics loop");
while (!exitRenderLoop) {
// This is the "GPU thread".
graphicsContext->ThreadFrame();
graphicsContext->ThreadFrame(false);
graphicsContext->SwapBuffers();
}
} else {
Expand All @@ -1012,7 +1017,7 @@ extern "C" bool JNICALL Java_org_ppsspp_ppsspp_NativeActivity_runEGLRenderLoop(J

if (useCPUThread) {
EmuThreadStop();
while (graphicsContext->ThreadFrame()) {
while (graphicsContext->ThreadFrame(false)) {
continue;
}
EmuThreadJoin();
Expand Down
2 changes: 1 addition & 1 deletion ext/native/gfx/gl_debug_log.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// Utility to be able to liberally sprinkle GL error checks around your code
// and easily disable them all in release builds - just undefine DEBUG_OPENGL.

// #define DEBUG_OPENGL
#define DEBUG_OPENGL

#if defined(DEBUG_OPENGL)

Expand Down
71 changes: 63 additions & 8 deletions ext/native/thin3d/GLQueueRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,45 @@ static std::string GetInfoLog(GLuint name, Getiv getiv, GetLog getLog) {
return infoLog;
}

void GLQueueRunner::RunInitSteps(const std::vector<GLRInitStep> &steps) {
void GLQueueRunner::RunInitSteps(const std::vector<GLRInitStep> &steps, bool skipGLCalls) {
if (skipGLCalls) {
// Some bookkeeping still needs to be done.
for (size_t i = 0; i < steps.size(); i++) {
const GLRInitStep &step = steps[i];
switch (step.stepType) {
case GLRInitStepType::BUFFER_SUBDATA:
{
if (step.buffer_subdata.deleteData)
delete[] step.buffer_subdata.data;
break;
}
case GLRInitStepType::TEXTURE_IMAGE:
{
GLRTexture *tex = step.texture_image.texture;
if (step.texture_image.allocType == GLRAllocType::ALIGNED) {
FreeAlignedMemory(step.texture_image.data);
} else {
delete[] step.texture_image.data;
}
break;
}
case GLRInitStepType::CREATE_PROGRAM:
{
WARN_LOG(G3D, "CREATE_PROGRAM found with skipGLCalls, not good");
break;
}
case GLRInitStepType::CREATE_SHADER:
{
WARN_LOG(G3D, "CREATE_PROGRAM found with skipGLCalls, not good");
break;
}
default:
break;
}
}
return;
}

CHECK_GL_ERROR_IF_DEBUG();
glActiveTexture(GL_TEXTURE0);
GLuint boundTexture = (GLuint)-1;
Expand All @@ -111,16 +149,16 @@ void GLQueueRunner::RunInitSteps(const std::vector<GLRInitStep> &steps) {
case GLRInitStepType::CREATE_BUFFER:
{
GLRBuffer *buffer = step.create_buffer.buffer;
glGenBuffers(1, &buffer->buffer);
glBindBuffer(buffer->target_, buffer->buffer);
glGenBuffers(1, &buffer->buffer_);
glBindBuffer(buffer->target_, buffer->buffer_);
glBufferData(buffer->target_, step.create_buffer.size, nullptr, step.create_buffer.usage);
CHECK_GL_ERROR_IF_DEBUG();
break;
}
case GLRInitStepType::BUFFER_SUBDATA:
{
GLRBuffer *buffer = step.buffer_subdata.buffer;
glBindBuffer(GL_ARRAY_BUFFER, buffer->buffer);
glBindBuffer(GL_ARRAY_BUFFER, buffer->buffer_);
glBufferSubData(GL_ARRAY_BUFFER, step.buffer_subdata.offset, step.buffer_subdata.size, step.buffer_subdata.data);
if (step.buffer_subdata.deleteData)
delete[] step.buffer_subdata.data;
Expand Down Expand Up @@ -445,7 +483,21 @@ void GLQueueRunner::InitCreateFramebuffer(const GLRInitStep &step) {
currentReadHandle_ = fbo->handle;
}

void GLQueueRunner::RunSteps(const std::vector<GLRStep *> &steps) {
void GLQueueRunner::RunSteps(const std::vector<GLRStep *> &steps, bool skipGLCalls) {
if (skipGLCalls) {
// Dry run
for (size_t i = 0; i < steps.size(); i++) {
const GLRStep &step = *steps[i];
switch (step.stepType) {
case GLRStepType::RENDER:
break;
// TODO
}
delete steps[i];
}
return;
}

CHECK_GL_ERROR_IF_DEBUG();
for (size_t i = 0; i < steps.size(); i++) {
const GLRStep &step = *steps[i];
Expand Down Expand Up @@ -836,7 +888,7 @@ void GLQueueRunner::PerformRenderPass(const GLRStep &step) {
{
// TODO: Add fast path for glBindVertexBuffer
GLRInputLayout *layout = c.bindVertexBuffer.inputLayout;
GLuint buf = c.bindVertexBuffer.buffer ? c.bindVertexBuffer.buffer->buffer : 0;
GLuint buf = c.bindVertexBuffer.buffer ? c.bindVertexBuffer.buffer->buffer_ : 0;
assert(!c.bindVertexBuffer.buffer->Mapped());
if (buf != curArrayBuffer) {
glBindBuffer(GL_ARRAY_BUFFER, buf);
Expand Down Expand Up @@ -865,14 +917,14 @@ void GLQueueRunner::PerformRenderPass(const GLRStep &step) {
if (c.bind_buffer.target == GL_ARRAY_BUFFER) {
Crash();
} else if (c.bind_buffer.target == GL_ELEMENT_ARRAY_BUFFER) {
GLuint buf = c.bind_buffer.buffer ? c.bind_buffer.buffer->buffer : 0;
GLuint buf = c.bind_buffer.buffer ? c.bind_buffer.buffer->buffer_ : 0;
assert(!c.bind_buffer.buffer->Mapped());
if (buf != curElemArrayBuffer) {
glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, buf);
curElemArrayBuffer = buf;
}
} else {
GLuint buf = c.bind_buffer.buffer ? c.bind_buffer.buffer->buffer : 0;
GLuint buf = c.bind_buffer.buffer ? c.bind_buffer.buffer->buffer_ : 0;
assert(!c.bind_buffer.buffer->Mapped());
glBindBuffer(c.bind_buffer.target, buf);
}
Expand Down Expand Up @@ -1352,6 +1404,9 @@ void GLQueueRunner::fbo_unbind() {
}

GLRFramebuffer::~GLRFramebuffer() {
if (handle == 0 && z_stencil_buffer == 0 && z_buffer == 0 && stencil_buffer == 0)
return;

CHECK_GL_ERROR_IF_DEBUG();
if (gl_extensions.ARB_framebuffer_object || gl_extensions.IsGLES) {
if (handle) {
Expand Down
4 changes: 2 additions & 2 deletions ext/native/thin3d/GLQueueRunner.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,9 @@ class GLQueueRunner {
public:
GLQueueRunner() {}

void RunInitSteps(const std::vector<GLRInitStep> &steps);
void RunInitSteps(const std::vector<GLRInitStep> &steps, bool skipGLCalls);

void RunSteps(const std::vector<GLRStep *> &steps);
void RunSteps(const std::vector<GLRStep *> &steps, bool skipGLCalls);
void LogSteps(const std::vector<GLRStep *> &steps);

void CreateDeviceObjects();
Expand Down
Loading

0 comments on commit 77752c4

Please sign in to comment.