IViewExtension and other infrastructure changes in Dynamo to support DM extension#5314
Conversation
…n as an extension
aparajit-pratap
changed the title
There was a problem hiding this comment.
Exposing the selection changed event through ViewLoadedParams so that it can register with and handled by DMExtension. See PR: https://git.autodesk.com/Dynamo/DynamoPro/pull/178
|
@aparajit-pratap I know you're filling in the details, but there are few places where you are making public things that were not designed to be public. Can you provide in your details a high-level explanation of what you need your view extension to do, and what you need it to have access to? It is quite possible that we can implement an interface which meets your requirements without having to expose a |
There was a problem hiding this comment.
We need this additional matrix transformation here as the model up vector being used in Dynamo is flipped wrt the default up vector in Helix.
There was a problem hiding this comment.
Should we just fix the up vector thing now? I don't want to have to keep putting kludges in the code.
There was a problem hiding this comment.
Exposing Watch3DView as an IWatch3DView property in ViewLoadedParams class so that it can be passed to DMExtension.
There was a problem hiding this comment.
See my comment below regarding moving this interface to the view model instead. That way we get access to the Watch3DViewModel that we need through the DynamoViewModel which we already have :)
|
@aparajit-pratap I'll largely defer to @ikeough for this review as it touches a lot of the helix stuff, it seems. Make sure your code matches the coding standards: |
There was a problem hiding this comment.
Because these methods just call methods on the view model, I propose that instead of implementing the IWatch3DView interface on the view, you do it on the view model. So, make it IWatch3DViewModel. Make Watch3DViewModelBase : IWatch3DViewModel, and get access to the Watch3DViewModel you want to include in the LoadedParams through the DynamoViewModel.Watch3DViewModels property. DynamoViewModel is already provided as part of the loaded params.
|
@ikeough I agree with your idea of creating an |
IViewExtension and other infrastructure changes in Dynamo to support DM extension
3 participants
Purpose
This PR is to support the following tasks:
http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-8041
http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-8042
This PR makes few changes to API's in support for DM feature as an
IViewExtension. Please refer to associated PR for the Extension implementation here.A new event is introduced in
NodeModelcalledOnRequestRenderPackagesto request for additional render packages for the node when it is updated. This is called inOnRenderPackageUpdateCompleted.OnRequestRenderPackagesreturns with additional RP's if available and these are aggregated with the node's RP's before being sent toWatch3DViewModelBaseto handle for rendering.A new event
SelectionCollectionChangedis introduced inViewLoadedParamsso that it can be subscribed to by the newIViewExtensionfor node selection/de-selection events in the worskpace.Impact assessment
Added a new interface,
IWatch3DViewto expose a limited set of methods/properties to extensions like DM Extension. This interface is exposed to the externalIViewExtensionthrough theBackgroundPreViewproperty viaViewLoadedParams.HelixWatchViewModel.DeleteGeometryForIdentifier()access has been changed fromprotectedtointernalas it is called fromWatch3DViewin order to implement theIWatch3DViewinterface method for use in the extension.@ikeough is there a better way to update the
Model3DDictionaryoutside ofHelixWatchViewModel?References (FILL ME IN)
Reviewers
@ikeough Reviewer 1
@pboyer Reviewer 2
FYIs
@benglin