Skip to content

Handle missing vertexcolor and facecolor in mesh3d #4353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 15, 2019

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Nov 14, 2019

Fix #4348 & #4349
gl-mesh3d used to stop when encountering a missing vertexcolor or facecolor which is fixed by gl-mesh3d v2.1.2 and v2.1.3 patches.
This PR bumps gl-mesh3d and adds an image test.

@plotly/plotly_js

@archmoj archmoj added bug something broken status: reviewable labels Nov 14, 2019
@alexcjohnson
Copy link
Collaborator

rename mising -> missing

missing vertex color reverts to white, but missing face color is omitted. Is that intentional / desirable? Offhand I'd think we'd want the faces to be white as well. Or perhaps a light gray in both cases...

@archmoj
Copy link
Contributor Author

archmoj commented Nov 14, 2019

We could use gray color for both cases which is pretty easy to do.
But I think when encountring missing data we should display the face or vertex as invisible.
In that regard, we could try turning vertices with missing vertexcolor to full transparent.
@alexcjohnson what do you suggest?

@alexcjohnson
Copy link
Collaborator

In general I feel like data should not disappear if some secondary characteristic of it (color in this case, but more generally anything beyond its position) is missing or invalid. For example scatter traces with a short marker.color array treat the missing items as black. (that said I see that a short marker.size array yields size=0, this argument would say it should yield the default size)

This is mostly useful for debugging, so you can see what parts you got right and what parts have issues. If your goal is to make a transparent face, you should specify this explicitly rather than relying on an implicit "bad data becomes transparent"

- display mesh3d missing colors gray
- update baseline
@archmoj
Copy link
Contributor Author

archmoj commented Nov 14, 2019

In general I feel like data should not disappear if some secondary characteristic of it (color in this case, but more generally anything beyond its position) is missing or invalid. For example scatter traces with a short marker.color array treat the missing items as black. (that said I see that a short marker.size array yields size=0, this argument would say it should yield the default size)

This is mostly useful for debugging, so you can see what parts you got right and what parts have issues. If your goal is to make a transparent face, you should specify this explicitly rather than relying on an implicit "bad data becomes transparent"

@alexcjohnson Good call.
Done in 3066f32.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mesh3d error when missing a facecolor
2 participants