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

EMSUSD-248 fix lost notification listeners #3202

Merged
merged 5 commits into from
Jul 12, 2023

Conversation

pierrebai-adsk
Copy link
Collaborator

@pierrebai-adsk pierrebai-adsk commented Jul 6, 2023

The order of operations when loading a Maya scene depends on the relative order of unrelated events. Some things are triggered by Maya callbacks, some are done when a stage is set on a proxy node, which gets set when the proxy node is computed, which depends on when it gets dirtied and when something accesses it. Furthermore, some other things are done in the on-idle callback of Maya, which depends on when Qt events are processed. On top of all this, some things are triggered by timers.

For example:

  • notification listeners are cleared in Maya callbacks.
  • notification listeners are registered when the proxy node is computed
  • The layer manager initialize itself with a timer.
  • The layer manager is cleared by a Qt signal being emitted, which itself is triggered by a Maya callback.
  • The Maya session state stage is reset in an on-idle callback.

The problem in our case is that the notification listeners were getting wiped out because they were cleared in a Maya after-new / after-open callback, but now that is getting called after the proxy shape has been computed, which is where the notification listeners for the new stage were registered.

IOW, the new listeners were cleared by a function that was meant to clear the old listeners. The fix is to clear the listeners in a before-new / before-open callback instead.

  • Rename beforeNewCallback() to isInNewScene().
  • Rename beforeNewCallback(bool) to setInNewScene(bool)
  • Those were confusing because they had the same name as the real Maya callback, but with different parameters.
  • Move the code that cleared listeners from afterOpen to beforeOpen.
  • Added setupListener() and clearListener() to make the code clearer.
  • Fixed a bug that left the in-new-scene flag to true when opening a Maya scene.

The order of operations when loading a Maya scene depends on the relative order of unrelated events. Some things are triggered by Maya callbacks, some are done when a stage is set on a proxy node, which gets set when the proxy node is computed, which depends on when it gets dirtied and when something accesses it. Furthermore, some other things are done in the on-idle callback of maya, which depends on when Qt events are processed. On top of all this, some things are triggered by timers.

For example:
- notification listeners are cleared in Maya callbacks.
- notification listeners are registered when the proxy node is computed
- The layer manager initialize itself with a timer.
- The layer manager is cleared by a Qt signal being emitted, which itself is triggered by a Maya callback.
- The Maya session state stage is reset in an on-idle callback.

The problem in our case is that the notification listeners were getting wiped out because they were cleared in a Maya after-new / after-open callback, but now that is getting called after the proxy shape has been computed, which is where the notification listeners for the new stage were registered.

IOW, they new listeners were cleared by a function that was meant to clear the old listeners. The fix is to clear the listerener in a before-new / before-open callback instead.

- Rename beforeNewCallback() to isInNewScene().
- Rename beforeNewCallback(bool) to setInNewScene(bool)
_ Those were confusing because they had the same name as the real Maya callback, but with different parameters.
- Move the code that cleared listeners from afterOpen to beforeOpen.
- Added setupListener() and clearListener() to make the code clearer.
- Fixed abug that left the in-new-scene flag to true when opening a Maya scene.
@pierrebai-adsk pierrebai-adsk added bug Something isn't working adsk Related to Autodesk plugin labels Jul 6, 2023
@pierrebai-adsk
Copy link
Collaborator Author

Only the known flaky testVP2RenderDelegateDisplayLayers test failed.

@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Jul 10, 2023
@seando-adsk seando-adsk added do-not-merge-yet Development is not finished, PR not ready for merge and removed ready-for-merge Development process is finished, PR is ready for merge labels Jul 11, 2023
@pierrebai-adsk pierrebai-adsk removed the do-not-merge-yet Development is not finished, PR not ready for merge label Jul 11, 2023
@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Jul 11, 2023
Copy link
Collaborator

@seando-adsk seando-adsk left a comment

Choose a reason for hiding this comment

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

Thanks for the detailed description and investigation of the problem/solution.

@seando-adsk seando-adsk merged commit 26a15c3 into dev Jul 12, 2023
@seando-adsk seando-adsk deleted the bailp/EMSUSD-248/lost-notification-listeners branch July 12, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adsk Related to Autodesk plugin bug Something isn't working ready-for-merge Development process is finished, PR is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants