Fix review comments #51

Merged
Bogdan Nagirniak merged 15 commits from BLEN-430 into hydra-render 2023-06-08 18:10:55 +02:00
13 changed files with 63 additions and 136 deletions

View File

@ -38,7 +38,6 @@ __all__ = (
"HydraRenderEngine",
"export_mtlx",
"register_plugins",
"get_render_plugins",
)
import os
@ -48,7 +47,7 @@ from pathlib import Path
import bpy
import _bpy_hydra
from _bpy_hydra import register_plugins, get_render_plugins
from _bpy_hydra import register_plugins
class HydraRenderEngine(bpy.types.RenderEngine):
@ -68,7 +67,6 @@ class HydraRenderEngine(bpy.types.RenderEngine):
@classmethod
def register(cls):
_bpy_hydra.init()
root_folder = "blender.shared" if platform.system() == 'Windows' else "lib"
os.environ['PXR_MTLX_STDLIB_SEARCH_PATHS'] = os.pathsep.join([
str(Path(bpy.app.binary_path).parent / f"{root_folder}/materialx/libraries"),

View File

@ -21,7 +21,6 @@ void FinalEngine::render(Depsgraph *depsgraph)
{
/* Release the GIL before calling into hydra, in case any hydra plugins call into python. */
pxr::TF_PY_ALLOW_THREADS_IN_SCOPE();
engine_->Execute(render_index_.get(), &tasks_);
}
@ -143,7 +142,6 @@ void FinalEngineGL::render(Depsgraph *depsgraph)
{
/* Release the GIL before calling into hydra, in case any hydra plugins call into python. */
pxr::TF_PY_ALLOW_THREADS_IN_SCOPE();
engine_->Execute(render_index_.get(), &tasks_);
}

View File

@ -11,7 +11,6 @@ void PreviewEngine::render(Depsgraph *depsgraph)
{
/* Release the GIL before calling into hydra, in case any hydra plugins call into python. */
pxr::TF_PY_ALLOW_THREADS_IN_SCOPE();
engine_->Execute(render_index_.get(), &tasks_);
}

View File

@ -20,16 +20,6 @@
namespace blender::render::hydra {
static PyObject *init_func(PyObject * /*self*/, PyObject *args)
{
CLOG_INFO(LOG_RENDER_HYDRA, 0, "Init");
pxr::PlugRegistry::GetInstance().RegisterPlugins(std::string(BKE_appdir_program_dir()) +
"/blender.shared/usd");
Py_RETURN_NONE;
}
static PyObject *register_plugins_func(PyObject * /*self*/, PyObject *args)
{
PyObject *pyplugin_dirs;
@ -64,41 +54,6 @@ static PyObject *register_plugins_func(PyObject * /*self*/, PyObject *args)
Py_RETURN_NONE;
}
static PyObject *get_render_plugins_func(PyObject * /*self*/, PyObject *args)
{
pxr::PlugRegistry &registry = pxr::PlugRegistry::GetInstance();
pxr::TfTokenVector plugin_ids = pxr::UsdImagingGLEngine::GetRendererPlugins();
PyObject *ret = PyTuple_New(plugin_ids.size());
PyObject *val;
for (int i = 0; i < plugin_ids.size(); ++i) {
PyObject *descr = PyDict_New();
PyDict_SetItemString(descr, "id", val = PyUnicode_FromString(plugin_ids[i].GetText()));
Py_DECREF(val);
PyDict_SetItemString(
descr,
"name",
val = PyUnicode_FromString(
pxr::UsdImagingGLEngine::GetRendererDisplayName(plugin_ids[i]).c_str()));
Py_DECREF(val);
std::string plugin_name = plugin_ids[i];
plugin_name = plugin_name.substr(0, plugin_name.size() - 6);
plugin_name[0] = tolower(plugin_name[0]);
std::string path = "";
pxr::PlugPluginPtr plugin = registry.GetPluginWithName(plugin_name);
if (plugin) {
path = plugin->GetPath();
}
PyDict_SetItemString(descr, "path", val = PyUnicode_FromString(path.c_str()));
Py_DECREF(val);
PyTuple_SetItem(ret, i, descr);
}
return ret;
}
static PyObject *engine_create_func(PyObject * /*self*/, PyObject *args)
{
PyObject *pyengine;
@ -267,9 +222,7 @@ static PyObject *engine_set_render_setting_func(PyObject * /*self*/, PyObject *a
}
static PyMethodDef methods[] = {
{"init", init_func, METH_VARARGS, ""},
{"register_plugins", register_plugins_func, METH_VARARGS, ""},
{"get_render_plugins", get_render_plugins_func, METH_VARARGS, ""},
{"engine_create", engine_create_func, METH_VARARGS, ""},
{"engine_free", engine_free_func, METH_VARARGS, ""},

View File

@ -241,9 +241,9 @@ void BlenderSceneDelegate::set_setting(const std::string &key, const pxr::VtValu
pxr::SdfPath BlenderSceneDelegate::prim_id(ID *id, const char *prefix) const
{
/* Making id of object in form like <prefix>_<pointer in 16 hex digits format> */
char str[32];
snprintf(str, 32, "%s_%016llx", prefix, (uintptr_t)id);
return GetDelegateID().AppendElementString(str);
char name[32];
snprintf(name, sizeof(name), "%s_%016llx", prefix, (uintptr_t)id);
return GetDelegateID().AppendElementString(name);
}
pxr::SdfPath BlenderSceneDelegate::object_prim_id(Object *object) const

View File

@ -69,17 +69,16 @@ void CurvesData::update()
pxr::VtValue CurvesData::get_data(pxr::SdfPath const &id, pxr::TfToken const &key) const
{
pxr::VtValue ret;
if (key == pxr::HdTokens->points) {
ret = vertices_;
return pxr::VtValue(vertices_);
}
else if (key == pxr::HdPrimvarRoleTokens->textureCoordinate) {
ret = uvs_;
return pxr::VtValue(uvs_);
}
else if (key == pxr::HdTokens->widths) {
ret = widths_;
return pxr::VtValue(widths_);
}
return ret;
return pxr::VtValue();
}
bool CurvesData::update_visibility()
@ -110,19 +109,17 @@ pxr::HdPrimvarDescriptorVector CurvesData::primvar_descriptors(
if (!vertices_.empty()) {
primvars.emplace_back(pxr::HdTokens->points, interpolation, pxr::HdPrimvarRoleTokens->point);
}
if (!widths_.empty()) {
primvars.emplace_back(pxr::HdTokens->widths, interpolation, pxr::HdPrimvarRoleTokens->none);
}
else if (interpolation == pxr::HdInterpolationFaceVarying) {
}
else if (interpolation == pxr::HdInterpolationConstant) {
if (!uvs_.empty()) {
primvars.emplace_back(pxr::HdPrimvarRoleTokens->textureCoordinate,
interpolation,
pxr::HdPrimvarRoleTokens->textureCoordinate);
}
}
else if (interpolation == pxr::HdInterpolationConstant) {
if (!widths_.empty()) {
primvars.emplace_back(pxr::HdTokens->widths, interpolation, pxr::HdPrimvarRoleTokens->none);
}
}
return primvars;
}
@ -160,14 +157,12 @@ void CurvesData::write_curves(Curves *curves)
curve_vertex_counts_.push_back(num_points);
/* Set radius similar to Cycles if isn't set */
widths_.push_back(radii ? radii[i] : 0.01f);
for (int j = 0; j < num_points; j++) {
int ind = first_point_index + j;
widths_.push_back(radii ? radii[ind] * 2 : 0.01f);
vertices_.push_back(pxr::GfVec3f(positions[ind][0], positions[ind][1], positions[ind][2]));
}
}
write_uv_maps(curves);
}
@ -188,8 +183,9 @@ void CurvesData::write_material()
{
Object *object = (Object *)id;
Material *mat = nullptr;
/* TODO: Using only first material. Add support for multimaterial. */
if (BKE_object_material_count_eval(object) > 0) {
mat = BKE_object_material_get_eval(object, object->actcol);
mat = BKE_object_material_get_eval(object, 0);
DagerD marked this conversation as resolved Outdated

Add comment here about using first material and TODO about support multimaterial

Add comment here about using first material and TODO about support multimaterial
}
if (!mat) {

View File

@ -21,16 +21,13 @@ static std::string cache_image_file(Image *image,
ImageUser *iuser,
bool check_exist)
{
std::string file_path(FILE_MAX, 0);
char file_path[FILE_MAX];
char file_name[32];
snprintf(file_name, 32, "img_%016llx.hdr", (uintptr_t)image);
BLI_path_join(file_path.data(),
file_path.capacity(),
BKE_tempdir_session(),
"hydra_image_cache",
file_name);
snprintf(file_name, sizeof(file_name), "img_%016llx.hdr", (uintptr_t)image);
DagerD marked this conversation as resolved
Review

add same sizeof(...) to other places with snprintf()

add same `sizeof(...)` to other places with `snprintf()`
BLI_path_join(
file_path, sizeof(file_path), BKE_tempdir_session(), "hydra_image_cache", file_name);
if (check_exist && BLI_exists(file_path.c_str())) {
if (check_exist && BLI_exists(file_path)) {
return file_path;
}
@ -38,18 +35,19 @@ static std::string cache_image_file(Image *image,
ImageSaveOptions opts;
opts.im_format.imtype = R_IMF_IMTYPE_RADHDR;
if (BKE_image_save_options_init(&opts, main, scene_delegate->scene, image, iuser, true, false)) {
STRNCPY(opts.filepath, file_path.c_str());
ReportList reports;
if (BKE_image_save(&reports, main, image, iuser, &opts)) {
CLOG_INFO(LOG_RENDER_HYDRA_SCENE, 1, "%s -> %s", image->id.name, file_path.c_str());
if (BKE_image_save_options_init(&opts, main, scene_delegate->scene, image, iuser, false, false))
{
STRNCPY(opts.filepath, file_path);
if (BKE_image_save(nullptr, main, image, iuser, &opts)) {
CLOG_INFO(LOG_RENDER_HYDRA_SCENE, 1, "%s -> %s", image->id.name, file_path);
}
else {
file_path = "";
memset(file_path, 0, sizeof(file_path));
}
}
BKE_image_save_options_free(&opts);
CLOG_INFO(LOG_RENDER_HYDRA_SCENE, 2, "%s -> %s", image->id.name, file_path);
return file_path;
}

View File

@ -102,11 +102,10 @@ void InstancerData::update()
pxr::VtValue InstancerData::get_data(pxr::TfToken const &key) const
{
ID_LOG(3, "%s", key.GetText());
pxr::VtValue ret;
if (key == pxr::HdInstancerTokens->instanceTransform) {
ret = mesh_transforms_;
return pxr::VtValue(mesh_transforms_);
}
return ret;
return pxr::VtValue();
}
bool InstancerData::update_visibility()
@ -128,7 +127,7 @@ bool InstancerData::update_visibility()
char name[16];
for (auto &l_inst : light_instances_.values()) {
for (int i = 0; i < l_inst.count; ++i) {
snprintf(name, 16, "L_%08x", i);
snprintf(name, sizeof(name), "L_%08x", i);
change_tracker.MarkRprimDirty(l_inst.data->prim_id.AppendElementString(name),
pxr::HdChangeTracker::DirtyVisibility);
}
@ -321,15 +320,15 @@ bool InstancerData::is_instance_visible(Object *object)
pxr::SdfPath InstancerData::object_prim_id(Object *object) const
{
/* Making id of object in form like <prefix>_<pointer in 16 hex digits format> */
char str[32];
snprintf(str, 32, "O_%016llx", (uint64_t)object);
return prim_id.AppendElementString(str);
char name[32];
snprintf(name, sizeof(name), "O_%016llx", (uint64_t)object);
return prim_id.AppendElementString(name);
}
pxr::SdfPath InstancerData::light_prim_id(LightInstance const &inst, int index) const
{
char name[16];
snprintf(name, 16, "L_%08x", index);
snprintf(name, sizeof(name), "L_%08x", index);
return inst.data->prim_id.AppendElementString(name);
}

View File

@ -142,20 +142,20 @@ pxr::VtValue LightData::get_data(pxr::TfToken const &key) const
pxr::VtValue ret;
auto it = data_.find(key);
if (it != data_.end()) {
ret = it->second;
return pxr::VtValue(it->second);
}
else {
std::string n = key.GetString();
if (boost::algorithm::contains(n, "object:visibility:")) {
if (boost::algorithm::ends_with(n, "camera") || boost::algorithm::ends_with(n, "shadow")) {
ret = false;
return pxr::VtValue(false);
}
else {
ret = true;
return pxr::VtValue(true);
}
}
}
return ret;
return pxr::VtValue();
}
bool LightData::update_visibility()
@ -171,38 +171,32 @@ bool LightData::update_visibility()
pxr::TfToken LightData::prim_type(Light *light)
{
pxr::TfToken ret;
switch (light->type) {
case LA_LOCAL:
case LA_SPOT:
ret = pxr::HdPrimTypeTokens->sphereLight;
break;
return pxr::TfToken(pxr::HdPrimTypeTokens->sphereLight);
case LA_SUN:
ret = pxr::HdPrimTypeTokens->distantLight;
break;
return pxr::TfToken(pxr::HdPrimTypeTokens->distantLight);
case LA_AREA:
switch (light->area_shape) {
case LA_AREA_SQUARE:
case LA_AREA_RECT:
ret = pxr::HdPrimTypeTokens->rectLight;
break;
return pxr::TfToken(pxr::HdPrimTypeTokens->rectLight);
case LA_AREA_DISK:
case LA_AREA_ELLIPSE:
ret = pxr::HdPrimTypeTokens->diskLight;
break;
return pxr::TfToken(pxr::HdPrimTypeTokens->diskLight);
default:
ret = pxr::HdPrimTypeTokens->rectLight;
return pxr::TfToken(pxr::HdPrimTypeTokens->rectLight);
}
break;
default:
ret = pxr::HdPrimTypeTokens->sphereLight;
return pxr::TfToken(pxr::HdPrimTypeTokens->sphereLight);
}
return ret;
}
} // namespace blender::render::hydra

View File

@ -2,6 +2,7 @@
* Copyright 2011-2022 Blender Foundation */
#include <Python.h>
#include <unicodeobject.h>
#include <pxr/imaging/hd/material.h>
#include <pxr/imaging/hd/renderDelegate.h>
@ -74,14 +75,13 @@ void MaterialData::update()
pxr::VtValue MaterialData::get_data(pxr::TfToken const &key) const
{
pxr::VtValue ret;
if (key == scene_delegate_->settings.mx_filename_key) {
if (!mtlx_path_.GetResolvedPath().empty()) {
ret = mtlx_path_;
}
ID_LOG(3, "%s", key.GetText());
if (!mtlx_path_.GetResolvedPath().empty()) {
return pxr::VtValue(mtlx_path_);
}
return ret;
}
return pxr::VtValue();
}
pxr::VtValue MaterialData::get_material_resource() const
@ -112,7 +112,9 @@ void MaterialData::export_mtlx()
std::string path;
if (!PyErr_Occurred()) {
DagerD marked this conversation as resolved Outdated

Move PyUnicode_Check(result) into block below

Move `PyUnicode_Check(result)` into block below
if (PyUnicode_Check(result)) {
path = PyUnicode_AsUTF8(result);
}
Py_DECREF(result);
}
else {

View File

@ -26,18 +26,11 @@ void MeshData::init()
ID_LOG(1, "");
Object *object = (Object *)id;
if (object->type == OB_MESH && object->mode == OB_MODE_OBJECT &&
BLI_listbase_is_empty(&object->modifiers))
{
write_submeshes((Mesh *)object->data);
}
else {
Mesh *mesh = BKE_object_to_mesh(nullptr, object, false);
if (mesh) {
write_submeshes(mesh);
}
BKE_object_to_mesh_clear(object);
}
write_transform();
write_materials();
@ -86,17 +79,16 @@ void MeshData::update()
pxr::VtValue MeshData::get_data(pxr::SdfPath const &id, pxr::TfToken const &key) const
{
pxr::VtValue ret;
if (key == pxr::HdTokens->points) {
ret = vertices_;
return pxr::VtValue(vertices_);
}
else if (key == pxr::HdTokens->normals) {
ret = submesh(id).normals;
return pxr::VtValue(submesh(id).normals);
}
else if (key == pxr::HdPrimvarRoleTokens->textureCoordinate) {
ret = submesh(id).uvs;
return pxr::VtValue(submesh(id).uvs);
}
return ret;
return pxr::VtValue();
}
bool MeshData::update_visibility()
@ -194,7 +186,7 @@ pxr::SdfPathVector MeshData::submesh_paths() const
pxr::SdfPath MeshData::submesh_prim_id(int index) const
{
char name[16];
snprintf(name, 16, "SM_%04x", index);
snprintf(name, sizeof(name), "SM_%04x", index);
return prim_id.AppendElementString(name);
}

View File

@ -131,13 +131,12 @@ void WorldData::update(World *world)
pxr::VtValue WorldData::get_data(pxr::TfToken const &key) const
{
pxr::VtValue ret;
auto it = data_.find(key);
if (it != data_.end()) {
ret = it->second;
ID_LOG(3, "%s", key.GetText());
return pxr::VtValue(it->second);
}
return ret;
return pxr::VtValue();
}
void WorldData::write_transform()

View File

@ -255,7 +255,6 @@ void ViewportEngine::render(Depsgraph *depsgraph, bContext *context)
{
/* Release the GIL before calling into hydra, in case any hydra plugins call into python. */
pxr::TF_PY_ALLOW_THREADS_IN_SCOPE();
engine_->Execute(render_index_.get(), &tasks);
if ((bl_engine_->type->flag & RE_USE_GPU_CONTEXT) == 0) {