Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sverchok open subdiv #4590

Merged
merged 14 commits into from
Aug 8, 2022
Merged

Conversation

GeneralPancakeMSTR
Copy link
Contributor

@GeneralPancakeMSTR GeneralPancakeMSTR commented Jul 23, 2022

Addressed problem description

#4575 Add Catmull-Clark subdivision to Sverchok

  • Adds the OpenSubdiv node.
  • Uses include parametric hard-surface modeling, direct subdivision of mesh data.

Solution description

  • Added OpenSubdiv node, which implements Catmull-Clark subdivision standard to most 3d-modeling tools.
  • Implementation is a python wrapper around the OpenSubdiv C++ library.

Preflight checklist

  • Code changes complete.

  • Code documentation complete.

  • Documentation for users complete (or not required, if user never sees these changes).

  • Manual testing done.

  • [~] Unit-tests implemented.

    • I have not implemented unit tests in the strict sense of the term, but have run a complete manual test, including freshly installing Sverchok, installing the pyOpenSubdiv dependency, and testing the OpenSubdiv node.
    • Install Test: sverchok_install
    • Node test: sverchok_test
  • Ready for merge.

@portnov
Copy link
Collaborator

portnov commented Jul 23, 2022

@zeffii please take a look.

@zeffii
Copy link
Collaborator

zeffii commented Jul 24, 2022

@portnov this was bound to happen at some point. It will be interesting to see how stable it is, or if input-checking (to ensure stability) will introduce a noticeable penalty.

@Durman
Copy link
Collaborator

Durman commented Jul 25, 2022

The test fails because:

image

You have to add file with documentation into docs.nodes.modifier_change folder in rst format and also add the name of the file with documentation into list in modifier_change_index.rst file. You can at least create empty file with documentation.

I've added a few lines about nodes documentation - http://nortikin.github.io/sverchok/docs/contributing/node_api.html#documentation

@portnov
Copy link
Collaborator

portnov commented Jul 25, 2022

@zeffii my concern here is about storing dll/so in the repo... Doesn't seem to be a very good idea, to be honest.

@zeffii
Copy link
Collaborator

zeffii commented Jul 25, 2022

@portnov maybe it should be part of sverchok-extra ? i was under the impression that the dll/so would not be included by default, but are downloaded (for the appropriate platform) as soon as the user pip installs PyOpenSubdiv..

@GeneralPancakeMSTR
Copy link
Contributor Author

i was under the impression that the dll/so would not be included by default, but are downloaded (for the appropriate platform) as soon as the user pip installs PyOpenSubdiv..

@zeffii This is correct. I changed how I am doing things so that my pyOpenSubdiv package is installed as a module, which loads the .dll/.so files into python's site-packages. Sverchok never actually directly sees the .dll/.so files.

@portnov
Copy link
Collaborator

portnov commented Jul 25, 2022

Ah, so in current solution the so/dll is not in Sverchok, but is installed as a dependency from pip?
Then I have nothing against this :)

…ers; Better handling of no inputs situation; Internal check to prevent subidivision level greater than 5; added node muting implementation; added node documentation.
@GeneralPancakeMSTR
Copy link
Contributor Author

GeneralPancakeMSTR commented Jul 25, 2022

@portnov That's right.

I've pushed changes according to the suggestions. Let me know what else I can do, or if the changes aren't quite what are being asked for.

Also here are two example setups for the documentation:

sverchok_opensubdiv_vector_test

sverchok_opensubdiv_many_bodies

nodes/modifier_change/opensubdivide.py Outdated Show resolved Hide resolved
nodes/modifier_change/opensubdivide.py Outdated Show resolved Hide resolved
nodes/modifier_change/opensubdivide.py Outdated Show resolved Hide resolved
@Durman
Copy link
Collaborator

Durman commented Jul 26, 2022

Now documentation building test fails. You can go into Files Changed tab, find the rst file and find the hint about the problem. Another, less visual, way is to go into details of the failed test.

@GeneralPancakeMSTR
Copy link
Contributor Author

@Durman I'll work on these changes. I can't seem to get pictures to show up in the opensubdivide.rst doc. Let me know if you have any suggestions.

@Durman
Copy link
Collaborator

Durman commented Jul 27, 2022

Just copy past links of the images from your previous post. No need to add them into git.

@GeneralPancakeMSTR
Copy link
Contributor Author

Just copy past links of the images from your previous post. No need to add them into git.

Hopefully I got it right, finally >_<

@portnov
Copy link
Collaborator

