david-cattermole / mayaMatchMoveSolver

A Bundle Adjustment solver for MatchMove related tasks.
https://david-cattermole.github.io/mayaMatchMoveSolver/
Other
102 stars 27 forks source link

SolverAffects Pre-Solve Fails on Container-based Rigs #183

Closed david-cattermole closed 3 years ago

david-cattermole commented 3 years ago

Problem

I have a single Bundle, a child of the wrist rivet. I am solving for Wrist IK tx/ty. The status of the Marker and the wrist tx/ty in the mmSolver Solver GUI is showing as Invalid

The rig is using Maya Containers.

Here is the error message.

# Traceback (most recent call last):
#   File "<maya console>", line 1, in <module>
#   File "C:\mayaMatchMoveSolver\0.3.10\python\mmSolver\utils\nodeaffects.py", line 316, in find_plugs_affecting_transform
#     plugs = _get_attribute_plugs(nodes)
#   File "C:\mayaMatchMoveSolver\0.3.10\python\mmSolver\utils\nodeaffects.py", line 252, in _get_attribute_plugs
#     type_cache=type_cache)
#   File "C:\mayaMatchMoveSolver\0.3.10\python\mmSolver\utils\nodeaffects.py", line 201, in _convert_node_to_plugs
#     settable = maya.cmds.getAttr(node_attr, settable=True)
# ValueError: No object matches name: myCharacter_1:scBaryConstraint_mesh_m_high_follicleFACSDriverFirstLevel_001.outputTranslateX // 
// Error: No object matches name: myCharacter_1:scBaryConstraint_mesh_m_high_follicleFACSDriverFirstLevel_001.outputTranslateX

// Warning: mmSolver._api.execute : Invalid parameters and errors, skipping solve: [1001] // 
// mmSolver.tools.solver.lib.collection : Total Time: 15.478 seconds // 
// mmSolver.tools.solver.lib.collection : Max Frame Deviation: -0.00 pixels at frame None // 
// mmSolver.tools.solver.lib.collection : Average Deviation: 0.00 pixels // 
// Warning: mmSolver.tools.solver.lib.collection : 2020-12-15 04:45:47 | Solved | Average Deviation 0.00px | Max Deviation -0.00px at None | Time 15.478sec //

Software Versions

david-cattermole commented 3 years ago

scBaryConstraint_mesh_m_high_follicleFACSDriverFirstLevel_001 could be a follicle node, or similar to a follicle node.

I suspect this problem is probably caused by the reduction of the full dot-separated attribute path. node.outputTranslateX could actually be node.myAttribute[0].outputTranslateX, but the nodeaffects module is actually removing the myAttribute[0] part.

I need to check and test the assumptions of a single dot in the node names, and see if we don't need to expand/shorten the full attribute name path.

david-cattermole commented 3 years ago

Here is another example of a similar bug with a different rig:

# Traceback (most recent call last):
#   File "<maya console>", line 1, in <module>
#   File "C:\mayaMatchMoveSolver\0.3.10\python\mmSolver\utils\nodeaffects.py", line 316, in find_plugs_affecting_transform
#     plugs = _get_attribute_plugs(nodes)
#   File "C:\mayaMatchMoveSolver\0.3.10\python\mmSolver\utils\nodeaffects.py", line 252, in _get_attribute_plugs
#     type_cache=type_cache)
#   File "C:\mayaMatchMoveSolver\0.3.10\python\mmSolver\utils\nodeaffects.py", line 200, in _convert_node_to_plugs
#     node_attr = _get_full_path_plug(node_attr)
#   File "C:\mayaMatchMoveSolver\0.3.10\python\mmSolver\utils\nodeaffects.py", line 102, in _get_full_path_plug
#     attr = maya.cmds.attributeName(plug, long=True)
# RuntimeError: Target objects must be specified in "node.attribute" format //
// Warning: mmSolver._api.execute : Invalid parameters and errors, skipping solve: [1001] //
// mmSolver.tools.solver.lib.collection : Total Time: 6.968 seconds //
// mmSolver.tools.solver.lib.collection : Max Frame Deviation: -0.00 pixels at frame None //
// mmSolver.tools.solver.lib.collection : Average Deviation: 0.00 pixels //
// Warning: mmSolver.tools.solver.lib.collection : 2020-12-17 11:32:08 | Solved | Average Deviation 0.00px | Max Deviation -0.00px at None | Time 6.968sec // 

