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

Added back set_sync_settings #86

Closed
Bogdan Nagirniak wants to merge 2 commits from hydra-sync_settings into hydra-render
pull from: hydra-sync_settings
merge into: BogdanNagirniak:hydra-render

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 4 Commits 2 Files Changed 8 +73 -6
Bogdan Nagirniak commented 2023-08-03 19:25:52 +02:00
Owner
Copy Link

Purpose

set_sync_settings() are used to provide specific settings for Hydra scene delegate required for some render delegate addon, e.g. https://github.com/bnagirniak/RPRHydraRenderBlenderAddon/blob/add-lightToken/src/hydrarpr/engine.py#L44 .

Technical steps

Added back set_sync_settings() to HydraRenderEngine. Made them more specific.
Added struct HydraSceneDelegate::SyncSettings.

Note

Works with https://github.com/bnagirniak/RPRHydraRenderBlenderAddon/pull/26

### Purpose `set_sync_settings()` are used to provide specific settings for Hydra scene delegate required for some render delegate addon, e.g. https://github.com/bnagirniak/RPRHydraRenderBlenderAddon/blob/add-lightToken/src/hydrarpr/engine.py#L44 . ### Technical steps Added back `set_sync_settings()` to `HydraRenderEngine`. Made them more specific. Added `struct HydraSceneDelegate::SyncSettings`. ### Note Works with https://github.com/bnagirniak/RPRHydraRenderBlenderAddon/pull/26
Bogdan Nagirniak added 2 commits 2023-08-03 19:26:12 +02:00 
Added HydraSceneDelegate::SyncSettings.
Bogdan Nagirniak requested review from Brecht Van Lommel 2023-08-03 19:26:27 +02:00
Bogdan Nagirniak requested review from Brian Savery (AMD) 2023-08-03 19:26:28 +02:00
Bogdan Nagirniak requested review from Georgiy Markelov 2023-08-03 19:26:28 +02:00
Bogdan Nagirniak requested review from Vasyl Pidhirskyi 2023-08-03 19:26:28 +02:00
Brian Savery (AMD) approved these changes 2023-08-03 19:33:22 +02:00
Brian Savery (AMD) left a comment
Collaborator
Copy Link

The name is kinda weird IMO. "sync_settings"?
Also could we just pass a dictionary rather than doing one at a time?

The name is kinda weird IMO. "sync_settings"? Also could we just pass a dictionary rather than doing one at a time?
Bogdan Nagirniak commented 2023-08-03 20:07:42 +02:00
Author
Owner
Copy Link

The name is kinda weird IMO. "sync_settings"?

Maybe set_hydra_settings ?

Also could we just pass a dictionary rather than doing one at a time?

Yes, but it requires to parse python dict on C++ side instead of python side. Not hard, but not sure if this really need here.

> The name is kinda weird IMO. "sync_settings"? Maybe `set_hydra_settings` ? > Also could we just pass a dictionary rather than doing one at a time? Yes, but it requires to parse python dict on C++ side instead of python side. Not hard, but not sure if this really need here.
Bogdan Nagirniak commented 2023-08-04 13:34:02 +02:00
Author
Owner
Copy Link

Seems this PR opens more questions, there is another approach of how to provide some custom settings to hydra prims, not to use sync_settings, so we can look at approach with USD hook in blender/blender#108823. But we'll start thinking about it after hydra-render merge.
Therefore, I'll close this PR.

Seems this PR opens more questions, there is another approach of how to provide some custom settings to hydra prims, not to use `sync_settings`, so we can look at approach with USD hook in https://projects.blender.org/blender/blender/pulls/108823. But we'll start thinking about it after hydra-render merge. Therefore, I'll close this PR.
Bogdan Nagirniak closed this pull request 2023-08-04 13:34:02 +02:00
Brecht Van Lommel commented 2023-08-04 14:01:36 +02:00
Collaborator
Copy Link

Besides hooks, if performance is a concern then an alternative would be to provide a mapping to Blender properties.

So for example we have built-in Object.visible_camera, which could be mapped to rpr:object:visibility:camera. But not just built-in Blender properties, also any custom properties registered by the renderer could do this. Perhaps even with wildcards from e.g. scene.rpr.* to rpr:scene:*.

The advantage is that this mapping could be done in C++, with all the strings interned and hashed already, RNA properties looked up and validated. On the other hand a Python hook could do more complicated logic which may be required in some cases, I'm not sure.

Besides hooks, if performance is a concern then an alternative would be to provide a mapping to Blender properties. So for example we have built-in `Object.visible_camera`, which could be mapped to `rpr:object:visibility:camera`. But not just built-in Blender properties, also any custom properties registered by the renderer could do this. Perhaps even with wildcards from e.g. `scene.rpr.*` to `rpr:scene:*`. The advantage is that this mapping could be done in C++, with all the strings interned and hashed already, RNA properties looked up and validated. On the other hand a Python hook could do more complicated logic which may be required in some cases, I'm not sure.
Brian Savery (AMD) commented 2023-08-04 17:04:16 +02:00
Collaborator
Copy Link

Besides hooks, if performance is a concern then an alternative would be to provide a mapping to Blender properties.

So for example we have built-in Object.visible_camera, which could be mapped to rpr:object:visibility:camera. But not just built-in Blender properties, also any custom properties registered by the renderer could do this. Perhaps even with wildcards from e.g. scene.rpr.* to rpr:scene:*.

The advantage is that this mapping could be done in C++, with all the strings interned and hashed already, RNA properties looked up and validated. On the other hand a Python hook could do more complicated logic which may be required in some cases, I'm not sure.

I think the "other hand" of flexibility needed for render engines defined in Python is going to be needed. But for sure there is probably a better way to do this.

My point I made offline is that there is some overlap with what is being done here with the "python chasers" PR and should be made as close as possible.

> Besides hooks, if performance is a concern then an alternative would be to provide a mapping to Blender properties. > > So for example we have built-in `Object.visible_camera`, which could be mapped to `rpr:object:visibility:camera`. But not just built-in Blender properties, also any custom properties registered by the renderer could do this. Perhaps even with wildcards from e.g. `scene.rpr.*` to `rpr:scene:*`. > > The advantage is that this mapping could be done in C++, with all the strings interned and hashed already, RNA properties looked up and validated. On the other hand a Python hook could do more complicated logic which may be required in some cases, I'm not sure. I think the "other hand" of flexibility needed for render engines defined in Python is going to be needed. But for sure there is probably a better way to do this. My point I made offline is that there is some overlap with what is being done here with the "python chasers" PR and should be made as close as possible.

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Brecht Van Lommel
Georgiy Markelov
Vasyl Pidhirskyi
Brian Savery (AMD)
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
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#86
No description provided.
Delete Branch "hydra-sync_settings"

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?