Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

Feature/cinema4d integration #3831

Closed
wants to merge 12 commits into from

Conversation

ninglesby
Copy link

@ninglesby ninglesby commented Sep 12, 2022

Brief description

Beginning work on a Cinema4D integration for OpenPype

Description

  • Added Cinema4D to the Launcher
  • Added .c4d as a file extensions associated with Cinema4D
  • Created a base Cinema4D Host
  • Created methods for creating selection sets in C4D with imprinted data
  • Created a Camera creator plugin
  • Created a Camera (Alembic) exporter plugin
  • Created a Workfile publish plugin
  • Created Set Frame range function for workfile settings
  • Created Set resolution function for workfile settings

Additional info

This is still very much a WIP. I haven't dived to deeply into figuring out if this is allowed for licensing reasons, but I am including a python3.dll because pyside is expecting that dll and it is not shipped with C4D so including that is the only way I found to get OP gui to work with C4D.

Feature Request

Documentation (add "type: documentation" label)

No documentation yet.

Testing notes:

This was built with r26, and they just changed their versioning scheme to the year so I think technically cinema4d 2023 is a minor update to r26. It should be backward compatible to r23 but I haven't fully tested that.

}
for key, value in defaults.items():
if not env.get(key):
env[key] = value
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no newline at end of file

