Adds support for Line2 objects

[ferrolho]

Adds support for importing and rendering Line2 objects, including LineMaterial and LineGeometry. This allows for rendering lines with a configurable width (in either pixel-units or world-units), colour, dashes.

Also adds a watch script to webpack for easier development.

This PR complements rdeits/MeshCat.jl#270. This PR fixes rdeits/MeshCat.jl#246.

Figure 1. Example of working code in MeshCat.jl (PR in that repo to follow).

---

This change is 

[ferrolho]

Hi, @jwnimmer-tri! Can you have a look at this PR? Thanks! 🙏

[SeanCurtis-TRI]

+a:@SeanCurtis-TRI

[SeanCurtis-TRI]

Hi @ferrolho,

Thanks for the PR. I'll be helping shepherd this feature through so we can check all of the boxes.

I've taken the liberty of pushing review of this PR into reviewable. I'd suggest clicking on the link and exploring it if you're not already familiar with it. If you have any questions, please ask.

The two biggest items to tackle up front are documentation and test.

Documentation: The Readme.md file in the root of the repo documents the various supported meshcat commands. We should add a section for line geometry.

Tests: There is a test directory although the testing is quite informal. Essentially, the practice is to create a static webpage that exercises the parameters. Ideally it should be self-documenting so that someone exploring the test pages can know that what they're seeing is what they should expect to see. (See background.html as an example.)

Let me know if you have any questions.

[SeanCurtis-TRI]

Quick update on documentaiton:

1. I skimmed the PR less quickly and realize all you've done is extend the parameter space for the set_object command. So, you can limit your self to that.
2. The test is definitely the more important detail -- it unblocks reviewers from being able to explicitly see the code being exercised.

[ferrolho]

Hi, @SeanCurtis-TRI! Nice to "meet" you. 😃 Thank you so much for having a look at this PR! I believe I've addressed all your concerns in f17f1e6. Let me know what you think.

Here is what the test page looks like:

[SeanCurtis-TRI]

Great update! We're so close.

I've scattered some comments around the interview. I use a particular comment style supported by reviewable. Each comment is prefixed either with "nit" or "btw".

• "nit" - I've called out a defect that needs correction, but I think it's a straightforward fix. The solution is to effect the change and we're done. If you disagree, you can say so. I may be mistaken in my comment. If you make the change, you can type "Done" (and reviewable will automatically change your disposition state to completed and no longer blocking.
• "btw" - This comment isn't tagging a defect, but calling out a design choice. I'll make an alternative suggestion to see if you've considered it already. But, ultimately, I leave the choice up to you, the developer. If you accept the suggestion, you can make the change and then type "Done". If you reject the suggestion (or have an alternative), you can put a quick note and mark yourself sa done.

@SeanCurtis-TRI reviewed 4 of 4 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @ferrolho)

---

Readme.md line 205 at r2 (raw file):
BTW Not just "width". You detail the other properties well below, so, for here, probably simple enough to do:

...for lines with configurable properties.

---

test/fatlines.html line 360 at r2 (raw file):

                ])
            });

nit: I'd recommend we provide an initial pose for the camera so when you first load the page, the lines are immediately visible. This worked for me:

            // Pose the camera so its initial pose orbits around the line stack
            // and has all lines in view.
            viewer.handle_command({
                type: "set_transform",
                path: "/Cameras/default",
                matrix: new Float32Array([1, 0, 0, 0,
                                          0, 1, 0, 0,
                                          0, 0, 1, 0,
                                          0, 0, 0, 1]) 
            });
            viewer.handle_command({type: "set_property",
                path: "/Cameras/default/rotated/<object>",
                property: "position",
                value: [2, 3, 2]
            });
            viewer.handle_command({
                type: "set_target",
                value: [0.5, 1., -0.5]
            });

---