portnov commented Jul 31, 2022

I installed PyOpenSubdiv from preferences, restarted Blender and got

  File "/home/portnov/SSD/soft/blender-3.0.0-linux-x64/3.0/scripts/modules/addon_utils.py", line 351, in enable
    mod = __import__(module_name)
  File "/home/portnov/.config/blender/3.0/scripts/addons_contrib/sverchok/__init__.py", line 82, in <module>
    node_list = make_node_list(nodes)
  File "/home/portnov/.config/blender/3.0/scripts/addons_contrib/sverchok/core/__init__.py", line 63, in make_node_list
    import_modules(names, '{}.{}'.format(base_name, category), node_list)
  File "/home/portnov/.config/blender/3.0/scripts/addons_contrib/sverchok/core/__init__.py", line 69, in import_modules
    im = importlib.import_module('.{}'.format(m), base)
  File "/home/portnov/SSD/soft/blender-3.0.0-linux-x64/3.0/python/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/home/portnov/.config/blender/3.0/scripts/addons_contrib/sverchok/nodes/modifier_change/opensubdivide.py", line 27, in <module>
    from pyOpenSubdiv.pysubdivision import pysubdivide
  File "/home/portnov/SSD/soft/blender-3.0.0-linux-x64/3.0/python/lib/python3.9/site-packages/pyOpenSubdiv/pysubdivision.py", line 5, in <module>
    OpenSubdiv_clib = load_library.load_library()
  File "/home/portnov/SSD/soft/blender-3.0.0-linux-x64/3.0/python/lib/python3.9/site-packages/pyOpenSubdiv/clib/load_library.py", line 6, in load_library
    OpenSubdiv_clib = ctypes.CDLL(os.path.join(os.path.dirname(__file__),'ctypes_OpenSubdiv.so'))
  File "/home/portnov/SSD/soft/blender-3.0.0-linux-x64/3.0/python/lib/python3.9/ctypes/__init__.py", line 374, in __init__
    self._handle = _dlopen(self._name, mode)
OSError: libosdCPU.so.3.4.4: cannot open shared object file: No such file or directory
sv: import settings
sv: import all modules
Traceback (most recent call last):
  File "/home/portnov/SSD/soft/blender-3.0.0-linux-x64/3.0/scripts/modules/addon_utils.py", line 351, in enable
    mod = __import__(module_name)
  File "/home/portnov/.config/blender/3.0/scripts/addons_contrib/sverchok-extra/__init__.py", line 21, in <module>
    import sverchok
  File "/home/portnov/.config/blender/3.0/scripts/addons_contrib/sverchok/__init__.py", line 79, in <module>
    raise interupted_activation_detected()
NameError: 

Blender 3.0.0, debian gnu/linux. Am I required to install some library additionally?

Will try with newer blender...

@portnov
Copy link
Collaborator

portnov commented Jul 31, 2022

Nope, the same with 3.2.1

  File "/home/portnov/SSD/soft/blender-3.2.1-linux-x64/3.2/scripts/modules/addon_utils.py", line 335, in enable
    mod = __import__(module_name)
  File "/home/portnov/.config/blender/3.2/scripts/addons_contrib/sverchok/__init__.py", line 82, in <module>
    node_list = make_node_list(nodes)
  File "/home/portnov/.config/blender/3.2/scripts/addons_contrib/sverchok/core/__init__.py", line 63, in make_node_list
    import_modules(names, '{}.{}'.format(base_name, category), node_list)
  File "/home/portnov/.config/blender/3.2/scripts/addons_contrib/sverchok/core/__init__.py", line 69, in import_modules
    im = importlib.import_module('.{}'.format(m), base)
  File "/home/portnov/SSD/soft/blender-3.2.1-linux-x64/3.2/python/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/home/portnov/.config/blender/3.2/scripts/addons_contrib/sverchok/nodes/modifier_change/opensubdivide.py", line 27, in <module>
    from pyOpenSubdiv.pysubdivision import pysubdivide
  File "/home/portnov/SSD/soft/blender-3.2.1-linux-x64/3.2/python/lib/python3.10/site-packages/pyOpenSubdiv/pysubdivision.py", line 5, in <module>
    OpenSubdiv_clib = load_library.load_library()
  File "/home/portnov/SSD/soft/blender-3.2.1-linux-x64/3.2/python/lib/python3.10/site-packages/pyOpenSubdiv/clib/load_library.py", line 6, in load_library
    OpenSubdiv_clib = ctypes.CDLL(os.path.join(os.path.dirname(__file__),'ctypes_OpenSubdiv.so'))
  File "/home/portnov/SSD/soft/blender-3.2.1-linux-x64/3.2/python/lib/python3.10/ctypes/__init__.py", line 374, in __init__
    self._handle = _dlopen(self._name, mode)
