BogdanNagirniak
/
blender
forked from blender/blender
0
0
Fork 3
Code Pull Requests Activity

Move to use blender::Map container instead std::unordered_map #47

Merged
Bogdan Nagirniak merged 17 commits from Vasyl-Pidhirskyi/blender_bn:BLEN-418 into hydra-render 2023-06-02 12:02:46 +02:00
Conversation 0 Commits 17 Files Changed 12 +160 -153
Vasyl Pidhirskyi commented 2023-05-29 21:26:21 +02:00
Collaborator
Copy Link

Purpose

Move to use blender::Map container instead std::unordered_map.
Move to use blender::Set container instead std::set.

Technical steps

Adjusted code according to BLI_map.hh, BLI_set.hhand BLI_hash.hh implementations.
Refactored InstancerDataMap, ObjectDataMap, MaterialDataMap, mesh_instances_, light_instances_, available_objects, and available_materials.

### Purpose Move to use blender::Map container instead std::unordered_map. Move to use blender::Set container instead std::set. ### Technical steps Adjusted code according to `BLI_map.hh`, `BLI_set.hh`and `BLI_hash.hh` implementations. Refactored `InstancerDataMap`, `ObjectDataMap`, `MaterialDataMap`, `mesh_instances_`, `light_instances_`, `available_objects`, and `available_materials`.
Vasyl Pidhirskyi added 8 commits 2023-05-29 21:26:34 +02:00
Vasyl Pidhirskyi requested review from Brian Savery (AMD) 2023-05-29 21:26:47 +02:00
Vasyl Pidhirskyi requested review from Georgiy Markelov 2023-05-29 21:26:47 +02:00
Brian Savery (AMD) was assigned by Vasyl Pidhirskyi 2023-05-29 21:27:06 +02:00
Georgiy Markelov was assigned by Vasyl Pidhirskyi 2023-05-29 21:27:06 +02:00
Bogdan Nagirniak was assigned by Vasyl Pidhirskyi 2023-05-29 21:27:07 +02:00
Vasyl Pidhirskyi self-assigned this 2023-05-29 21:27:07 +02:00
Georgiy Markelov requested changes 2023-05-30 13:22:05 +02:00
Georgiy Markelov left a comment
Collaborator
Copy Link

see comments

see comments
source/blender/render/hydra/scene_delegate/blender_scene_delegate.cc Outdated
@ -350,3 +350,3 @@
}
objects_[id] = ObjectData::create(this, object, id);
objects_.add_overwrite(id, ObjectData::create(this, object, id));
Georgiy Markelov commented 2023-05-30 12:55:22 +02:00
Collaborator
Copy Link

seems like here should be add_new.

seems like here should be `add_new`.
Vasyl-Pidhirskyi marked this conversation as resolved
source/blender/render/hydra/scene_delegate/blender_scene_delegate.cc Outdated
@ -387,3 +387,3 @@
}
instancers_[id] = std::make_unique<InstancerData>(this, object, id);
instancers_.add_overwrite(id, std::make_unique<InstancerData>(this, object, id));
Georgiy Markelov commented 2023-05-30 13:01:44 +02:00
Collaborator
Copy Link

seems like here should be add_new.

