From 4bac9efffef41be4837435e0f11888a7c84178cd Mon Sep 17 00:00:00 2001 From: rdb Date: Wed, 3 Jan 2018 00:10:24 +0100 Subject: [PATCH] display: fix dispatch_compute and extract_texture_data in MT pipeline This is required to make the test suite run with pipelining enabled. It's necessary to let the draw thread do these tasks because that's where the OpenGL context is bound to. However, we need to let it access the data from the calling thread, so we have to temporarily change the pipeline stage of the draw thread. I'm not really happy about this solution; it would be better to temporarily make the context current to the app thread, but we need a window for that, which we don't currently require to be passed into that method. --- panda/src/display/graphicsEngine.cxx | 74 ++++++++++++++++++++++------ panda/src/display/graphicsEngine.h | 9 ++++ 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/panda/src/display/graphicsEngine.cxx b/panda/src/display/graphicsEngine.cxx index e7ea4acd78..874ab9be13 100644 --- a/panda/src/display/graphicsEngine.cxx +++ b/panda/src/display/graphicsEngine.cxx @@ -1118,17 +1118,34 @@ extract_texture_data(Texture *tex, GraphicsStateGuardian *gsg) { // has finished its current task. WindowRenderer *wr = get_window_renderer(draw_name, 0); RenderThread *thread = (RenderThread *)wr; - MutexHolder holder2(thread->_cv_mutex); + MutexHolder cv_holder(thread->_cv_mutex); while (thread->_thread_state != TS_wait) { thread->_cv_done.wait(); } - // OK, now the draw thread is idle. That's really good enough for our - // purposes; we don't *actually* need to make the draw thread do the work - // --it's sufficient that it's not doing anything else while we access the - // GSG. - return gsg->extract_texture_data(tex); + // Temporarily set this so that it accesses data from the current thread. + int pipeline_stage = Thread::get_current_pipeline_stage(); + int draw_pipeline_stage = thread->get_pipeline_stage(); + thread->set_pipeline_stage(pipeline_stage); + + // Now that the draw thread is idle, signal it to do the extraction task. + thread->_gsg = gsg; + thread->_texture = tex; + thread->_thread_state = TS_do_extract; + thread->_cv_start.notify(); + thread->_cv_mutex.release(); + thread->_cv_mutex.acquire(); + + //XXX is this necessary, or is acquiring the mutex enough? + while (thread->_thread_state != TS_wait) { + thread->_cv_done.wait(); + } + + thread->set_pipeline_stage(draw_pipeline_stage); + thread->_gsg = nullptr; + thread->_texture = nullptr; + return thread->_result; } } @@ -1148,6 +1165,7 @@ extract_texture_data(Texture *tex, GraphicsStateGuardian *gsg) { void GraphicsEngine:: dispatch_compute(const LVecBase3i &work_groups, const ShaderAttrib *sattr, GraphicsStateGuardian *gsg) { nassertv(sattr->get_shader() != (Shader *)NULL); + nassertv(gsg != nullptr); ReMutexHolder holder(_lock); @@ -1156,26 +1174,43 @@ dispatch_compute(const LVecBase3i &work_groups, const ShaderAttrib *sattr, Graph string draw_name = gsg->get_threading_model().get_draw_name(); if (draw_name.empty()) { // A single-threaded environment. No problem. + gsg->set_state_and_transform(state, TransformState::make_identity()); + gsg->dispatch_compute(work_groups[0], work_groups[1], work_groups[2]); } else { // A multi-threaded environment. We have to wait until the draw thread // has finished its current task. WindowRenderer *wr = get_window_renderer(draw_name, 0); RenderThread *thread = (RenderThread *)wr; - MutexHolder holder2(thread->_cv_mutex); + MutexHolder cv_holder(thread->_cv_mutex); while (thread->_thread_state != TS_wait) { thread->_cv_done.wait(); } - // OK, now the draw thread is idle. That's really good enough for our - // purposes; we don't *actually* need to make the draw thread do the work - // --it's sufficient that it's not doing anything else while we access the - // GSG. - } + // Temporarily set this so that it accesses data from the current thread. + int pipeline_stage = Thread::get_current_pipeline_stage(); + int draw_pipeline_stage = thread->get_pipeline_stage(); + thread->set_pipeline_stage(pipeline_stage); - gsg->set_state_and_transform(state, TransformState::make_identity()); - gsg->dispatch_compute(work_groups[0], work_groups[1], work_groups[2]); + // Now that the draw thread is idle, signal it to do the compute task. + thread->_gsg = gsg; + thread->_state = state.p(); + thread->_work_groups = work_groups; + thread->_thread_state = TS_do_compute; + thread->_cv_start.notify(); + thread->_cv_mutex.release(); + thread->_cv_mutex.acquire(); + + //XXX is this necessary, or is acquiring the mutex enough? + while (thread->_thread_state != TS_wait) { + thread->_cv_done.wait(); + } + + thread->set_pipeline_stage(draw_pipeline_stage); + thread->_gsg = nullptr; + thread->_state = nullptr; + } } /** @@ -2584,6 +2619,17 @@ thread_main() { do_pending(_engine, current_thread); break; + case TS_do_compute: + nassertd(_gsg != nullptr && _state != nullptr) break; + _gsg->set_state_and_transform(_state, TransformState::make_identity()); + _gsg->dispatch_compute(_work_groups[0], _work_groups[1], _work_groups[2]); + break; + + case TS_do_extract: + nassertd(_gsg != nullptr && _texture != nullptr) break; + _result = _gsg->extract_texture_data(_texture); + break; + case TS_terminate: do_pending(_engine, current_thread); do_close(_engine, current_thread); diff --git a/panda/src/display/graphicsEngine.h b/panda/src/display/graphicsEngine.h index 853487d05a..c21bec1d73 100644 --- a/panda/src/display/graphicsEngine.h +++ b/panda/src/display/graphicsEngine.h @@ -123,6 +123,8 @@ public: TS_do_flip, TS_do_release, TS_do_windows, + TS_do_compute, + TS_do_extract, TS_terminate, TS_done }; @@ -299,6 +301,13 @@ private: ConditionVar _cv_start; ConditionVar _cv_done; ThreadState _thread_state; + + // These are stored for extract_texture_data and dispatch_compute. + GraphicsStateGuardian *_gsg; + Texture *_texture; + const RenderState *_state; + LVecBase3i _work_groups; + bool _result; }; WindowRenderer *get_window_renderer(const string &name, int pipeline_stage);