BLEN-365: Improve creation algorithm of PreviewEngine #21

Merged
Bogdan Nagirniak merged 11 commits from BLEN-365 into hydra-render 2023-04-10 12:44:48 +02:00
Collaborator

Purpose
PreviewEngine shouldn’t be recreated every time when material is changed.

Technical steps
Python:

  1. Creation and sync of engine moved to def update

C++:

  1. Added delayed deletion for PreviewEngine (60 sec for now). Timer is refreshed with every change;
  2. Moved creation of BlenderSceneDelegate from every Engine's inheritor to Engine::Engine;
  3. PreviewEngine::sync clears render_index and scene_delegate's data;
**Purpose** `PreviewEngine` shouldn’t be recreated every time when material is changed. **Technical steps** Python: 1. Creation and sync of engine moved to `def update` C++: 1. Added delayed deletion for `PreviewEngine` (60 sec for now). Timer is refreshed with every change; 2. Moved creation of `BlenderSceneDelegate` from every `Engine`'s inheritor to `Engine::Engine`; 3. `PreviewEngine::sync` clears `render_index` and `scene_delegate`'s data;
Georgiy Markelov added 3 commits 2023-04-04 13:12:40 +02:00
Brian Savery (AMD) was assigned by Georgiy Markelov 2023-04-04 13:17:29 +02:00
Georgiy Markelov self-assigned this 2023-04-04 13:17:29 +02:00
Bogdan Nagirniak was assigned by Georgiy Markelov 2023-04-04 13:17:30 +02:00
Georgiy Markelov requested review from Brian Savery (AMD) 2023-04-04 13:18:20 +02:00
Brian Savery (AMD) approved these changes 2023-04-05 00:45:05 +02:00
Bogdan Nagirniak requested changes 2023-04-05 16:11:02 +02:00
@ -45,0 +46,4 @@
scene_delegate = std::make_unique<BlenderSceneDelegate>(
render_index.get(),
pxr::SdfPath::AbsoluteRootPath().AppendElementString("scene"),
BlenderSceneDelegate::EngineType::PREVIEW);

There could be other types of engines

There could be other types of engines
DagerD marked this conversation as resolved
@ -18,1 +15,3 @@
BlenderSceneDelegate::EngineType::PREVIEW);
scene_delegate->clear_data();
for (auto &prim : render_index->GetRprimIds()) {

Move this to clear()

Move this to clear()
DagerD marked this conversation as resolved
@ -19,2 +21,4 @@
namespace blender::render::hydra {
static double preview_engine_lifetime = 60.0;
static PreviewEngine *preview_engine;

Use unique_ptr

Use unique_ptr
DagerD marked this conversation as resolved
@ -21,0 +26,4 @@
double delete_preview_engine(uintptr_t uuid, void *user_data)
{
if (preview_engine) {
preview_engine->stop_renderer();

add logging

add logging
DagerD marked this conversation as resolved
@ -21,0 +33,4 @@
}
return preview_engine_lifetime;
}

Move this function to preview_engein.cc
Probably it can be static function of PreviewEngine class

Move this function to preview_engein.cc Probably it can be static function of PreviewEngine class
DagerD marked this conversation as resolved
@ -117,0 +135,4 @@
if (BLI_timer_is_registered(1)) {
BLI_timer_unregister(1);
}
preview_engine->bl_engine = bl_engine;

Add PreviewEngine::update_engine() instead public usage of bl_engine

Add PreviewEngine::update_engine() instead public usage of bl_engine
DagerD marked this conversation as resolved
@ -477,4 +477,10 @@ pxr::VtValue BlenderSceneDelegate::GetLightParamValue(pxr::SdfPath const &id,
return pxr::VtValue();
}
void BlenderSceneDelegate::clear_data()

Rename to just "clear"

Rename to just "clear"
DagerD marked this conversation as resolved
Bogdan Nagirniak requested changes 2023-04-05 16:23:25 +02:00
@ -21,0 +26,4 @@
double delete_preview_engine(uintptr_t uuid, void *user_data)
{
if (preview_engine) {
preview_engine->stop_renderer();

Do not stop render, check if it is in render process, then return preview_engine_lifetime.

Do not stop render, check if it is in render process, then return preview_engine_lifetime.
DagerD marked this conversation as resolved
Georgiy Markelov added 4 commits 2023-04-07 12:27:05 +02:00
Bogdan Nagirniak requested changes 2023-04-07 14:00:27 +02:00
@ -54,6 +54,12 @@ Engine::~Engine()
render_delegate = nullptr;
engine = nullptr;
hgi = nullptr;
bl_engine = nullptr;

no need

no need
DagerD marked this conversation as resolved
@ -57,0 +57,4 @@
bl_engine = nullptr;
}
bool Engine::is_converged()

This function is not needed

This function is not needed
DagerD marked this conversation as resolved
@ -19,2 +20,4 @@
namespace blender::render::hydra {
static std::unique_ptr<PreviewEngine> preview_engine;

move to PreviewEngine class as static

move to PreviewEngine class as static
DagerD marked this conversation as resolved
@ -116,0 +124,4 @@
}
preview_engine->update_bl_engine(bl_engine);
CLOG_INFO(LOG_EN, 2, "Engine %016llx %s", engine, engine_type);

This block should be like:
engine = PreviewEngine::get(bl_engine, render_delegate_id);

This block should be like: `engine = PreviewEngine::get(bl_engine, render_delegate_id);`
DagerD marked this conversation as resolved
@ -436,1 +436,4 @@
void BlenderSceneDelegate::clear()
{
for (auto it = materials.begin(); it != materials.end(); ++it) {
for (auto &it : materials) {
  it.second->remove();
}
``` for (auto &it : materials) { it.second->remove(); } ```
DagerD marked this conversation as resolved
@ -25,2 +25,4 @@
private:
std::map<pxr::TfToken, pxr::VtValue> data;
short p_type;
short p_shape;

pxr::TfToken p_type;
make prim_type() with Light * parameter

`pxr::TfToken p_type;` make prim_type() with `Light *` parameter
DagerD marked this conversation as resolved
Georgiy Markelov added 1 commit 2023-04-07 17:55:47 +02:00
Georgiy Markelov requested review from Bogdan Nagirniak 2023-04-07 17:56:10 +02:00
Bogdan Nagirniak added 2 commits 2023-04-10 12:05:22 +02:00
Bogdan Nagirniak approved these changes 2023-04-10 12:15:37 +02:00
Bogdan Nagirniak left a comment
Owner

Added some code improvements. Tested - works good.

Added some code improvements. Tested - works good.
Bogdan Nagirniak added 1 commit 2023-04-10 12:42:47 +02:00
Bogdan Nagirniak merged commit 3f97872f21 into hydra-render 2023-04-10 12:44:48 +02:00
Sign in to join this conversation.
No Label
No Milestone
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: BogdanNagirniak/blender#21
No description provided.