diff --git a/intern/cycles/blender/display_driver.cpp b/intern/cycles/blender/display_driver.cpp index 3eab2bb8507..0ddc7def86c 100644 --- a/intern/cycles/blender/display_driver.cpp +++ b/intern/cycles/blender/display_driver.cpp @@ -480,26 +480,12 @@ class DrawTile { return false; } - if (!gl_vertex_buffer) { - glGenBuffers(1, &gl_vertex_buffer); - if (!gl_vertex_buffer) { - LOG(ERROR) << "Error allocating tile VBO."; - gl_resources_destroy(); - return false; - } - } - return true; } void gl_resources_destroy() { texture.gl_resources_destroy(); - - if (gl_vertex_buffer) { - glDeleteBuffers(1, &gl_vertex_buffer); - gl_vertex_buffer = 0; - } } inline bool ready_to_draw() const @@ -512,9 +498,6 @@ class DrawTile { /* Display parameters the texture of this tile has been updated for. */ BlenderDisplayDriver::Params params; - - /* OpenGL resources needed for drawing. */ - uint gl_vertex_buffer = 0; }; class DrawTileAndPBO { @@ -560,6 +543,30 @@ struct BlenderDisplayDriver::Tiles { tiles.clear(); } } finished_tiles; + + /* OpenGL vertex buffer needed for drawing. */ + uint gl_vertex_buffer = 0; + + bool gl_resources_ensure() + { + if (!gl_vertex_buffer) { + glGenBuffers(1, &gl_vertex_buffer); + if (!gl_vertex_buffer) { + LOG(ERROR) << "Error allocating tile VBO."; + return false; + } + } + + return true; + } + + void gl_resources_destroy() + { + if (gl_vertex_buffer) { + glDeleteBuffers(1, &gl_vertex_buffer); + gl_vertex_buffer = 0; + } + } }; BlenderDisplayDriver::BlenderDisplayDriver(BL::RenderEngine &b_engine, BL::Scene &b_scene) @@ -626,6 +633,12 @@ bool BlenderDisplayDriver::update_begin(const Params ¶ms, need_clear_ = false; } + if (!tiles_->gl_resources_ensure()) { + tiles_->gl_resources_destroy(); + gl_context_disable(); + return false; + } + if (!tiles_->current_tile.gl_resources_ensure()) { tiles_->current_tile.gl_resources_destroy(); gl_context_disable(); @@ -825,7 +838,8 @@ static void vertex_buffer_update(const DisplayDriver::Params ¶ms) static void draw_tile(const float2 &zoom, const int texcoord_attribute, const int position_attribute, - const DrawTile &draw_tile) + const DrawTile &draw_tile, + const uint gl_vertex_buffer) { if (!draw_tile.ready_to_draw()) { return; @@ -834,9 +848,9 @@ static void draw_tile(const float2 &zoom, const GLTexture &texture = draw_tile.texture; DCHECK_NE(texture.gl_id, 0); - DCHECK_NE(draw_tile.gl_vertex_buffer, 0); + DCHECK_NE(gl_vertex_buffer, 0); - glBindBuffer(GL_ARRAY_BUFFER, draw_tile.gl_vertex_buffer); + glBindBuffer(GL_ARRAY_BUFFER, gl_vertex_buffer); /* Draw at the parameters for which the texture has been updated for. This allows to always draw * texture during bordered-rendered camera view without flickering. The validness of the display @@ -956,10 +970,14 @@ void BlenderDisplayDriver::draw(const Params ¶ms) glEnableVertexAttribArray(texcoord_attribute); glEnableVertexAttribArray(position_attribute); - draw_tile(zoom_, texcoord_attribute, position_attribute, tiles_->current_tile.tile); + draw_tile(zoom_, + texcoord_attribute, + position_attribute, + tiles_->current_tile.tile, + tiles_->gl_vertex_buffer); for (const DrawTile &tile : tiles_->finished_tiles.tiles) { - draw_tile(zoom_, texcoord_attribute, position_attribute, tile); + draw_tile(zoom_, texcoord_attribute, position_attribute, tile, tiles_->gl_vertex_buffer); } display_shader_->unbind(); @@ -1062,6 +1080,7 @@ void BlenderDisplayDriver::gl_resources_destroy() tiles_->current_tile.gl_resources_destroy(); tiles_->finished_tiles.gl_resources_destroy_and_clear(); + tiles_->gl_resources_destroy(); gl_context_disable(); diff --git a/intern/cycles/session/session.cpp b/intern/cycles/session/session.cpp index d03063a7dda..f6e06f20aba 100644 --- a/intern/cycles/session/session.cpp +++ b/intern/cycles/session/session.cpp @@ -49,12 +49,9 @@ Session::Session(const SessionParams ¶ms_, const SceneParams &scene_params) { TaskScheduler::init(params.threads); - session_thread_ = nullptr; - delayed_reset_.do_reset = false; pause_ = false; - cancel_ = false; new_work_added_ = false; device = Device::create(params.device, stats, profiler); @@ -73,48 +70,79 @@ Session::Session(const SessionParams ¶ms_, const SceneParams &scene_params) } full_buffer_written_cb(filename); }; + + /* Create session thread. */ + session_thread_ = new thread(function_bind(&Session::thread_run, this)); } Session::~Session() { + /* Cancel any ongoing render operation. */ cancel(); - /* Make sure path tracer is destroyed before the device. This is needed because destruction might - * need to access device for device memory free. */ - /* TODO(sergey): Convert device to be unique_ptr, and rely on C++ to destruct objects in the + /* Signal session thread to end. */ + { + thread_scoped_lock session_thread_lock(session_thread_mutex_); + session_thread_state_ = SESSION_THREAD_END; + } + session_thread_cond_.notify_all(); + + /* Destroy session thread. */ + session_thread_->join(); + delete session_thread_; + + /* Destroy path tracer, before the device. This is needed because destruction might need to + * access device for device memory free. + * TODO(sergey): Convert device to be unique_ptr, and rely on C++ to destruct objects in the * pre-defined order. */ path_trace_.reset(); + /* Destroy scene and device. */ delete scene; delete device; + /* Stop task scheduler. */ TaskScheduler::exit(); } void Session::start() { - if (!session_thread_) { - session_thread_ = new thread(function_bind(&Session::run, this)); + { + /* Signal session thread to start rendering. */ + thread_scoped_lock session_thread_lock(session_thread_mutex_); + assert(session_thread_state_ == SESSION_THREAD_WAIT); + session_thread_state_ = SESSION_THREAD_RENDER; } + + session_thread_cond_.notify_all(); } void Session::cancel(bool quick) { - if (quick && path_trace_) { - path_trace_->cancel(); + /* Check if session thread is rendering. */ + bool rendering; + { + thread_scoped_lock session_thread_lock(session_thread_mutex_); + rendering = (session_thread_state_ == SESSION_THREAD_RENDER); } - if (session_thread_) { - /* wait for session thread to end */ + if (rendering) { + /* Cancel path trace operations. */ + if (quick && path_trace_) { + path_trace_->cancel(); + } + + /* Cancel other operations. */ progress.set_cancel("Exiting"); + /* Signal unpause in case the render was paused. */ { thread_scoped_lock pause_lock(pause_mutex_); pause_ = false; - cancel_ = true; } pause_cond_.notify_all(); + /* Wait for render thread to be cancelled or finished. */ wait(); } } @@ -192,11 +220,46 @@ void Session::run_main_render_loop() break; } } - - path_trace_->flush_display(); } -void Session::run() +void Session::thread_run() +{ + while (true) { + { + thread_scoped_lock session_thread_lock(session_thread_mutex_); + + if (session_thread_state_ == SESSION_THREAD_WAIT) { + /* Continue waiting for any signal from the main thread. */ + session_thread_cond_.wait(session_thread_lock); + continue; + } + else if (session_thread_state_ == SESSION_THREAD_END) { + /* End thread immediately. */ + break; + } + } + + /* Execute a render. */ + thread_render(); + + /* Go back from rendering to waiting. */ + { + thread_scoped_lock session_thread_lock(session_thread_mutex_); + if (session_thread_state_ == SESSION_THREAD_RENDER) { + session_thread_state_ = SESSION_THREAD_WAIT; + } + } + session_thread_cond_.notify_all(); + } + + /* Flush any remaining operations and destroy display driver here. This ensure + * graphics API resources are created and destroyed all in the session thread, + * which can avoid problems contexts and multiple threads. */ + path_trace_->flush_display(); + path_trace_->set_display_driver(nullptr); +} + +void Session::thread_render() { if (params.use_profiling && (params.device.type == DEVICE_CPU)) { profiler.start(); @@ -338,9 +401,9 @@ bool Session::run_wait_for_work(const RenderWork &render_work) const bool no_work = !render_work; update_status_time(pause_, no_work); - /* Only leave the loop when rendering is not paused. But even if the current render is un-paused - * but there is nothing to render keep waiting until new work is added. */ - while (!cancel_) { + /* Only leave the loop when rendering is not paused. But even if the current render is + * un-paused but there is nothing to render keep waiting until new work is added. */ + while (!progress.get_cancel()) { scoped_timer pause_timer; if (!pause_ && (render_work || new_work_added_ || delayed_reset_.do_reset)) { @@ -427,7 +490,8 @@ void Session::do_delayed_reset() tile_manager_.update(buffer_params_, scene); /* Update temp directory on reset. - * This potentially allows to finish the existing rendering with a previously configure temporary + * This potentially allows to finish the existing rendering with a previously configure + * temporary * directory in the host software and switch to a new temp directory when new render starts. */ tile_manager_.set_temp_dir(params.temp_dir); @@ -544,12 +608,14 @@ double Session::get_estimated_remaining_time() const void Session::wait() { - if (session_thread_) { - session_thread_->join(); - delete session_thread_; + /* Wait until session thread either is waiting or ending. */ + while (true) { + thread_scoped_lock session_thread_lock(session_thread_mutex_); + if (session_thread_state_ != SESSION_THREAD_RENDER) { + break; + } + session_thread_cond_.wait(session_thread_lock); } - - session_thread_ = nullptr; } bool Session::update_scene(int width, int height) diff --git a/intern/cycles/session/session.h b/intern/cycles/session/session.h index adfd1346600..4017964d4aa 100644 --- a/intern/cycles/session/session.h +++ b/intern/cycles/session/session.h @@ -172,7 +172,8 @@ class Session { BufferParams buffer_params; } delayed_reset_; - void run(); + void thread_run(); + void thread_render(); /* Update for the new iteration of the main loop in run implementation (run_cpu and run_gpu). * @@ -205,10 +206,19 @@ class Session { int2 get_effective_tile_size() const; - thread *session_thread_; + /* Session thread that performs rendering tasks decoupled from the thread + * controlling the sessions. The thread is created and destroyed along with + * the session. */ + thread *session_thread_ = nullptr; + thread_condition_variable session_thread_cond_; + thread_mutex session_thread_mutex_; + enum { + SESSION_THREAD_WAIT, + SESSION_THREAD_RENDER, + SESSION_THREAD_END, + } session_thread_state_ = SESSION_THREAD_WAIT; bool pause_ = false; - bool cancel_ = false; bool new_work_added_ = false; thread_condition_variable pause_cond_; diff --git a/tests/python/CMakeLists.txt b/tests/python/CMakeLists.txt index 2ca4e27bca5..fabac659555 100644 --- a/tests/python/CMakeLists.txt +++ b/tests/python/CMakeLists.txt @@ -683,19 +683,16 @@ if(WITH_CYCLES OR WITH_OPENGL_RENDER_TESTS) set(_cycles_render_tests bake;${render_tests};osl) foreach(render_test ${_cycles_render_tests}) - # Enable just one simple test for Metal until more tests are passing. - if((NOT (_cycles_device MATCHES "METAL")) OR (render_test MATCHES "camera")) - add_python_test( - cycles_${render_test}_${_cycles_device_lower} - ${CMAKE_CURRENT_LIST_DIR}/cycles_render_tests.py - -blender "${TEST_BLENDER_EXE}" - -testdir "${TEST_SRC_DIR}/render/${render_test}" - -idiff "${OPENIMAGEIO_IDIFF}" - -outdir "${TEST_OUT_DIR}/cycles" - -device ${_cycles_device} - -blacklist ${_cycles_blacklist} - ) - endif() + add_python_test( + cycles_${render_test}_${_cycles_device_lower} + ${CMAKE_CURRENT_LIST_DIR}/cycles_render_tests.py + -blender "${TEST_BLENDER_EXE}" + -testdir "${TEST_SRC_DIR}/render/${render_test}" + -idiff "${OPENIMAGEIO_IDIFF}" + -outdir "${TEST_OUT_DIR}/cycles" + -device ${_cycles_device} + -blacklist ${_cycles_blacklist} + ) endforeach() endforeach() endif()