The error message is slightly different, but is failing in almost the exact same place.

ktonegawa commented 3 years ago

Hi David,

I tested a couple of rigs I have and I am also got a similar error to ONE rig:

import mmSolver.tools.solver.tool;
mmSolver.tools.solver.tool.open_window();
// Error: Target objects must be specified in "node.attribute" format
# Traceback (most recent call last):
#   File "<maya console>", line 1, in <module>
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 316, in find_plugs_affecting_transform
#     plugs = _get_attribute_plugs(nodes)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 252, in _get_attribute_plugs
#     type_cache=type_cache)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 200, in _convert_node_to_plugs
#     node_attr = _get_full_path_plug(node_attr)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 102, in _get_full_path_plug
#     attr = maya.cmds.attributeName(plug, long=True)
# RuntimeError: Target objects must be specified in "node.attribute" format // 
// Error: Target objects must be specified in "node.attribute" format
# Traceback (most recent call last):
#   File "<maya console>", line 1, in <module>
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 316, in find_plugs_affecting_transform
#     plugs = _get_attribute_plugs(nodes)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 252, in _get_attribute_plugs
#     type_cache=type_cache)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 200, in _convert_node_to_plugs
#     node_attr = _get_full_path_plug(node_attr)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 102, in _get_full_path_plug
#     attr = maya.cmds.attributeName(plug, long=True)
# RuntimeError: Target objects must be specified in "node.attribute" format // 
// Warning: mmSolver._api.execute : Invalid parameters and errors, skipping solve: [1] // 
// mmSolver.tools.solver.lib.collection : Total Time: 1.938 seconds // 
// mmSolver.tools.solver.lib.collection : Max Frame Deviation: -0.00 pixels at frame None // 
// mmSolver.tools.solver.lib.collection : Average Deviation: 0.00 pixels // 
// Warning: mmSolver.tools.solver.lib.collection : 2020-12-26 19:11:19 | Solved | Average Deviation 0.00px | Max Deviation -0.00px at None | Time 1.938sec // 

Im also getting the "invalid" icon in the GUI image

this seems to only occur on Connected attributes...

david-cattermole commented 3 years ago

Thanks @ktonegawa,

Would it be possible to share the rig in question that triggers the error? Could you share it privately, if needed? Is the rig Mr. SuRuj 0.0.1 for Maya?

I believe there are two different errors, but they may be related.

Error 1 might be caused by nodes that have an array attribute:

#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 200, in _convert_node_to_plugs
#     node_attr = _get_full_path_plug(node_attr)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.10-maya2019-win64\python\mmSolver\utils\nodeaffects.py", line 102, in _get_full_path_plug
#     attr = maya.cmds.attributeName(plug, long=True)
# RuntimeError: Target objects must be specified in "node.attribute" format // 

Error 2 might be caused when using containers:

#   File "C:\mayaMatchMoveSolver\0.3.10\python\mmSolver\utils\nodeaffects.py", line 201, in _convert_node_to_plugs
#     settable = maya.cmds.getAttr(node_attr, settable=True)
# ValueError: No object matches name: myCharacter_1:scBaryConstraint_mesh_m_high_follicleFACSDriverFirstLevel_001.outputTranslateX // 

I'll dig further...

David

david-cattermole commented 3 years ago

I can also reproduce error 1 using the "Mr. SuRuj 0.0.1 for Maya" rig.

ktonegawa commented 3 years ago

Hi @david-cattermole , yes that is exactly the rig I used to produce that error.

david-cattermole commented 3 years ago

After having a closer look with @ktonegawa at "Error 1" it appears that maya.cmds.listConnections does not return the parent attribute, and only returns the leaf attribute name.

For example, for a parentConstraint node the target[0].targetParentMatrix will be returned from listConnections as nodeName.targetParentMatrix removing the target[0] and making the full path to the attribute invalid.

This appears to be a limitation of Maya's command and we'll need to workaround this some way.

@ktonegawa is going to look at this further.

David

david-cattermole commented 3 years ago

It is possible that we'll need to use the Maya Python API (v2.0) to replace the need for maya.cmds.listConnections in this function. That could improve the speed, and also guarantee correctness.

For example, this function from the cmdx project gets the connections like this and uses maya.api.OpenMaya.MPlug(): https://github.com/mottosso/cmdx/blob/f84b3c503b888a92242d0310265429209713de53/cmdx.py#L2987

ktonegawa commented 3 years ago

So i have been looking into this the entire day today and my head just hurts. But here are my findings so far.

As @david-cattermole mentioned, some of the issues stemmed from a "dot reduction" path, and more specifically I realized it was due to the fact that maya.cmds.affects() was not picking up compound attributes properly, plus the added fact that maya.cmds.attributeName() converts attribute names from say targetTranslateOffset to Target Translate Offset, it caused an error of Maya trying to find this attribute.

To get around this, I so far came up with this solution, which probably is not that elegant, but at least now in this particular scene I am using for testing the second reported error no longer pops up: https://github.com/ktonegawa/mayaMatchMoveSolver/commit/fb300cf678639908ec4e0e25a24e7e8e292a4d08#diff-de0658a556f4949d497f61f71f915e4b98ba36167e48556b6958325cdb1c0e35R253-R262

Now the next issue I am seeing, is when attempting to solve for the current frame, I interestingly get two different errors from when using runTests.py and when manually testing in Maya GUI:

Maya GUI:

# Traceback (most recent call last):
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\tools\solver\ui\solver_window.py", line 662, in apply
#     self)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\tools\solver\lib\collection.py", line 793, in run_solve_ui
#     info_fn=info_fn,
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\tools\solver\lib\collection.py", line 640, in execute_collection
#     info_fn=info_fn,
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\_api\execute.py", line 788, in execute
#     save_node_attrs = collectionutils.disconnect_animcurves(kwargs)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\_api\collectionutils.py", line 131, in disconnect_animcurves
#     connObj = connPlug.node()
#   File "S:\Maya_2019_DI\build\Release\runTime\Python\Lib\site-packages\maya\OpenMaya.py", line 16234, in node
# RuntimeError: (kFailure): Object does not exist

runTests.py:

Warning: Plug-in, "mmSolver", is not loaded.

--------------------------------------------------------------------------------
Name: test.test_utils.test_nodeaffects.TestNodeAffects.test_plug_attr_not_found_error
Warning: file: C:/Users/Desktop02/Documents/maya/projects/default/scenes/mayaMatchmoveSolver_solver_test7_kazMod5_issue1
83_example01.ma line 3663: Unrecognized node type 'vectorRenderGlobals'; preserving node information during this session
.
Warning: Errors have occurred while reading this scene that may result in data loss.
Read 2 files in  1.8 seconds.
mmSolver.tools.solver.tool : WARNING : Could not get window.
mmSolver.tools.solver.lib.collection : WARNING : 0
mmSolver.tools.solver.lib.collection : WARNING : Solve start (2021-01-09 22:45:13)
mmSolver.tools.solver.lib.collection : WARNING : 1
mmSolver.tools.solver.lib.collection : WARNING : 0
mmSolver.tools.solver.lib.collection : WARNING : 33
mmSolver.tools.solver.lib.collection : WARNING : Evaluating frames [1]
Solving... frames: 1
Iteration 0001 | Eval 0001ERROR: Dynamic attributes that aren't animated cannot be set; name=|SuRuJ:rig_Grp|SuRuJ:root_G
rp|SuRuJ:ctrlCurves_Grp|SuRuJ:leftHandIKCtrl_Grp|SuRuJ:left_Hand_IK_Ctrl.translateY plug=SuRuJ:left_Hand_IK_Ctrl.transla
teY
ERROR: Dynamic attributes that aren't animated cannot be set; name=|SuRuJ:rig_Grp|SuRuJ:root_Grp|SuRuJ:ctrlCurves_Grp|Su
RuJ:leftHandIKCtrl_Grp|SuRuJ:left_Hand_IK_Ctrl.translateX plug=SuRuJ:left_Hand_IK_Ctrl.translateX
 | error avg  88.4488   min  88.4488   max  88.4488
ERROR: Dynamic attributes that aren't animated cannot be set; name=|SuRuJ:rig_Grp|SuRuJ:root_Grp|SuRuJ:ctrlCurves_Grp|Su
RuJ:leftHandIKCtrl_Grp|SuRuJ:left_Hand_IK_Ctrl.translateY plug=SuRuJ:left_Hand_IK_Ctrl.translateY
ERROR: Dynamic attributes that aren't animated cannot be set; name=|SuRuJ:rig_Grp|SuRuJ:root_Grp|SuRuJ:ctrlCurves_Grp|Su
RuJ:leftHandIKCtrl_Grp|SuRuJ:left_Hand_IK_Ctrl.translateX plug=SuRuJ:left_Hand_IK_Ctrl.translateX
ERROR: Dynamic attributes that aren't animated cannot be set; name=|SuRuJ:rig_Grp|SuRuJ:root_Grp|SuRuJ:ctrlCurves_Grp|Su
RuJ:leftHandIKCtrl_Grp|SuRuJ:left_Hand_IK_Ctrl.translateY plug=SuRuJ:left_Hand_IK_Ctrl.translateY
ERROR: Dynamic attributes that aren't animated cannot be set; name=|SuRuJ:rig_Grp|SuRuJ:root_Grp|SuRuJ:ctrlCurves_Grp|Su
RuJ:leftHandIKCtrl_Grp|SuRuJ:left_Hand_IK_Ctrl.translateX plug=SuRuJ:left_Hand_IK_Ctrl.translateX
ERROR: Dynamic attributes that aren't animated cannot be set; name=|SuRuJ:rig_Grp|SuRuJ:root_Grp|SuRuJ:ctrlCurves_Grp|Su
RuJ:leftHandIKCtrl_Grp|SuRuJ:left_Hand_IK_Ctrl.translateY plug=SuRuJ:left_Hand_IK_Ctrl.translateY
ERROR: Dynamic attributes that aren't animated cannot be set; name=|SuRuJ:rig_Grp|SuRuJ:root_Grp|SuRuJ:ctrlCurves_Grp|Su
RuJ:leftHandIKCtrl_Grp|SuRuJ:left_Hand_IK_Ctrl.translateX plug=SuRuJ:left_Hand_IK_Ctrl.translateX
Solver returned SUCCESS    | error avg  88.4488   min  88.4488   max  88.4488  iterations 001
mmSolver.tools.solver.lib.collection : WARNING : 66
mmSolver.tools.solver.lib.collection : WARNING : Solve Ended
mmSolver.tools.solver.lib.collection : WARNING : 100
mmSolver.tools.solver.lib.collection : INFO : Total Time: 2.288 seconds
mmSolver.tools.solver.lib.collection : INFO : Max Frame Deviation: 88.45 pixels at frame 1
mmSolver.tools.solver.lib.collection : INFO : Average Deviation: 88.45 pixels
mmSolver.tools.solver.lib.collection : WARNING : 2021-01-09 22:45:15 | Solved | avg deviation 88.45px | max deviation 88
.45px at 1 | time 2.288sec
mmSolver.tools.solver.lib.collection : WARNING : 2021-01-09 22:45:15 | Solved | Average Deviation 88.45px | Max Deviatio
n 88.45px at 1 | Time 2.288sec
Time:   4.628000
Python Profiler: C:\Users\Desktop02\Documents\git_remote_repos\mayaMatchMoveSolver\tests\profile\test_test_utils_test_no
deaffects_TestNodeAffects_test_plug_attr_not_found_error.pstat
.
----------------------------------------------------------------------
Ran 1 test in 4.686s

OK

From this though, I suspect what is happening is due to the solver's native method of trying to initially disconnect the animation curve from the desired node to perform the solve, it fails to really deduce where the animation curve is. In this specific rig, the SuRuj:left_Hand_IK_Ctrl is being driven by a separate node's (which in this case is apparently a "Character Node") custom attribute, which then is connected to an animCurve node. So really its going like this: SuRuj:SuRuj>SuRuJ_left_Hand_IK_Ctrl_translateX>SuRuj:left_Hand_IK_Ctrl

And when trying to add any of these custom attributes from this "Character Node" into the solver we get this error:

# Traceback (most recent call last):
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\tools\solver\widget\attribute_widget.py", line 331, in addClicked
#     lib_attr.add_attributes_to_collection(attr_list, col)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\tools\solver\lib\attr.py", line 57, in add_attributes_to_collection
#     return col.add_attribute_list(attr_list)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\_api\collection.py", line 627, in add_attribute_list
#     self._set.add_members(name_list)
#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\_api\sethelper.py", line 121, in add_members
#     maya.cmds.sets(*name_list, edit=True, include=set_node, noWarnings=True)
# RuntimeError: Error while parsing arguments.
# 

I haven't quite looked into this yet, but I was thinking then maybe if we can before the solve process actually kicks in if a feature can be implemented to detect if an attribute is driven, then traversing back a few connections and see if the connected node has some attributes that can be used to still utilize within the solve.

I saw that the main part where this detection of whether an attribute was either animated or connected was done via src/Attr.cpp but I figure the way the attributes in this have been set up there isn't really anywhere that stores any connected node info.

In the end, I don't really know if any of this gets closer to answering the original issue at hand, or how to move forward with all of this information.

Feedback would be appreciated...

david-cattermole commented 3 years ago

Hello @ktonegawa,

It's not a simple problem and I'm a little embarrassed about the complexity of the nodeaffects.py file.

I think you've gotten to the bottom of a lot of the problems, and I have commented on the commit you've made: https://github.com/ktonegawa/mayaMatchMoveSolver/commit/fb300cf678639908ec4e0e25a24e7e8e292a4d08#

Firstly, this traceback really confuses me, I don't understand how that could happen unless the node has been deleted - no where in that function are we deleting any nodes, it will only disconnect attributes:

#   File "C:\Users\Desktop02\Documents\maya\2019\modules\mayaMatchMoveSolver-0.3.11-maya2019-win64\python\mmSolver\_api\collectionutils.py", line 131, in disconnect_animcurves
#     connObj = connPlug.node()

The second error (ERROR: Dynamic attributes that aren't animated cannot be set;) is almost definitely caused by the Character Set (Node) that is used by the rig. Personally, I have never seen Character Sets used in production rigs before. The Character Sets feature is used with the Trax Editor in Maya for animation clips. Have you ever worked with a rig using Character Sets in production?

I haven't quite looked into this yet, but I was thinking then maybe if we can before the solve process actually kicks in if a feature can be implemented to detect if an attribute is driven, then traversing back a few connections and see if the connected node has some attributes that can be used to still utilize within the solve.

The first thing you should do is simply delete the Character Set from the rig, up-version the rig and try to solve again with the new rig. If this works, I am happy to say "Character Sets are not supported, sorry". I do not think we should worry about supporting an unneeded or uncommon use-case, especially when the solution sounds to be quite complicated. I will confirm with the original author of this bug report to see if they need/want Character Sets to be supported (I suspect not).

If Character Sets do need to be supported... I think we can handle it in the Solver UI, so the solver doesn't need to know about the special use-case. When a user clicks "+" (Add Attribute) into the Output Attributes widget we can detect the Character Set and find the real attribute (on the Character Set Node) that will actually be solved. If it's really needed, we can push this further and ensure the UI seamlessly displays the attribute on the control, but then solves the Character Set Node attribute instead. This workaround is all very complicated and I would simply prefer to not support Character Sets.

I saw that the main part where this detection of whether an attribute was either animated or connected was done via src/Attr.cpp but I figure the way the attributes in this have been set up there isn't really anywhere that stores any connected node info.

You are correct, the attribute state is looked up as needed, and the error is triggered from that ./src/Attr.cpp file, however for the Python API, you can use mmSolver.api.Attribute.get_state(). https://github.com/ktonegawa/mayaMatchMoveSolver/blob/f4e58de6b7ca7be50d2dbf1629483791ca0f0877/python/mmSolver/_api/attribute.py#L166-L198

Does this help? I will do some tests of my own later today.

David

ktonegawa commented 3 years ago

Hey @david-cattermole, as you suspected, deleting the Character Set node from the original and testing with that seems to allow for the solver to function properly: https://github.com/ktonegawa/mayaMatchMoveSolver/commit/2d0bbb88391dd2d20bebc527e855b6822b467e8f

Clearly my initial idea of using maya.cmds.attributeQuery() did not work and that your implemented method is far superior ガ━━(゚Д゚;)━━━ン!!!!!

And to answer your question, no I have never seen Character Set nodes being used in at least Matchmove/Roto-Anim rigs, but I've also never seen Container nodes be used either.

I heard from a Rigging friend that Container nodes are not really a great way to manage assets anyways, and that there are "better ways of doing it" (not sure what they are but irrelevant to this conversation). I guess it means every studio probably has some kind legacy workflow/procedure that is so embedded with their pipeline its hard to adjust.

If we wanted to take the route of not really bothering with coming up with a solution for this since like you say it is indeed complicated, perhaps adding a LOG message or some kind of pop that warns the user that the solve has failed due to related circumstances, and instead off some other suggesting such as utilizing this plugin's Tools>Create Controller feature and solving for that controller instead (sounds silly but it does work).

By the way, I noticed a super annoying bug while testing, and seems like the solver does not run properly if there are no keys set on the desired output? Is this intentional? I think it would be really good to have it so that it can work for both Static and Animated attributes...

# ValueError: No object matches name: myCharacter_1:scBaryConstraint_mesh_m_high_follicleFACSDriverFirstLevel_001.outputTranslateX // 
// Error: No object matches name: myCharacter_1:scBaryConstraint_mesh_m_high_follicleFACSDriverFirstLevel_001.outputTranslateX

Regarding this error, it would be great if the reported user can test with latest commit and submit any new changes in traceback error output.

david-cattermole commented 3 years ago

Hello @ktonegawa,

Your commit https://github.com/ktonegawa/mayaMatchMoveSolver/commit/2d0bbb88391dd2d20bebc527e855b6822b467e8f looks good to me. It would be good to remove the unneeded commented lines before a final PR, otherwise, it's great!

Clearly my initial idea of using maya.cmds.attributeQuery() did not work and that your implemented method is far superior

FYI, the function mmSolver.utils.node.attribute_exists() is a port of the MEL command attributeExists into Python, nothing more - it's very helpful, and tests a number of different cases (it uses attributeQuery, and other functions I think).

And to answer your question, no I have never seen Character Set nodes being used in at least Matchmove/Roto-Anim rigs, but I've also never seen Container nodes be used either.

Agreed. Unless I hear otherwise, I think we should leave Character Sets "unsupported".

perhaps adding a LOG message or some kind of pop that warns the user that the solve has failed due to related circumstances, and instead off some other suggesting such as utilizing this plugin's Tools>Create Controller feature and solving for that controller instead (sounds silly but it does work).

We can perhaps detect Character Sets and print an error to the user. That's not a bad idea, although I do think we shouldn't waste much time on it since I'm pretty sure Character Sets are unlikely to be used. If that's the case, I have documented it as a separate issue; #198.

By the way, I noticed a super annoying bug while testing, and seems like the solver does not run properly if there are no keys set on the desired output? Is this intentional?

Do you mean that Animated attributes need to have keyframes? If so, yes it is intentional. The user needs to key the attributes they wish to solve as animated.

I think it would be really good to have it so that it can work for both Static and Animated attributes...

I'm not sure I understand. To solve Animated and Static Attributes, you'll need to use the "Standard" solver tab. I would hope that the "Standard" solver tab will also solve only Static attributes. Is this not the case?

ktonegawa commented 3 years ago

We can perhaps detect Character Sets and print an error to the user. That's not a bad idea, although I do think we shouldn't waste much time on it since I'm pretty sure Character Sets are unlikely to be used. If that's the case, I have documented it as a separate issue; https://github.com/david-cattermole/mayaMatchMoveSolver/issues/198

I don't think its entirely necessary to be creating an error message specifically for Character Sets, but a general one for attributes that may be driven or just attributes that this solver cannot necessarily find the appropriate attributes to solve for.

I'm not sure I understand. To solve Animated and Static Attributes, you'll need to use the "Standard" solver tab. I would hope that the "Standard" solver tab will also solve only Static attributes. Is this not the case?

Well when I tested to just solve the current frame for the IK hand controller when no keys were set, it "solved" without any errors, but the resulting solve had a max deviation of 0.0 which clearly means it didn't actually do anything (and the controller in question had no changes), but as soon as I added a key and solved, it solved correctly. So what I was asking was if it is intentional to have "Static Attributes" to not solve?

Buuut, I just did a quick test on just a really simple object and indeed it seems like you should be able to solve for static attributes as well, which probably means this is some kind of bug. I can take a look into it later in the future if you'd like...

david-cattermole commented 3 years ago

I don't think its entirely necessary to be creating an error message specifically for Character Sets, but a general one for attributes that may be driven or just attributes that this solver cannot necessarily find the appropriate attributes to solve for.

Hmm, I see your point. I'll edit the issue #198 and see if I can summarise the idea properly. Other than rigs with Character Sets I'm not sure I an think of a good, common case and how it should work (at least right now).

So what I was asking was if it is intentional to have "Static Attributes" to not solve?

When using the Standard Solver, both Animated and Static attributes should be solved, even on single frame solves. When using the Basic Solver, only Animated attributes should be solved, even on single frames.

Basic Solver: https://github.com/david-cattermole/mayaMatchMoveSolver/blob/01dc00cdcb0205c474429e08f158e85fdd89f6be/python/mmSolver/_api/solverbasic.py#L303-L304

Standard Solver: https://github.com/david-cattermole/mayaMatchMoveSolver/blob/01dc00cdcb0205c474429e08f158e85fdd89f6be/python/mmSolver/_api/solverstandard.py#L769-L770

If for some reason that's not correct it's a bug and should be logged and fixed.

david-cattermole commented 3 years ago

@ktonegawa the original reporting user is in the process of testing to confirm your fix is working.

I think we can consider the original bug(s) are (probably) fixed - none of the changes you've made make me think anything new would break - the code is just handles more use-cases that I didn't think about.

Would it be possible to remove the excess commented code and we can merge this into the develop_v0.3.x branch (ready for release in v0.3.12)?

ktonegawa commented 3 years ago

@ktonegawa the original reporting user is in the process of testing to confirm your fix is working.

I think we can consider the original bug(s) are (probably) fixed - none of the changes you've made make me think anything new would break - the code is just handles more use-cases that I didn't think about.

Would it be possible to remove the excess commented code and we can merge this into the develop_v0.3.x branch (ready for release in v0.3.12)?

Great to hear, hope it works out on the reporting user's end too.

And of course once we get that confirmed and sorted I will do a proper PR with the commented lines removed :)

david-cattermole commented 3 years ago

Hello @ktonegawa,

I think there is some confusion for the reporting user about installing the fix.

This weekend I will merge your tools-issue183 branch into a beta testing branch and release a "pre-release" beta version. I expect this will help the debugging process.

Otherwise I might just release a proper version, as I don't think your fixes have any negative side affects.

David

david-cattermole commented 3 years ago

This bug fix will be included in v0.3.12, but I will keep this issue open until I get confirmation that the problem is fixed.

I suspect the issue will not present itself anyway by default since I have added a new checkbox "Evaluate Object Relationships" (#187), which is disabled by default.

ktonegawa commented 3 years ago

Hi @david-cattermole, sure, why don't I create a PR that which has all of the commented lines and logs cleaned up so it is ready for you to pull.

david-cattermole commented 3 years ago

Thanks @ktonegawa, but it's ok, I have pulled your changes into the latest release v0.3.12 :D

The only thing left is to get confirmation from the reporting user that the change fixes the problem.

That being said, it was requested that a "pre-solve" checkbox be added #187 to allow disabling of the "affects" code, so that the solver will start faster. I added the check-box (named Evaluate Object Relationships), and by default it's disabled. One of the reasons for the check-box was to offer a similar tool to the "Legacy" tab, which will (likely) be removed in v0.4.x.

It's very unfortunate, as I wanted the feature to be turned on by default without the need to turn it off - ever. I still have hopes of re-writing the entire module and converting it into C++ or some other code that can evaluate faster, but I am taking a pragmatic approach, because I'm unsure when I'll be do that, or even if it's possible.

David

david-cattermole commented 3 years ago

@ktonegawa I have confirmation that your bug fix works perfectly in v0.3.12 with the custom rigs (that happen to use Containers).

In summary the bug was caused by my code: 1) Incorrectly assuming attribute names always exist. 2) Assuming the listConnections function returns full path attributes for compounds. 3) Not handling compound attributes.

I'm closing this as it was confirmed to be fixed!