# C4D's python ships without python3.dll which pyside expects
if "win" in env.get("PLAT"):
new_dll_path = [
os.path.join(pype_root, "openpype", "hosts", "cinema4d", "resource", "windows", "bin")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (98 > 79 characters)

env[pythonpath_key] = os.pathsep.join(new_c4dpython_path)


# C4D's python ships without python3.dll which pyside expects
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too many blank lines (2)

new_c4dpython_path.append(norm_path)


env[pythonpath_key] = os.pathsep.join(new_c4dpython_path)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too many blank lines (2)

pythonpath_key = "PYTHONPATH"

if int(_app.name) == 23:

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line contains whitespace

print("Not Implemented")

def reset_resolution():
WorkfileSettings().set_resolution()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no newline at end of file

def reset_colorspace():
print("Not Implemented")

def reset_resolution():
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected 2 blank lines, found 1

def reset_frame_range():
WorkfileSettings().set_frame_range_handles()

def reset_colorspace():
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected 2 blank lines, found 1

@@ -0,0 +1,10 @@
from .lib import WorkfileSettings

def reset_frame_range():
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected 2 blank lines, found 1

for key, value in defaults.items():
exporter[value["id"]] = settings.get(key) or value["default"]

c4d.documents.SaveDocument(doc, filepath, c4d.SAVEDOCUMENTFLAGS_DONTADDTORECENTLIST, c4d.FORMAT_ABCEXPORT)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no newline at end of file

Comment on lines +92 to +98
members_hierarchy = list(
set(
[str(x) for x in members] + \
[str(x) for x in children] + \
[str(x) for x in parents]
)
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
members_hierarchy = list(
set(
[str(x) for x in members] + \
[str(x) for x in children] + \
[str(x) for x in parents]
)
)
members_hierarchy = list(
set(str(x) for x in members + children + parents))
)

Would this be more readable?

Copy link
Collaborator

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you mentioned on Discord you're working on a draft implementation I hadn't expected you to be this far along. This really is very solid work. So, thank you from the community already!

It's a lot of code already - but I've taken a quick peek and commented some first things that stood out to me. Hopefully it's helpful.

Comment on lines +112 to +113
label = "{0} ({1})".format(name,
data["asset"])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
label = "{0} ({1})".format(name,
data["asset"])
label = "{0} ({1})".format(name, data["asset"])

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look! Sorry about the volume of code, I did a lot of copying and pasting from Maya/Blender/Nuke hosts to figure out how to put this together so I am sure there are a few missteps, and vestigial code from the other hosts that still needs to be cleaned out. I really appreciate you taking the time!

Comment on lines +84 to +85
# Collect Children - c4d.BaseObject
children = [lib.ObjectPath(obj=obj) for member in members for obj in lib.walk_hierarchy(member.obj.GetDown())]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note - I know NOTHING of Cinema4D but I had a quick glance over this page.
Wouldn't GetDown just return the "first child" of each node? Shouldn't this be GetChildren()?

It seems walk_hierarchy does seem to work around that with GetNext() - but wouldn't starting from the child only walk the hierarchy of those children. I feel the logic is a bit off somewhere.

Is it walk_hierarchy that is also walking the siblings of the object passed into it? I wouldn't expect that myself. I would've expected it to walk down the hierarchy from the node passed in and not touch its direct siblings.

Copy link
Author

@ninglesby ninglesby Sep 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should children include all descendants in the hierarchy or just the direct children of the object? I was collecting all children, GetChildren() is only direct children.

I see what your saying with walk_hierarchy not working as you would expect. There is a reason and I don't know if it is a good one. In C4D the pattern for listing most things in a project file is to get the first object, like doc.GetFirstMaterial() or doc.GetFirstObject(), which then requires something like:

obj = doc.GetFirstObject()
while obj:
   #do stuff with obj
   obj = obj.GetNext()

What I should probably do is just make a separate get_siblings() function for anything like that, so that walk_hierarchy operates more like os.walk

Copy link
Collaborator

@BigRoy BigRoy Sep 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should children include all descendants in the hierarchy or just the direct children of the object? I was collecting all children, GetChildren() is only direct children.

Your assumption is correct - it should be ALL children + grandchildren.

I see what your saying with walk_hierarchy not working as you would expect. There is a reason and I don't know if it is a good one. In C4D the pattern for listing most things in a project file is to get the first object, like doc.GetFirstMaterial() or doc.GetFirstObject(), which then requires something like:

Sort of, yes. I read the code as it currently doing this:

current_node = member.obj
first_child_of_current_node = member.obj.GetDown()
all_siblings_and_its_children_and_grandchildren = walk_hierarchy(first_child_of_current_node)

Which if that is the case then functionality wise it does do as it should. I just wouldn't have expected to have to pass the first child of the obj to walk its hierarchy.

So solely based on function name I wouldn't have expected walk_hierarchy to also go over "siblings" of the input object but would have instead expected it to just go down, and go from there. But I can see from the C4D logic point of view how walk_hierarchy with just the .Next() implementation is much more generic maybe. I somehow would've expected walk_hierarchy to first do a .GetDown() internally so that it's really only about walking down the hierarchy and its children's siblings, etc.

Comment on lines +115 to +144
# Append start frame and end frame to label if present
if "frameStart" and "frameEnd" in data:

# if frame range on maya set is the same as full shot range
# adjust the values to match the asset data
if (ctx_frame_start_handle == data["frameStart"]
and ctx_frame_end_handle == data["frameEnd"]): # noqa: W503, E501
data["frameStartHandle"] = ctx_frame_start_handle
data["frameEndHandle"] = ctx_frame_end_handle
data["frameStart"] = ctx_frame_start
data["frameEnd"] = ctx_frame_end
data["handleStart"] = ctx_handle_start
data["handleEnd"] = ctx_handle_end

# if there are user values on start and end frame not matching
# the asset, use them

else:
if "handles" in data:
data["handleStart"] = data["handles"]
data["handleEnd"] = data["handles"]
else:
data["handleStart"] = 0
data["handleEnd"] = 0

data["frameStartHandle"] = data["frameStart"] - data["handleStart"] # noqa: E501
data["frameEndHandle"] = data["frameEnd"] + data["handleEnd"] # noqa: E501

if "handles" in data:
data.pop('handles')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI - This logic can probably be greatly simplified, see #3437 - even though that one is still in discussion.

Comment on lines +42 to +48
ctx_frame_start = context.data['frameStart']
ctx_frame_end = context.data['frameEnd']
ctx_handle_start = context.data['handleStart']
ctx_handle_end = context.data['handleEnd']
ctx_frame_start_handle = context.data['frameStartHandle']
ctx_frame_end_handle = context.data['frameEndHandle']

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be removable - see comment down below about #3437

Comment on lines +16 to +24

Limitations:
- Does not take into account nodes connected to those
within an objectSet. Extractors are assumed to export
with history preserved, but this limits what they will
be able to achieve and the amount of data available
to validators. An additional collector could also
append this input data into the instance, as we do
for `pype.rig` with collect_history.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is coming from Maya implementation - and I'm not sure is relevant to your code.

Suggested change
Limitations:
- Does not take into account nodes connected to those
within an objectSet. Extractors are assumed to export
with history preserved, but this limits what they will
be able to achieve and the amount of data available
to validators. An additional collector could also
append this input data into the instance, as we do
for `pype.rig` with collect_history.

@antirotor
Copy link
Member

That is really impressive! What I would suggest is that you'll move the creator/publishing logic to the new infrastructure we call .... the new publisher! :) I can help you with that as I am now finishing Houdini transition to that. It can probably help you with a few things (and complicate few more) but it is worth it.

@ninglesby
Copy link
Author

@antirotor Is there a good spot to see an example of what that should look like?

@BigRoy
Copy link
Collaborator

BigRoy commented Sep 13, 2022

Is there a good spot to see an example of what that should look like?

This is the Houdini PR: #3046
Which is a good reference for the changes involved.
Here's a similar but older one for After Effects: #2838

And according to this PR #2896 that added documentation for the New Publisher this is the related documentation page:

@antirotor antirotor added host: Cinema4D Cinema 4D type: feature Larger, user affecting changes and completely new things labels Sep 13, 2022
@antirotor
Copy link
Member

Exactly as @BigRoy listed. In the Houdini integration there is this part of code:

https://github.com/pypeclub/OpenPype/blob/9b32b4926ce8eb3356c9aea899acf05b0fe77ece/openpype/hosts/houdini/api/plugin.py#L95-L182

defining common "new" Creator. Of note there is collect_instances() that should pick up any created instances from the current scene - in this particular case it looks for the nodes with attribute id containing specific signature string - but that can reflect the logic of the host that suits it more. For example in Maya, it would go over the sets and look for such attribute there or you can use completely different logic how to store instance data. We think it is better to use general stuff available to you as opposed to for example some custom types, because that might breakt the scene if you open in in "vanilla" DCC. You can still work with the scene files coming from OpenPype Maya in vanilla Maya because there is no custom magic.

get_pre_create_attr_defs() defines options on the creator, in Houdini case there is the "use selection" option that is used during the creation, but there might be creators where this option isn't needed so that creator can override this to fit it's needs.

Everything is written in the documentation BigRoy mentioned but if you have any questions, we are here for you.

@ninglesby
Copy link
Author

Thanks! I am going to dig into this and let you know if I hit any roadblocks.

@BigRoy
Copy link
Collaborator

BigRoy commented Dec 22, 2022

@ninglesby Any chance we can see some activity on this again? Would love to see this moving into the community. Let us know where we can help out.

@Minkiu
Copy link
Contributor

Minkiu commented Sep 20, 2023

Hey @ninglesby friendly ping about this PR, are you able to dedicate time to it?

@BigRoy
Copy link
Collaborator

BigRoy commented Sep 20, 2023

@Minkiu If there's big interest on this I can try and spend some spare time on this - that is, if there are testers available. ;)

@mkolar
Copy link
Member

mkolar commented Jan 25, 2024

I'm afraid we might need to close this stale PR. cinema4d would be great, but the demand is really minimal and @ninglesby seems quiet for a while. Better to put efforts into C4D for AYON if it comes to it.

@mkolar mkolar closed this Jan 25, 2024
@ynbot ynbot added this to the next-patch milestone Jan 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community contribution host: Cinema4D Cinema 4D type: feature Larger, user affecting changes and completely new things
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants