Move visualization properties from visualizer to Nodes

[dcambie]

For many PvTrace simulation it would be nice to be able to add node visualization properties, such as color transparency etc. However, currently, the only visualization options (wireframe, transparency, opacity and reflectivity) are attributes of MeshcatRenderer and apply in the same way to all the scene objects.

This can be modified at runtime *by setting the corresponding Meshcat material attributes) but there is currently no way to save the visualization options in the scene definition (e.g. in the YML file)

This commit allows for more granular control on the visual appearances of the scene, when rendered. At the same time, the old global switches at the MeshcatRenderer are kept and are given priority as, e.g. it may be needed/necessary/useful to show all scene nodes as wireframe for debugging purposes.

[danieljfarrell]

Hi Dario,

Oh great to see you have been testing out the CLI branch. Do you find yourself using this now rather than Python scripts? It's such a big change that I have not merged it into the master yet. But maybe there could be a beta release at some point? Which reminds we, I never got around to publishing the last release, I should do that soon!

---

I really like this idea!

The history of the visualiser is that I needed something for debugging the ray tracing algorithm and testing all of the node coordinate system transformations. If I remember correctly, I think you actually found a bug in that code, using the visualiser. So it is certainly a bit barebones and I've toyed with the idea of making it an optional dependency, but it does seem popular.

I wonder if visualisation properties/styles could be implemented in away that preserves the strict separation between model and view objects?

I see two places in the pull request where model and view objects have blurry responsibilities:

1. The model class Material inheriting from the view class g.MeshBasicMaterial could be a problem if we want to add more fully featured visualisers in the future such as VTK or OpenCSG?
2. Similarly, the YML file format adds view attributes directly to the model.

Let me know what you think.

[dcambie]

Hi Dan,

Ok, I understand your point of keeping model and view separate, it makes a lot of sense.

Would about the visualization attribute being kept as a dictionary in Node? I believe that makes more sense than Geometry (whose base class is an abstract base class).

Ideally a few basic appearance attribute that can be shared across all different visualization engines (i.e. color: int, transparency: float and maybe visible: bool) and other could be implementation-specific. Also, it would be nice if any arbitrary parameter can be set for a specific visualization engine and passed directly to the corresponding visualization object without redirection/dedicated PvTrace code.

imho All of this could be as simple as an appearance dictionary as new attribute in Node (see pseudocode as last commit)

e.g. corresponding YML (in the schema meshcat and other visualizer can be optional with "additionalProperties": True

  ball-lens:
    location: [0, 0, 2]
    sphere:
      radius: 1.0
      material:
        refractive-index: 1.5
    appearance:
      color: 0x00ff00
      visible: True
      meshcat:
        opacity: 0.5

What do you think about that?

Yes I've been playing a bit with the CLI branch as we currently have a script that generate a scene, but for a different reactor it needs some changes so I was considering generating the YML files instead and use those for the scene definition. I'll test more of that in the coming days and I'll come back to you with more details.

[danieljfarrell]

Yeah this is nice solution. Busy right now but will get back to you later.

[danieljfarrell]

Hi Dario,

Let me know when you are ready for me to take a look and I’ll do the merge.

[dcambie]

I'd say it is ready.

There are just a couple of changes that are not strictly related to this PR but should be non controversial, namely:

• elif absorption_spectrum is not None: these are just to fix the parsing of yml where the spectrum is defined as pandas dataframe. Otherwise when spectrum is a numpy array a ValueError is raised (The truth value of an array with more than one element is ambiguous. Use a.any() or a.all())

• a more diagnostic error on file not found for spectrum

• a minor change to the hello world.

[danieljfarrell]

I should make an alpha release of the CLI branch soon!