Loading mayaHydra should not set scene as modified. #216
Conversation
There was a problem hiding this comment.
Trivial test for modified scene.
| @@ -26,6 +26,8 @@ class TestPolygonPrimitives(mtohUtils.MayaHydraBaseTestCase): | |||
| IMAGE_DIFF_FAIL_THRESHOLD = 0.05 | |||
| IMAGE_DIFF_FAIL_PERCENT = 1.5 | |||
|
|
|||
| _requiredPlugins = ['modelingToolkit'] | |||
There was a problem hiding this comment.
This plugin was being loaded for all tests, in fixturesUtils.py, but only this test was using it.
| @@ -19,9 +19,6 @@ | |||
| import sys | |||
| import unittest | |||
|
|
|||
| # Plugins that are bundled and loaded by default in a Maya installation | |||
There was a problem hiding this comment.
ArubaTesselator plugin is unused by our tests, and modelingToolkit was only used by a single test.
| for defaultPlugin in DEFAULT_PLUGINS: | ||
| cmds.loadPlugin(defaultPlugin, quiet=True) | ||
| isModified = cmds.file(query=True, modified=True) | ||
| assert isModified == wasModified, ('Loading plugin %s modified the scene' % pluginName) |
There was a problem hiding this comment.
Assert that loading the test fixture plugin does not mark the scene as modified.
| @@ -43,61 +43,6 @@ | |||
| HD_STORM = "HdStormRendererPlugin" | |||
| HD_STORM_OVERRIDE = "mayaHydraRenderOverride_" + HD_STORM | |||
|
|
|||
| def loadPlugin(pluginName): | |||
There was a problem hiding this comment.
Removed dead code.
| @@ -35,13 +35,6 @@ | |||
| HD_STORM_OVERRIDE = "mayaHydraRenderOverride_" + HD_STORM | |||
| MAYAUSD_PLUGIN_NAME = 'mayaUsdPlugin' | |||
|
|
|||
| def checkForPlugin(pluginName: str): | |||
There was a problem hiding this comment.
Removed dead code.
| @@ -78,6 +79,44 @@ namespace { | |||
| setenv(name.c_str(), value.c_str(), 1); | |||
| #endif | |||
| } | |||
|
|
|||
| class SceneModifiedGuard | |||
There was a problem hiding this comment.
Core change in the pull request. Create an RAII guard class to set the scene as unmodified after loading the mayaHydra plugin. Plugins often add default nodes on being loaded. This conceptually does not change the modified state of the scene.
test/testUtils/mtohUtils.py
Outdated
| # We've just opened a new scene, so we should not be modified. Setting | ||
| # Storm should conceptually not change that status. | ||
| cmds.file(modified=False) |
There was a problem hiding this comment.
Should this be an assert? The comment makes it sound like this is checking that setting Storm does not change the modified state, but this just forces it to false
There was a problem hiding this comment.
It is setting it to unmodified, because the line above (setHdStormRenderer()) modifies the file. I can update the comment if you want.
There was a problem hiding this comment.
What confuses me is the contradiction between :
[...] we should not be modified. Setting Storm should conceptually not change that status
and
(setHdStormRenderer()) modifies the file
It's unclear to me whether Storm should mark the file as modified. If it should, then why do force the flag to false, and if it shouldn't, then should this not be an assert, or otherwise point to a problem where setting Storm does change the modified flag without us wanting it to do so?
No description provided.