seems like here should be `add_new`.
Vasyl-Pidhirskyi marked this conversation as resolved
source/blender/render/hydra/scene_delegate/curves.cc Outdated
@ -200,3 +200,3 @@
mat_data_ = scene_delegate_->material_data(p_id);
if (!mat_data_) {
scene_delegate_->materials_[p_id] = std::make_unique<MaterialData>(scene_delegate_, mat, p_id);
scene_delegate_->materials_.add_overwrite(
Georgiy Markelov commented 2023-05-30 13:03:14 +02:00
Collaborator
Copy Link

seems like here should be add_new.

seems like here should be `add_new`.
Vasyl-Pidhirskyi marked this conversation as resolved
source/blender/render/hydra/scene_delegate/instancer.cc Outdated
@ -393,3 +389,1 @@
mesh_instances_.erase(it);
it = mesh_instances_.begin();
}
mesh_instances_.remove_if([&](auto item) { return item.value.indices.empty(); });
Georgiy Markelov commented 2023-05-30 13:19:03 +02:00
Collaborator
Copy Link

missed .data->remove()?

missed `.data->remove()`?
Vasyl-Pidhirskyi marked this conversation as resolved
source/blender/render/hydra/scene_delegate/mesh.cc Outdated
@ -306,3 +306,2 @@
if (!m.mat_data) {
scene_delegate_->materials_[p_id] = std::make_unique<MaterialData>(
scene_delegate_, mat, p_id);
scene_delegate_->materials_.add_overwrite(
Georgiy Markelov commented 2023-05-30 13:21:18 +02:00
Collaborator
Copy Link

seems like here should be add_new.

seems like here should be `add_new`.
Vasyl-Pidhirskyi marked this conversation as resolved
Vasyl Pidhirskyi added 2 commits 2023-05-30 15:05:27 +02:00
Georgiy Markelov requested changes 2023-05-30 15:46:57 +02:00
source/blender/render/hydra/scene_delegate/instancer.cc Outdated
@ -363,3 +362,3 @@
LightInstance *inst = light_instance(p_id);
if (!inst) {
inst = &light_instances_[p_id];
inst = light_instances_.lookup_ptr(p_id);
Georgiy Markelov commented 2023-05-30 14:10:04 +02:00
Collaborator
Copy Link

lookup instead of lookup_ptr also crash here due to inst is empty

`lookup` instead of `lookup_ptr` also crash here due to `inst` is empty
Vasyl-Pidhirskyi marked this conversation as resolved
source/blender/render/hydra/scene_delegate/instancer.cc Outdated
@ -376,3 +374,1 @@
inst->data = std::make_unique<MeshData>(scene_delegate_, ob, p_id);
inst->data->init();
inst->data->insert();
inst = mesh_instances_.lookup_ptr(p_id);
Georgiy Markelov commented 2023-05-30 14:10:12 +02:00
Collaborator
Copy Link

lookup instead of lookup_ptr

`lookup` instead of `lookup_ptr`
Vasyl-Pidhirskyi marked this conversation as resolved
source/blender/render/hydra/scene_delegate/instancer.cc Outdated
@ -377,2 +374,2 @@
inst->data->init();
inst->data->insert();
inst = mesh_instances_.lookup_ptr(p_id);
if (inst) {
Georgiy Markelov commented 2023-05-30 14:04:38 +02:00
Collaborator
Copy Link

check logic here

check logic here
Vasyl-Pidhirskyi marked this conversation as resolved
Vasyl Pidhirskyi added 1 commit 2023-05-30 20:31:34 +02:00
Vasyl Pidhirskyi added 1 commit 2023-05-30 20:35:28 +02:00
Georgiy Markelov requested changes 2023-05-31 12:15:24 +02:00
source/blender/render/hydra/scene_delegate/instancer.cc Outdated
@ -464,2 +459,2 @@
auto it = mesh_instances_.find(id.GetPathElementCount() == 4 ? id.GetParentPath() : id);
if (it == mesh_instances_.end()) {
auto it = mesh_instances_.lookup_ptr(id.GetPathElementCount() == 4 ? id.GetParentPath() : id);
if (it == nullptr) {
Georgiy Markelov commented 2023-05-31 12:14:05 +02:00
Collaborator
Copy Link

if (!it) {
}

if (!it) { }
Vasyl-Pidhirskyi marked this conversation as resolved
source/blender/render/hydra/scene_delegate/instancer.cc Outdated
@ -473,2 +468,2 @@
auto it = light_instances_.find(id.GetPathElementCount() == 4 ? id.GetParentPath() : id);
if (it == light_instances_.end()) {
auto it = light_instances_.lookup_ptr(id.GetPathElementCount() == 4 ? id.GetParentPath() : id);
if (it == nullptr) {
Georgiy Markelov commented 2023-05-31 12:14:40 +02:00
Collaborator
Copy Link

if (!it) {
}

if (!it) { }
Vasyl-Pidhirskyi marked this conversation as resolved
Georgiy Markelov requested changes 2023-05-31 12:17:06 +02:00
source/blender/render/hydra/scene_delegate/blender_scene_delegate.cc Outdated
@ -213,3 +213,2 @@
{
for (auto &it : objects_) {
it.second->remove();
for (auto &it : objects_.values()) {
Georgiy Markelov commented 2023-05-31 12:17:04 +02:00
Collaborator
Copy Link

rename please it -> val as it misleads because values() doesn't return iterator.

rename please `it` -> `val` as it misleads because `values()` doesn't return iterator.
Vasyl-Pidhirskyi marked this conversation as resolved
Vasyl Pidhirskyi added 1 commit 2023-05-31 12:38:35 +02:00
Bogdan Nagirniak requested changes 2023-05-31 16:48:39 +02:00
Bogdan Nagirniak left a comment
Owner
Copy Link

Works good, but requires some renamings and improvements in using functions

Works good, but requires some renamings and improvements in using functions
source/blender/render/hydra/scene_delegate/blender_scene_delegate.cc Outdated
@ -213,3 +213,2 @@
{
for (auto &it : objects_) {
it.second->remove();
for (auto &val : objects_.values()) {
Bogdan Nagirniak commented 2023-05-31 15:48:35 +02:00
Owner
Copy Link

rename val -> data

rename `val` -> `data`
BogdanNagirniak marked this conversation as resolved
source/blender/render/hydra/scene_delegate/blender_scene_delegate.cc Outdated
@ -219,3 +219,2 @@
}
for (auto &it : materials_) {
it.second->remove();
for (auto &val : materials_.values()) {
Bogdan Nagirniak commented 2023-05-31 16:27:40 +02:00
Owner
Copy Link

rename val -> obj_data, i_data, m_data

rename `val` -> `obj_data`, `i_data`, `m_data`
BogdanNagirniak marked this conversation as resolved
source/blender/render/hydra/scene_delegate/blender_scene_delegate.cc Outdated
@ -273,2 +272,2 @@
if (it != objects_.end()) {
return it->second.get();
const std::unique_ptr<ObjectData> *val = objects_.lookup_ptr(p_id);
if (val != nullptr) {
Bogdan Nagirniak commented 2023-05-31 16:30:32 +02:00
Owner
Copy Link
auto obj_data = objects_.lookup_ptr(p_id);
if (obj_data) {
``` auto obj_data = objects_.lookup_ptr(p_id); if (obj_data) { ```
BogdanNagirniak marked this conversation as resolved
source/blender/render/hydra/scene_delegate/blender_scene_delegate.cc Outdated
@ -361,3 +361,2 @@
/* Check object inside instancers */
for (auto &it : instancers_) {
it.second->check_update(object);
for (auto &val : instancers_.values()) {
Bogdan Nagirniak commented 2023-05-31 16:31:10 +02:00
Owner
Copy Link

val -> i_data

`val` -> `i_data`
BogdanNagirniak marked this conversation as resolved
source/blender/render/hydra/scene_delegate/blender_scene_delegate.cc Outdated
@ -387,3 +387,3 @@
}
instancers_[id] = std::make_unique<InstancerData>(this, object, id);
instancers_.add_new(id, std::make_unique<InstancerData>(this, object, id));
Bogdan Nagirniak commented 2023-05-31 16:33:17 +02:00
Owner
Copy Link

use lookup_or_add_default

use `lookup_or_add_default`
Vasyl Pidhirskyi commented 2023-06-01 15:46:24 +02:00
Author
Collaborator
Copy Link

implemented via lookup_or_add

implemented via `lookup_or_add`
BogdanNagirniak marked this conversation as resolved
source/blender/render/hydra/scene_delegate/blender_scene_delegate.cc Outdated
@ -572,3 +570,4 @@
});
/* Remove unused materials */
std::set<pxr::SdfPath> available_materials;
Bogdan Nagirniak commented 2023-05-31 16:47:51 +02:00
Owner
Copy Link

Try to use blender::Set here

Try to use `blender::Set` here
Vasyl-Pidhirskyi marked this conversation as resolved
source/blender/render/hydra/scene_delegate/mesh.cc Outdated
@ -306,3 +306,2 @@
if (!m.mat_data) {
scene_delegate_->materials_[p_id] = std::make_unique<MaterialData>(
scene_delegate_, mat, p_id);
scene_delegate_->materials_.add_new(
Bogdan Nagirniak commented 2023-05-31 16:46:36 +02:00
Owner
Copy Link

lookup_or_add_default

`lookup_or_add_default`
Vasyl Pidhirskyi commented 2023-06-01 15:46:15 +02:00
Author
Collaborator
Copy Link

implemented via lookup_or_add

implemented via `lookup_or_add`
BogdanNagirniak marked this conversation as resolved
Brian Savery (AMD) approved these changes 2023-05-31 19:53:00 +02:00
Vasyl Pidhirskyi added 1 commit 2023-05-31 22:21:25 +02:00
Vasyl Pidhirskyi added 1 commit 2023-06-01 08:53:36 +02:00
Vasyl Pidhirskyi added 1 commit 2023-06-01 15:45:45 +02:00
Vasyl Pidhirskyi requested review from Bogdan Nagirniak 2023-06-01 15:49:58 +02:00
Vasyl Pidhirskyi added 1 commit 2023-06-01 15:50:11 +02:00
Georgiy Markelov approved these changes 2023-06-02 11:54:13 +02:00
Georgiy Markelov left a comment
Collaborator
Copy Link

Works fine

Works fine
Bogdan Nagirniak approved these changes 2023-06-02 12:01:10 +02:00
Bogdan Nagirniak left a comment
Owner
Copy Link

Tested - works good

Tested - works good
Bogdan Nagirniak merged commit 00167e6693 into hydra-render 2023-06-02 12:02:46 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Brian Savery (AMD)
Georgiy Markelov
Bogdan Nagirniak
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
Brian Savery (AMD)
Bogdan Nagirniak
Vasyl Pidhirskyi
4 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#47
No description provided.
Delete Branch "Vasyl-Pidhirskyi/blender_bn:BLEN-418"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?