Refactoring of BVTKNodes object handling (core and updating)

[tkeskita]

I'm starting to work on refactoring node/vtk obj update code. Two consequtive Multi Block Leaf nodes fail and there's room for clarifying code.

[tkeskita]

OK.. So this is now escalating to a full core refactoring, since updates are linked to how objects are handled in BVTKNodes. Therefore I've started to sketch an overall design spec, here's first draft:

Main targets of object handling in BVTKNodes:

• Nodes should indicate actual current status of VTK objects.
• Support typical node tree creation / editing workflows in Blender.
• Get right final result on first update run, without need to run several updates and/or node value corrections.
• General good programming practices where applicable, like user control, provide options, keep it simple etc.

Design items (note: List has been edited!):

• Node property node_id is used to link from node to it's VTK object in memory. Access is via BVTKCache. VTK object can be None for special nodes without VTK objects.
• Node provides/implements initialization, updating and freeing functions for it's VTK objects
• Node contains status:
  • none (no status information exists)
  • initialized (VTK object exists but no values / commands for it has been run yet)
  • error (setting value/running a command has failed, execution stops)
  • upstream-changed (a change has been introduced to an upstream node, may need to run update)
  • out-of-date (a change has been introduced to this node, may need to run update)
  • waiting-for-upstream (input node(s) are running an update)
  • updating (setting values / running commands for this node)
  • up-to-date (finished running commands for this node)
• Node status is shown by node (title bar) color. Colors are to be updated automatically when cursor is in BVTK node tree area. Implementation is started by just showing the status as text in node.
• Update mode for node tree to be specified by user: no automatic updates (start implementation with this), update this node automatically (to be the default option), or update whole node tree automatically.
• Updates are done by default in silent (no queue) mode. Special modal update operator to be created for visual (queue) updating, which updates node colors as update progresses.
• Allow entering values for lists (e.g. field name in Color Mapper) even before list enumerations exist (use e.g. text field). This is needed for full pre-definition of pipelines.

Comments welcome! This is gonna take some time and some iterations..

[NickGeneva]

Some ideas from my end:

Housekeeping items:

• I would suggest convergence on doc string style (personally I prefer the google napolean style but theres many). This could allow us to also have a fully fleshed out docs with docs for each method/class automatically via sphinx.
• I know some repos like to abide by a certain python style (such as python black). Not a necessity but just something to think about.
• ~Adding some status shields and maybe doing a rework of the readme may be useful/nice.~
• Add PR/Issue templates.
• Continuous integration possible? Would be nice to have a set of tests to always run commits against, but I have no idea if this is achievable since this a blender add on. I know we have a handful of examples now, would adding more to have better coverage be a good thing?
• Perhaps additional file structure changes would be good (moving everything but init.py into src folder)? Not sure.

Code refactoring:

• Perhaps the node status can be reflected in its colored title bar? I think this would be nice so you could see the status even when its collapsed.
• Regarding the cache I think there are a handful of features that should be thought bout:
  • A performance improvement would be only update the downstream vtkobjects, i.e. in a tree, only the nodes following a modified one actually need to be updated. Seems to me that there should be no need to update the reading of the file is all your changing is the color map. This is of course unless the top most vtkobjs are being modified as the tree executes. This would likely require a graphical representation of the node structure in the cache so you can do a search from any modified nodes. Not sure right now.
  • I'm not sure if node_id can be removed as of right now. I think we need some unique identifier for each node.
  • When reloading, the cache should be updated right after classes are registered. But this cant be done in the register method because the data is protected. I couldnt find a good listener to deal with this. May not be possible.
  • Thinking of the future, we should be mindful of designing for easy extensions for keyframing/baking animations etc. I have no idea how this would work. Just forward thinking.