test/fatlines.html line 376 at r2 (raw file):

            descriptionDiv.style.pointerEvents = 'none';
            descriptionDiv.style.boxShadow = '0 4px 6px rgba(0,0,0,0.3)';
            descriptionDiv.innerHTML = `

BTW I love this overlay. Looks so polished and is incredibly effective. It's kind of nice to see what this can look like when someone who is actually webpage savvy does it. Very nice.

---

src/index.js line 195 at r1 (raw file):

//   * A new `THREE.Mesh` if that json represents a special geometry
//   * `null` otherwise
function handle_special_material(mat) {

nit: You defined this function directly underneath the documentation for the next function (handle_special_geometry()). Let's put the documentation and function back together.

---

src/index.js line 198 at r1 (raw file):

    if (mat.type == "LineMaterial") {
        // Handle LineMaterial (for fat lines)
        let material = new LineMaterial({

nit: This entire function can be greatly simplified

let material = new LineMaterial(mat);

1. The default values you've encoded in the current dictionary exactly match the documented default values of LineMaterial, so we don't need to build in the redundancy.
2. By spelling out your own dictionary, you limit which properties can be set. By simply passing the whole dictionary, then the user can specify any valid property for LineMaterial in their dictionary and it will be used appropriately.

---

src/index.js line 238 at r1 (raw file):

        if (geom.data && geom.data.attributes) {
            if (geom.data.attributes.position) {
                let positions = geom.data.attributes.position.array;

BTW What's the inspiration for the intermediate data and attribute values?

While it is true that the constructed geometry does have an attribute geometry (and it has keys such as position), it's worth noting that the values stored in that array are different from the values provided here (the vertex sequence gets converted to a line segment sequence).

As such I'm inclined to think it might be better flattened. Simply as:

{
   type: "LineGeometry",
   uuid: "stuff",
   position: {
      array: new Float32Array([...])
   },
   color: {
      array: new Float32Array([...])
   }
}

It has the virtue of making the dictionary more compact and puts a bit more distance between this dictionary and the transformations it will undergo by the time it becomes the final in-memory LineGeometry.

That also makes it resemble things like BoxGeometry, where the type and the parameters are all contained at the same level. On the whole, if we go with the added complexity, we should be able to justify it.

What do you think?

---

Readme.md line 203 at r2 (raw file):

                </pre>
                Check <code>test/meshfile_object_obj.html</code> for the full demo.
                <p>Example (Line2 with <code>LineGeometry</code> and <code>LineMaterial</code>):

nit: When I look at this on github, the previous line ("Check ...) runs into this example. Perhaps an extra line break for greater separation?

---

Readme.md line 214 at r2 (raw file):

        geometries: [
            {
                uuid: "line-geom-uuid",

nit: All of the other examples in this file have uuid values that look like uuids. Let's continue the pattern here (even if it's just copy and paste from one of the other uuids...which is so frequently done in these examples.)

---

Readme.md line 253 at r2 (raw file):

                <li><code>color</code>: (optional) Float32Array of per-vertex RGB colors (r, g, b, r, g, b, ...)
                </ul>
                <p>LineMaterial supports these properties:

nit: For LineGeometry, we can simply refer them to the three.js documentation. We're not doing anything special. (See my note in index.js.) The full message is that we support all properties that LineGeometry supports. And then provide the link:

https://threejs.org/docs/#examples/en/lines/LineMaterial

We'll let the three.js documentation do the heavy lifting and not have to worry if it evolves at all.

[ferrolho]

Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @ferrolho)

---

Readme.md line 203 at r2 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

nit: When I look at this on github, the previous line ("Check ...) runs into this example. Perhaps an extra line break for greater separation?

Done

---

Readme.md line 205 at r2 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

BTW Not just "width". You detail the other properties well below, so, for here, probably simple enough to do:

...for lines with configurable properties.

Done

---

Readme.md line 214 at r2 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

nit: All of the other examples in this file have uuid values that look like uuids. Let's continue the pattern here (even if it's just copy and paste from one of the other uuids...which is so frequently done in these examples.)

Done

---

Readme.md line 253 at r2 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

nit: For LineGeometry, we can simply refer them to the three.js documentation. We're not doing anything special. (See my note in index.js.) The full message is that we support all properties that LineGeometry supports. And then provide the link:

https://threejs.org/docs/#examples/en/lines/LineMaterial

We'll let the three.js documentation do the heavy lifting and not have to worry if it evolves at all.

Done

[SeanCurtis-TRI]

@SeanCurtis-TRI reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @ferrolho)

[ferrolho]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @SeanCurtis-TRI)

---

src/index.js line 195 at r1 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

nit: You defined this function directly underneath the documentation for the next function (handle_special_geometry()). Let's put the documentation and function back together.

Oops! Addressed in 172afe6.

---

src/index.js line 198 at r1 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

nit: This entire function can be greatly simplified

let material = new LineMaterial(mat);

1. The default values you've encoded in the current dictionary exactly match the documented default values of LineMaterial, so we don't need to build in the redundancy.
2. By spelling out your own dictionary, you limit which properties can be set. By simply passing the whole dictionary, then the user can specify any valid property for LineMaterial in their dictionary and it will be used appropriately.

Addressed in 596901a. I've left material.uuid = mat.uuid; in there because I saw handle_special_geometry also assigns uuids explicitly, but I'm not sure whether it is needed.

---

src/index.js line 238 at r1 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

BTW What's the inspiration for the intermediate data and attribute values?

While it is true that the constructed geometry does have an attribute geometry (and it has keys such as position), it's worth noting that the values stored in that array are different from the values provided here (the vertex sequence gets converted to a line segment sequence).

As such I'm inclined to think it might be better flattened. Simply as:

{
   type: "LineGeometry",
   uuid: "stuff",
   position: {
      array: new Float32Array([...])
   },
   color: {
      array: new Float32Array([...])
   }
}

It has the virtue of making the dictionary more compact and puts a bit more distance between this dictionary and the transformations it will undergo by the time it becomes the final in-memory LineGeometry.

That also makes it resemble things like BoxGeometry, where the type and the parameters are all contained at the same level. On the whole, if we go with the added complexity, we should be able to justify it.

What do you think?

There was no grand inspiration and I agree it unnecessarily complicates the data structure. I've removed the data.attributes. levels in 14d8175.

---

test/fatlines.html line 360 at r2 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

nit: I'd recommend we provide an initial pose for the camera so when you first load the page, the lines are immediately visible. This worked for me:

            // Pose the camera so its initial pose orbits around the line stack
            // and has all lines in view.
            viewer.handle_command({
                type: "set_transform",
                path: "/Cameras/default",
                matrix: new Float32Array([1, 0, 0, 0,
                                          0, 1, 0, 0,
                                          0, 0, 1, 0,
                                          0, 0, 0, 1]) 
            });
            viewer.handle_command({type: "set_property",
                path: "/Cameras/default/rotated/<object>",
                property: "position",
                value: [2, 3, 2]
            });
            viewer.handle_command({
                type: "set_target",
                value: [0.5, 1., -0.5]
            });

I agree! Added your suggestion exactly as you wrote it in c9b2622.

[SeanCurtis-TRI]

Thanks for the contribution. It'll be nice to use polylines to visualize trajectories and such.

@SeanCurtis-TRI reviewed 4 of 4 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ferrolho)

---

src/index.js line 198 at r1 (raw file):

Previously, ferrolho (Henrique Ferrolho) wrote…

Addressed in 596901a. I've left material.uuid = mat.uuid; in there because I saw handle_special_geometry also assigns uuids explicitly, but I'm not sure whether it is needed.

It's not explicitly necessary to assign the uuid explicitly. It is one of the members of the class that sets itself from the parameter dictionary. But it's probably harmless to be explicit (and redundant) about it.