OSError: libosdCPU.so.3.4.4: cannot open shared object file: No such file or directory
sv: import settings
sv: import all modules
Traceback (most recent call last):
  File "/home/portnov/SSD/soft/blender-3.2.1-linux-x64/3.2/scripts/modules/addon_utils.py", line 335, in enable
    mod = __import__(module_name)
  File "/home/portnov/.config/blender/3.2/scripts/addons_contrib/sverchok-extra/__init__.py", line 21, in <module>
    import sverchok
  File "/home/portnov/.config/blender/3.2/scripts/addons_contrib/sverchok/__init__.py", line 79, in <module>
    raise interupted_activation_detected()
NameError: 

@portnov
Copy link
Collaborator

portnov commented Jul 31, 2022

l managed to make this work by installing libosdcpu3.4.4 package from debian repos. Should this be added into node's documentation? Or maybe PyOpenSubdiv should somehow include the library or ask the user to install it?

@portnov
Copy link
Collaborator

portnov commented Jul 31, 2022

Another question I have is about node name.

  • I'm not especially against current name "OpenSubdiv", but maybe it could be more general? Something like "Catmull-Clark Subdivision"? Just for discussion...
  • And anyway, the name of the node which is stated is documentation must be exactly the same as it appears in the menu and in the node's label.

@Durman
Copy link
Collaborator

Durman commented Aug 1, 2022

I've installed the library on windows without problems.

@GeneralPancakeMSTR
Copy link
Contributor Author

I will look into why it's not working on Linux this week.

@GeneralPancakeMSTR
Copy link
Contributor Author

@portnov I have managed to recreate the error you are getting. I don't know what the solution is yet, but hopefully I can figure it out. It probably has to do with me being a novice at C++ and linking properly.

I'm open to changing the node name, maybe opensubdivision (so that it doesn't overlap with the subdivision node)? Catmull-Clark Subdivision seems long-winded to me, and most people probably won't know what it's referring to, so I'm not sure putting that in the title would be useful information.

@GeneralPancakeMSTR
Copy link
Contributor Author

@portnov Can you give it a try now? I am cautiously optimistic that I was able to fix the dependency issues on Linux.

Also I changed the node name to OpenSubdivision. I would prefer to call it Subdivision, but I think that would confuse the user trying to choose between Subdivision and Subdivide. Let me know if that name is acceptable.

@portnov
Copy link
Collaborator

portnov commented Aug 8, 2022

@GeneralPancakeMSTR well, I did apt remove libosdcpu3.4.4 and then pip install -U pyopensubdiv, and this seems to be working now!

The name is okay, I do not see better alternative for now.

@portnov
Copy link
Collaborator

portnov commented Aug 8, 2022

Docs build is failing now. As far as I understand, the problem is simple:

docs/nodes/modifier_change/opensubdivision.rst:2: WARNING: Title underline too short.

@portnov
Copy link
Collaborator

portnov commented Aug 8, 2022

And I have a question. Can some sort of masking be implemented? To do not subdivide some faces or edges...

@GeneralPancakeMSTR
Copy link
Contributor Author

I did apt remove libosdcpu3.4.4 and then pip install -U pyopensubdiv, and this seems to be working now!

Phew I'm so glad. It was a subtle compile issue.

The name is okay, I do not see better alternative for now.

Agreed. If you have other suggestions, let me know.

And I have a question. Can some sort of masking be implemented? To do not subdivide some faces or edges...

No, the node does not implement masking in the way I assume you are referring; it's pretty much all or nothing at the moment. I can look into adding masking in the future, but, from my perspective, it's a non-trivial topological problem, which I would have to really think about how to solve.

@portnov
Copy link
Collaborator

portnov commented Aug 8, 2022

Then I think this can be merged now.

@portnov portnov merged commit aa53976 into nortikin:master Aug 8, 2022
@Durman
Copy link
Collaborator

Durman commented Aug 8, 2022

@GeneralPancakeMSTR My congratulations on the first commit to Sverchok. =)

@GeneralPancakeMSTR
Copy link
Contributor Author

I'm checking out bugs in the implementation (between Sverchok and the pyOpenSubdiv module itself), and will get back to you when I think it's stable. I'm sorry for the delay, I needed a break lol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants