informatics-isi-edu / mesh-viewer

3D mesh viewer
1 stars 1 forks source link

Display the caption w/ link (possibly in an image Legend) #18

Closed robes closed 6 years ago

robes commented 6 years ago

The model specification shown here:

https://github.com/informatics-isi-edu/mesh-viewer#model-specification

specifies for each mesh object a caption and a link. The caption should be displayed with the image and the caption should be a link to take the user to new location. We can explore a couple approaches:

1- display the (linkable) caption in the "Meshes" drop down menu along with the controls/toggles for each mesh object. 2- display the (linkable) captions in a "Legend" that can be displayed along with the image, along with the corresponding color for the mesh.

The latter approach (showing a Legend) may be the ideal but both are worth exploring.

NickolausDS commented 6 years ago

Looking through the Model Specification, I don't see a reference to displaying images. Do mesh links usually point to images instead of documentation?

I see the Meshes dropdown, but not a Legend. Would that be a new UI element that would go alongside the Meshes dropdown?

I can start prototyping on 1 above, since it's straight forward and doesn't depend on adding new UI pieces. We can move that around if having the caption/link in the dropdown would be undesirable.

robes commented 6 years ago

I see the Meshes dropdown, but not a Legend. Would that be a new UI element that would go alongside the Meshes dropdown?

I think currently, when there is no label property (per the model specification referenced above) the viewer displays the mesh object's basename. I could be wrong but that's what I recall.

So option one would be that when there is a label use that in the Meshes dropdown (in place of the default basename being used) and when there is a link then make the label in the meshes drop down a link (to the value of the link property which is supposed to be a URL).

NickolausDS commented 6 years ago

I started tinkering with mesh-viewer and it looks like the behavior here already exists. This is the model I've been using:

https://dev.facebase.org/mesh-viewer/view.html?model=https://dev.facebase.org/data/mesh/19dpf/19model.json

For clarity (since the terms label, link, caption are reused in multiple objects) here's the first mesh object in 19model.json:

{
  "id": "Taenia Marginalis Anterior",
  "label": "Taenia Marginalis Anterior",
  "url": "https://dev.facebase.org/data/mesh/19dpf/tna.obj",
  "color": [0.49, 0.49, 0.49],
  "opacity":1,
  "caption": {
               "description":"a TMA mesh",
               "link": { 
                           "label":"gene expression",
                           "url":"https://dev.facebase.org/data/mesh/gene.html"
                           }
              }
},

When label key above is removed, I can confirm it falls back on the basename and displays tna.obj. Additionally, caption.link.url is linked to under the "Meshes" tab for each mesh displayed, in this case it links to https://dev.facebase.org/data/mesh/19dpf/tna.obj.

It seems to me this fits the criteria, did we need anything else?

robes commented 6 years ago

It's close, but it doesn't fit the spec. I'm going to make a change to the model spec right now and then the remaining task (at least to satisfy approach 1) is to make it conform to spec.

robes commented 6 years ago

I've updated the spec, like this:

...
    "mesh" : [ {
        "id": "MESH7890",
        "url": "https://www.example.org/path/to/MOD1234.obj",
        "label": "My Mesh",
        "link": {
            "label": "ANATOMICAL STRUCTURE",
            "url": "https://www.example.org/path/to/info/about/ANATOMICALSTRUCTURE"
        },
        "caption": "This is a mesh object in a model.",
        "color": [1.00, 0.80, 0.40]
    } ],
...

Similar to how it is now implemented, I would say the logic should be:

NickolausDS commented 6 years ago

Are there enough meshes currently in use that we should check for legacy formats under the path: mesh.[].caption.link.url?

Additionally, the model I posted lists all of its labels under mesh.[].label where mesh.[].caption.link.label is the same value gene expression for all meshes (which would make them all indistinguishable from one another). Is this something we need to be aware of? This isn't a problem if we don't need to worry about backwards compatibility and all new meshes properly list their label under mesh.[].link.label.

robes commented 6 years ago

No, we don't need to be concerned with legacy formats. They are generated on-the-fly, so as soon as the mesh-viewer can support it we'll update the server to produce the new format.

And then secondly, right we don't need to worry about backwards compatibility.

NickolausDS commented 6 years ago

The current PR should be all you need then: #22

NickolausDS commented 6 years ago

I added the fix here for meshes that don't define links: #23

This should fix the mesh listed here: https://dev.facebase.org/mesh-viewer/view.html?model=/ermrest/catalog/1/entity/viz:model_json/id=1

NickolausDS commented 6 years ago

All Pull Requests for this Issue have been merged.