Houdini: OpenGL Review support LOPs/Solaris source mode #4890

BigRoy · 2023-04-21T12:03:06Z

Changelog Description

Allow reviews using the openGL node to use LOP Paths.

Additional info

Based on comment here that OpenGL review node publishing didn't support Solaris reviews yet using LOP paths.

TODO:

Update creator to allow 'use selection' for LOP paths and then have it auto-set to LOPs source mode.
Update collector to collect focal lengths for LOPs camera
- Make sure it always collects the correct data (see todo in code)
Update validator to allow LOPs scene path, etc.
Test

Testing notes:

Set up openGL review instance to use LOP path
Publish review
Set up openGL review instance to use OBJ camera path
Publish review

Make sure to publish with reviews enabled so it also tests the focal length burnin.

…views - However, note the todo. That's a massive implementation issue likely?

moonyuet · 2023-04-25T07:31:03Z

Guess the first TODO is finished if the LOP path is included as the force object. But if you are talking about camera inside LOP network, pretty sure it still needs some work.

(will investigate the collect review to see if it collects the LOP's camera)

moonyuet · 2023-04-25T07:40:29Z

checked the collector, it seems that it doesn't collect the correct focal length from camera in LOP networks

BigRoy · 2023-04-25T07:45:59Z

@moonyuet You will at least need to set it to LOPs source mode at the top. Without that you can't extract openGL with LOPs content.

moonyuet · 2023-04-25T07:50:30Z

@moonyuet You will at least need to set it to LOPs source mode at the top. Without that you can't extract openGL with LOPs content.

Thank you, the collector works to collect the focal length.

…as source in OP Creator

… parm type is lopnet

…s://github.com/BigRoy/OpenPype into enhancement/houdini_opengl_review_support_lops

moonyuet · 2023-06-21T14:39:44Z

@BigRoy I have updated the code for validating LOP networks and both add LOP networks and cameras into the opengl if the use selection enabled.
I also add an options for user to enable "LOP as source"(not sure if it is a good name). If they want to use LOP networks for review, they can enable it in the creator.

BigRoy · 2023-06-21T18:05:55Z

I also add an options for user to enable "LOP as source"(not sure if it is a good name). If they want to use LOP networks for review, they can enable it in the creator.

Can't we match what Houdini does with a "Source" enum attribute that is either "LOPs" or "SOPs"? Just so it's also familiar to those working with the OpenGL rop node?

BigRoy · 2023-06-21T18:07:49Z

openpype/hosts/houdini/plugins/create/create_review.py

@@ -56,30 +57,48 @@ def create(self, subset_name, instance_data, pre_create_data):
                "aspect": pre_create_data.get("aspect"),
            })

+        lop_enabled = pre_create_data.get("lopEnabled")


Can't we just stick close to what Houdini uses internally and define a opSource or source type of data which defines either "SOPs" or "LOPs". That would also leave the path open to potential other sources that might ever appear in the future in Houdini? Plus it's also clearer that when LOPs is not chosen that it uses SOPs.

BigRoy · 2023-06-21T18:10:32Z

openpype/hosts/houdini/plugins/publish/validate_scene_review.py

+        lop_node = hou.node(node_name)
+        if lop_node.type().name() != "lopnet":
+            return [node_name]


This seems like an invalid requirement. The LOP Network parameter can be set to any LOP node inside the LOP network and is not necessarily the LOP network itself. Right?

BigRoy · 2023-06-21T18:12:21Z

openpype/hosts/houdini/plugins/publish/validate_scene_review.py

+    def get_invalid_resolution(self, instance):
+        node = hou.node(instance.data.get("instance_node"))

        # The resolution setting is only used when Override Camera Resolution
        # is enabled. So we skip validation if it is disabled.
-        override = rop_node.parm("tres").eval()
+        override = node.parm("tres").eval()
        if not override:
            return

        invalid = []
-        res_width = rop_node.parm("res1").eval()
-        res_height = rop_node.parm("res2").eval()
+        res_width = node.parm("res1").eval()
+        res_height = node.parm("res2").eval()


Not sure why this was changed, but the old logic made more sense where it took the rop_node directly. Especially because the instance ROP node already seems to be retrieved in process

maybe thid is the issue coming from merging develop to the branch. i agree that rop_node is more concise than node

MustafaJafar · 2023-07-20T17:08:49Z

I tested it by modifying rop node manually and it successfully published with no errors

I have an idea to update the creator, I'm going to try it.
pseudocode something like this

In LOPs Context

  case 1: No Selection 
    will only set `source` to `LOPs`
  
  case 2: one node selected 
    use its path as `loppath`
    if camera: 
      use its primpath as `cameraprim`
  
  case 3: two or more are selected 
    use camera's primpath as `cameraprim`
    pop camera from selection
    use first node as `loppath`

Do you think it would be a good thing to differentiate between LOPs cameras and OBJs camera by something like this in the collector:

if obj_camera:
    instance.data["families"].append("review.obj")
else:
    instance.data["families"].append("review.lop")

this idea was inspired that houdini pointcache issue

I think this will be a good idea so that each can have proper validators with clear objective.

MustafaJafar · 2023-07-20T23:17:57Z

I gave it a shot.
I believe we will need at first to filter selected_nodes to sperate hou.ObjNode(s) from hou.LopNode(s)
because user can accidently have multiple selection from different contexts which can be confusing.

I may made that filtration in a different way.

Imagine lop camera was the first node in selection, a user will complaint about selecting the obj camera but a lop camera was set!

in the attached code, for this particular case the camera will be empty instead.

I can try again later.

Here you are what I have achieved (with a lot of overthinking)
create_review.zip

moonyuet · 2023-09-27T08:32:40Z

I gave it a shot. I believe we will need at first to filter selected_nodes to sperate hou.ObjNode(s) from hou.LopNode(s) because user can accidently have multiple selection from different contexts which can be confusing.

I may made that filtration in a different way.

Imagine lop camera was the first node in selection, a user will complaint about selecting the obj camera but a lop camera was set!

in the attached code, for this particular case the camera will be empty instead.

I can try again later.

Here you are what I have achieved (with a lot of overthinking) create_review.zip

I agree with you that we can append different families for the validation of different networks.
But I think it's a bit overthinking for the creator.
We can make some validators to make sure the nodes are LOP nodes if source type are set to LOP network, while validating the nodes are OBJ nodes if Objects are set as source type.
We need to probably validate the LOP Nodes such as Primitive or Reference node to make sure the primitive path are correct.

mkolar · 2024-02-06T21:58:22Z

Because we're splitting OpenPype into ayon-core and individual host addons, this PR would have to be re-created to target one of those.

We're closing it down, but we'll he happy for a new PR to ynput/ayon-houdini repository once it's up.

Draft implementation to collect focal length values for LOP openGL re…

575241e

…views - However, note the todo. That's a massive implementation issue likely?

BigRoy added type: enhancement Enhancements to existing functionality host: Houdini community contribution labels Apr 21, 2023

BigRoy requested review from antirotor and moonyuet April 21, 2023 12:03

ynbot added the size/S Denotes a PR changes 100-499 lines, ignoring general files label Apr 21, 2023

antirotor mentioned this pull request May 3, 2023

Houdini: Don't error in collect review if camera is not set correctly #4874

Merged

moonyuet added 6 commits June 21, 2023 21:59

update use selection LOP networks and an option for switching to LOP …

071ef7b

…as source in OP Creator

add validations for validating LOP network path and validating if the…

f23edd7

… parm type is lopnet

merge develop to the branch

5de1105

Merge branch 'enhancement/houdini_opengl_review_support_lops' of http…

a02e997

…s://github.com/BigRoy/OpenPype into enhancement/houdini_opengl_review_support_lops

fix the bug after conflict resolved for merging the develop branch

7c72943

hound fix

c8c7d50

moonyuet assigned antirotor Jun 21, 2023

ynbot added the host: UE label Jun 21, 2023

BigRoy commented Jun 21, 2023

View reviewed changes

moonyuet added 3 commits June 23, 2023 16:24

roy's comment

acfbe95

hound fix

374a27e

add fps data into review data

a32f378

mkolar closed this Feb 6, 2024

ynbot added this to the next-patch milestone Feb 6, 2024

jakubjezek001 removed this from the next-patch milestone Feb 15, 2024

BigRoy mentioned this pull request Jul 13, 2023

Enhancement: Houdini: Unable to review/Render using Lopimportcamera ynput/ayon-houdini#23

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Houdini: OpenGL Review support LOPs/Solaris source mode #4890

Houdini: OpenGL Review support LOPs/Solaris source mode #4890

BigRoy commented Apr 21, 2023

moonyuet commented Apr 25, 2023 •

edited

Loading

moonyuet commented Apr 25, 2023

BigRoy commented Apr 25, 2023

moonyuet commented Apr 25, 2023 •

edited

Loading

moonyuet commented Jun 21, 2023

BigRoy commented Jun 21, 2023

BigRoy Jun 21, 2023

BigRoy Jun 21, 2023

BigRoy Jun 21, 2023 •

edited

Loading

moonyuet Jun 22, 2023

MustafaJafar commented Jul 20, 2023 •

edited

Loading

MustafaJafar commented Jul 20, 2023 •

edited

Loading

moonyuet commented Sep 27, 2023

mkolar commented Feb 6, 2024

Houdini: OpenGL Review support LOPs/Solaris source mode #4890

Houdini: OpenGL Review support LOPs/Solaris source mode #4890

Conversation

BigRoy commented Apr 21, 2023

Changelog Description

Additional info

Testing notes:

moonyuet commented Apr 25, 2023 • edited Loading

moonyuet commented Apr 25, 2023

BigRoy commented Apr 25, 2023

moonyuet commented Apr 25, 2023 • edited Loading

moonyuet commented Jun 21, 2023

BigRoy commented Jun 21, 2023

BigRoy Jun 21, 2023

Choose a reason for hiding this comment

BigRoy Jun 21, 2023

Choose a reason for hiding this comment

BigRoy Jun 21, 2023 • edited Loading

Choose a reason for hiding this comment

moonyuet Jun 22, 2023

Choose a reason for hiding this comment

MustafaJafar commented Jul 20, 2023 • edited Loading

MustafaJafar commented Jul 20, 2023 • edited Loading

moonyuet commented Sep 27, 2023

mkolar commented Feb 6, 2024

moonyuet commented Apr 25, 2023 •

edited

Loading

moonyuet commented Apr 25, 2023 •

edited

Loading

BigRoy Jun 21, 2023 •

edited

Loading

MustafaJafar commented Jul 20, 2023 •

edited

Loading

MustafaJafar commented Jul 20, 2023 •

edited

Loading