• Adding additional support for info node may be good (debug beyond vtkAlgo objects).
• For some nodes (for example vtkDelaunay2D ) This works with two inputs but also supports a single input it seems, perhaps support for optional inputs should be added (different colored connection circle. Maybe this is just a bug with the delaunay2D node
• Perhaps having consistency checks between output/input connections would be good (e.g. a node should make sure its node before is giving a vtkAlgo object and not some PolyData) perhaps this can also be a tool tip or even color coated like blender texture nodes...

[tkeskita]

Hi Nick, thanks for your comments!

Housekeeping items:

Good ideas for down the road. Housekeeping could be improved, yes.

Automatic testing I think is very limited in Blender without GUI, may not work well for our needs.

What do you mean by status shield?

More examples (json tree files) I'm planning to add at least for the stuff now shown in docs. I'd like to have there a compact set of typical examples of common node usages, which could be used for testing as well as user examples.

A performance improvement would be only update the downstream vtkobjects, i.e. in a tree, only the nodes following a modified one actually need to be updated. Seems to me that there should be no need to update the reading of the file is all your changing is the color map. This is of course unless the top most vtkobjs are being modified as the tree executes. This would likely require a graphical representation of the node structure in the cache so you can do a search from any modified nodes. Not sure right now.

I think this can be done, if the node status feature is successfully added. Update needs to be initiated at the final output node, but it only needs to update up to the highest upstream node which is out-of-date/modified.

I'm not sure if node_id can be removed as of right now. I think we need some unique identifier for each node.

I was thinking of trying to use the objects themselves (BVTK nodes and Blender nodes) as keys for the map dicts.

Thinking of the future, we should be mindful of designing for easy extensions for keyframing/baking animations etc. I have no idea how this would work. Just forward thinking.

Me neither :-( Having a deterministic update system is a start, I hope.

Adding additional support for info node may be good (debug beyond vtkAlgo objects).

Can you give example for this what you're thinking?

For some nodes (for example vtkDelaunay2D ) This works with two inputs but also supports a single input it seems, perhaps support for optional inputs should be added (different colored connection circle. Maybe this is just a bug with the delaunay2D node

I'm not sure I follow you here, can you point me to a Python VTK example? At least vtkStreamTracer already now takes two or three inputs..?

Perhaps having consistency checks between output/input connections would be good (e.g. a node should make sure its node before is giving a vtkAlgo object and not some PolyData) perhaps this can also be a tool tip or even color coated like blender texture nodes...

Example for this as well? I'm not very well familiar with many of the VTK filters available.. But definitely nodes should stop in error if they encounter unexpected input which they can't handle.

PS. There's so much that could be improved, let's try to bite one thing at a time :-) Now I'm most irritated by the update system so I'll try to fix that.

[NickGeneva]

Thats what I figured regarding CI, thought it would be cool... but I also don't think its worth it.

What do you mean by status shield?

Read me shields! I'm sure you've seen these little buttons are repos. There great for acting as a download button, showing the liscense and also the documentation. I can add them to the readme in a PR in the near future. This is the one generated from readthedocs for this repo:

I think this can be done, if the node status feature is successfully added. Update needs to be initiated at the final output node, but it only needs to update up to the highest upstream node which is out-of-date/modified.

Agreed, will need to monitor when a nodes been modified or not and then query that. This isn't wouldn't be a huge improvement but would be a nice feature... however I could see it potentially resulting in bugs if we aren't careful.

Regarding the info node

Well right now there's a set of properties the info node provides information on objects that have points, cells, etc. Perhaps another debug node that shows information on output class, properties, etc. would be a good addition. So this could be inserted anywhere (like after color ramps for example). Would be more for debugging purposes I think.

Regarding the vtkDelaunay2D/ optional inputs.

I'll have to look more into this if I encounter it, it could just be this node... I was thinking if an input isnt required indicate it somehow but standard blender ui doesnt do such things so maybe not a good idea.

Example for this as well? I'm not very well familiar with many of the VTK filters available.. But definitely nodes should stop in error if they encounter unexpected input which they can't handle.

For example, Glyphs note should take polydata and a source object, color mapper takes poly data and the texture. So you could denote these distinct input types with different colored dots like how blender does this with vectors/scalars/colors/etc. I can look more into this.

Also no worries, everyone is busy, just spit-balling ideas from what I've seen from a couple weeks of looking at the code.

[tkeskita]

Thanks for clarifications! Those are good ideas, and good to have them now listed here.

[tkeskita]

Modified design items in 2nd post. Hoping to eliminate caches altogether and keep links to VTK objects in nodes. Planning to move actions like initialization, updating and deletion of VTK object to node functions.

[NickGeneva]

Modified design items in 2nd post. Hoping to eliminate caches altogether and keep links to VTK objects in nodes. Planning to move actions like initialization, updating and deletion of VTK object to node functions.

I tried looking to this when redoing the cache code, but couldnt figure out a good way. The requirements of node properties being a bstruct and also dealing with RNA were hard to work with. However, while working on this new node I found you can define bpy.props.PointerProperty to classes. Not sure if that can be used.

Some additional items:

• Recently I've been working on this new custom node, but it requires a custom VTK algorithm so it can get integrated into the pipeline. Do we have any place to put custom vtk algorithms? Obviously having custom vtk algos require a modification to the cache in its current state. Particularly this line, needs to be changed to check some list of custom algorithms.

• Right now I noted we have /generated_nodes, /custom_nodes, and then customfilter.py. Perhaps we should have a folder folders be /generated_vtk_nodes and /custom_vtk_nodes. Then we can add a folder /custom_bvtk_nodes where we can split up the customfilter.py file into files such as time_filters.py, util_filters.py, etc.

[tkeskita]

Thanks, I'm gonna see in a week or two if there's a problem.

Currently the special nodes are scattered around, put in places during development. Yes it's good idea to create a separate folder like special_nodes for them and reorganize them somehow (I don't have a clear picture how) at some point!

[tkeskita]

Thanks, I'm gonna see in a week or two if there's a problem.

Indeed it is a problem like you said @NickGeneva. I didn't get more luck with PointerProperty. Apparently it is just easiest to keep cache.py and use node_id there for mapping from node to VTK objects like currently.

Otherwise I'm making some progress on cleaning up the stuff related to node and VTK object initialization and updates. Seems promising so far! To be continued..

[tkeskita]

@NickGeneva about rearranging special nodes: Could that wait until I've merged #46? I'm now starting to modify special node code..

[NickGeneva]

@tkeskita No worries, I've added my new node into current file. See #54. Had to add some new items with the cache for this to work properly. Not sure if this causes you issues, if so let me know and we can work out merging.

Also yeah, the Blender RNA is a big pain and frustrating, I needed to make a global variable for this new node to function as well. Maybe we can generalize the cache so it can store more than just vtk objects... idk. But I think the local status and other items that can be stored in the node (through the built in props) is a major improvement.

Working on this new node gave me some additional naming ideas and implementation ideas for special nodes. But we can wait until merge.

[tkeskita]

I've now updated PR #46 so that vtkConeSource and Info nodes work as they should upon placement to node tree and updates. If you have time, please check the new core functionality and comment!

[tkeskita]

PR #46 is now merged to master! Half a year of work, hope you like it!