Vtk-js: Widgets refactoring

[agirault]

Following-up from discussion notes here:
https://docs.google.com/document/d/1lbFj4N68PhuFC8NElz_7wVW655mMdxRdP8Qkn17wwA4

cc: @lassoan @floryst @jcfr @martinken @thewtex @aylward @timthirion

Background

Widgets in vtk-js are at a very early stage. We brought up some issues with the current architecture, and believe now is the time to address some of them since the number of widgets in vtk.js is still very limited.

Identified Issues

• Confusions between roles of widget and widgetRepresentation leads to unnecessary complexities
• Lack of separation between representation and source limits data sharing between multiple representations and forces duplications and syncing of state
• Very costly to have a lot of handle widgets

Proposed solutions

• [ ] Separate widget architecture into 3 classes for:
  
  
  
  • interaction (listening currently done in widget, and handling currently done in widgetRep),
  
  
  • representation (currently actor and mapper in widgetRep)
  
  
  • state/data (currently source in widgetRep).
  
  
• [ ] Base handlewidget on glyphs to optimize the creation of a high number of similar widgets (?)

Please edit with suggestions...

[lassoan]

Design could be somewhat similar to model-view-controller pattern, so I will use these terms here, but the final class names may be different:

• Model: Stores point positions, curve interpolation type, etc. There is one instance shared between all renderers. Maybe it could store some state information, such as selected or highlighted point, so that a point that is being selected or dragged can be highlighted in all views.
• View: Contains actors. Stores display properties (color, opacity, etc). There must be a separate instance for each renderer. There are different views, for example one for 3D and another one for 2D views. Probably the view needs to observe GUI events (mouse move, click, etc) and generate high-level events (point clicked, point moved, key pressed) that the controller observes.
• Controller: Observes view events and updates the model based on that. Probably we need a controller instance for each renderer, but it might be shared between renderers.

Handle widget: I don't think we can afford to have a HandleWidget object per each handle. This is what killing the performance today. Instead a HandleWidget object must manage a set of points (up to at least a few thousands): rendering, picking, highlighting, selecting, moving

[agirault]

@lassoan what about:

• model/state/source: what you said, with API to update state (such as translate, scale...)
• view/representation: storing actor, mapper, and display properties, with API to check if it is picked at a certain location (but no event handling, so it would be almost the state of the rendering)
• controller: listen to events from interactor (instead of the representation), check if currently interacting with the representation (using representation API), and update model/state accordingly.

[lassoan]

Good idea, I think this would work better. This would somewhat change how pickers are created and registered in the picking manager. It is currently done by the widget representation, but maybe this would become responsibility of the controller.

[thewtex]

@floryst @jourdain #done?

[jourdain]

Yes

On Oct 29, 2018, at 21:11, Matt McCormick [e-mail] wrote:

@floryst @jourdain #done?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[agirault]

Do we have good doc for the new architecture?

[jcfr]

Cc: @finetjul

[jourdain]

@floryst had my notes but I'm not sure we managed to formalize it online.

[floryst]

Currently working on making the docs! I'm stepping through the code and notes and sketching up a rough doc.

[agirault]

Thanks, @floryst! Can we keep that issue open then? Or open another issue?

[floryst]

I'll re-open for the purpose of tracking docs when I make that PR.

[jourdain]

:tada: This issue has been resolved in version 12.1.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: