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

Removed old viewport draw in favour or VP2 GeometryOverride with Parallel Eval and Viewport Caching compatibility #26

Merged
merged 4 commits into from
Dec 4, 2023

Conversation

mdilena
Copy link
Contributor

@mdilena mdilena commented Dec 2, 2023

Checklist

  • I have read the CONTRIBUTING.md document
  • I formatted my changes with [clang-format (Google Style)]
  • I have added documentation regarding my changes where necessary

Types of Changes

  • Bugfix (change that fixes an issue)
  • New Feature (change that adds functionality)
  • Documentation Update (if none of the other choices apply)

Proposed Changes

This is something that has been on my mind since we adopted TwistSpline at work, but never had time to properly sit there and reading some docs on how to solve it, until last week.
The issue first came up when the animators on a project using the plugin were dealing with flipping geometry and needed a visual aid, but the spline debug axis weren't working interactively. I later discovered the issue was being triggered when Parallel Evaluation came into play, as I started noticing it on the rigging side when we started adding controller tags to rig controls by default (that also triggers parallel eval, other than animation keys).

This pull request aims at fixing that behaviour by replacing the legacy viewport draw code with the new GeometryOverride methods. I am by no means a c++ expert and it took me some time to understand what was doing what in there, but I think I sort of got it now and the comments from the footPrintNode_geometryOverride example from Autodesk (which I adapted for this plugin and left in there) helped getting this working.

Due to the amount of new code introduced I have split these new changes into two new modules: drawOverride.h and drawOverride.cpp. So all the code for the viewport logic has been implemented there, with the exception of some requirements that needed to go into the TwistSplineNode class.

I uploaded a couple of videos on Youtube to show the difference between the current behaviour and the new one. I should also mention that I ran the Profiler with and without Viewport Caching and the results seem as intended, with Viewport Caching cutting down at least half of the time of computation (on my machine, it went from 800-900 us to 300-400 us when caching was on).
Most importantly, the drawing is now interactive when manipulating a rig with Parallel Eval and Viewport Caching, and also during playback.

Current viewport implementation: https://youtu.be/uez-osG62bg
New viewport implementation from this pull request: https://youtu.be/-tBCMP8yGtU

Added a missing .setClean() call to the 'geometryChanging' output and conformed the MStatus error check to the rest of the plugs in compute(). Also removed a useless update call to the splineLength that was in no way meangingful for viewport drawing or caching.
@tbttfox
Copy link
Member

tbttfox commented Dec 4, 2023

This is something that has been on my mind since we adopted TwistSpline at work, but never had time to properly sit there and reading some docs on how to solve it, until last week.

Preaching to the choir!
And LGTM. I've spent quite a bit of time in that footprint example myself :)
Thanks again

@tbttfox tbttfox merged commit 4b0e19a into blurstudio:master Dec 4, 2023
12 checks passed
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.

2 participants