Skip to content

mesh3d document update #593

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.

Sign up for GitHub

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 2 commits into from
Jun 2, 2016
Merged

mesh3d document update #593

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfd29d4
mesh3d document update
monfera Jun 1, 2016
f256361
mesh3d document update - review feedback
monfera Jun 1, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 62 additions & 15 deletions src/traces/mesh3d/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,57 @@ var extendFlat = require('../../lib/extend').extendFlat;
module.exports = {
x: {
valType: 'data_array',
description: 'Sets the x coordinates of the vertices'
description: [
'Sets the X coordinates of the vertices. The nth element of vectors `x`, `y` and `z`',
'jointly represent the X, Y and Z coordinates of the nth vertex.'
].join(' ')
},
y: {
valType: 'data_array',
description: 'Sets the y coordinates of the vertices'
description: [
'Sets the Y coordinates of the vertices. The nth element of vectors `x`, `y` and `z`',
'jointly represent the X, Y and Z coordinates of the nth vertex.'
].join(' ')
},
z: {
valType: 'data_array',
description: 'Sets the z coordinates of the vertices'
description: [
'Sets the X coordinates of the vertices. The nth element of vectors `x`, `y` and `z`',
Copy link
Member

Choose a reason for hiding this comment

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

@monfera is that supposed to be "Sets the Z coordinates..."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixing it

'jointly represent the X, Y and Z coordinates of the nth vertex.'
].join(' ')
},

i: {
valType: 'data_array',
description: 'Sets the indices of x coordinates of the vertices'
description: [
'A vector of vertex indices, i.e. integer values between 0 and the length of the vertex',
'vectors, representing the *first* vertex of a triangle. For example, `{i[m], j[m], k[m]}`',
'together represent face m (triangle m) in the mesh, where `i[m] = n` points to the triplet',
'`{x[n], y[n], z[v]}` in the vertex arrays. Therefore, each element in `i` represents a',
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence ("each element in i represents a point in space") makes it sound like each element should be a triplet of xyz coordinates. Is that the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, for example, i[5] = 8 means that the first vertex of triangle #5 is a point in 3D space that is represented by the tuple {X: x[8], Y:y[8] and Z: z[8]}. One triangle is made up of three vertices (in this case, i[5], j[5] and k[5].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdtusz in other words, the values in i etc. just point to or index into the vertices.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right - that's what I figured. The direct relation to a point in space just made it seem a bit ambiguous (although I suppose most mesh3d users will be up to speed on graphics nomenclature).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. Normally, an example set of arrays, and rendering results would help. Once we have a blog entry on the use of the mesh maybe we can put it in the doc string? Other external links may be too volatile.

I tried to remove the ambiguity by starting the doc with saying, 'vector of vertex indices, i.e. integer values between 0 and...'

'point in space, which is the first vertex of a triangle.'
].join(' ')
},
j: {
valType: 'data_array',
description: 'Sets the indices of y coordinates of the vertices'
description: [
'A vector of vertex indices, i.e. integer values between 0 and the length of the vertex',
'vectors, representing the *second* vertex of a triangle. For example, `{i[m], j[m], k[m]}` ',
'together represent face m (triangle m) in the mesh, where `j[m] = n` points to the triplet',
'`{x[n], y[n], z[v]}` in the vertex arrays. Therefore, each element in `j` represents a',
Copy link
Member

Choose a reason for hiding this comment

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

should z[v] be z[n]?

'point in space, which is the second vertex of a triangle.'
].join(' ')

},
k: {
valType: 'data_array',
description: 'Sets the indices of z coordinates of the vertices'
description: [
'A vector of vertex indices, i.e. integer values between 0 and the length of the vertex',
'vectors, representing the *third* vertex of a triangle. For example, `{i[m], j[m], k[m]}`',
'together represent face m (triangle m) in the mesh, where `k[m] = n` points to the triplet ',
'`{x[n], y[n], z[v]}` in the vertex arrays. Therefore, each element in `k` represents a',
Copy link
Member

Choose a reason for hiding this comment

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

same -- z[n] ?

'point in space, which is the third vertex of a triangle.'
].join(' ')

},

delaunayaxis: {
Expand All @@ -46,9 +75,10 @@ module.exports = {
values: [ 'x', 'y', 'z' ],
dflt: 'z',
description: [
'Sets the Delaunay axis from which the triangulation of the mesh',
'takes place.',
'An alternative to setting the `i`, `j`, `k` indices triplets.'
'Sets the Delaunay axis, which is the axis that is perpendicular to the surface of the',
'Delaunay triangulation.',
'It has an effect if `i`, `j`, `k` are not provided and `alphahull` is set to indicate',
'Delaunay triangulation.'
].join(' ')
},

Expand All @@ -57,11 +87,28 @@ module.exports = {
role: 'style',
dflt: -1,
description: [
'Sets the shape of the mesh',
'If *-1*, Delaunay triangulation is used',
'If *>0*, the alpha-shape algorithm is used',
'If *0*, the convex-hull algorithm is used',
'An alternative to the `i`, `j`, `k` indices triplets.'
'Determines how the mesh surface triangles are derived from the set of',
'vertices (points) represented by the `x`, `y` and `z` arrays, if',
'the `i`, `j`, `k` arrays are not supplied.',
'For general use of `mesh3d` it is preferred that `i`, `j`, `k` are',
'supplied, because calculating the surface from points can be very',
'time consuming and usually lead to artifacts.',
Copy link
Member

Choose a reason for hiding this comment

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

I would say shorten this up a bit and rm the explanation "because calculating the surface from points can be very time consuming and usually lead to artifacts."
^^That's true, but we could mention that bit in the blog. Just a suggestion, but I think it might make that description slightly more readable and still informative


'If *-1*, Delaunay triangulation is used, which is mainly suitable if the',
'mesh is a single, more or less layer surface that is perpendicular to `delaunayaxis`.',
'In case the `delaunayaxis` intersects the mesh surface at more than one point',
'(e.g. sphere, cube, potato, animal, donut etc. shapes) it will lead to triangles',
Copy link
Member

Choose a reason for hiding this comment

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

for the purpose of the docstring we can probably rm the (e.g.) line or use just one example like (e.g. sphere)

'that are very long in the dimension of `delaunayaxis`.',

'If *>0*, the alpha-shape algorithm is used. In this case, the `alphahull` value',
Copy link
Member

Choose a reason for hiding this comment

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

🎉 thanks! exactly the info I was looking for.
To be a bit more concise I would say:
'If >0, the alpha-shape algorithm is used. In this case, the alphahull value signals the use of the alpha-shape algorithm, and its value acts as the parameter for the mesh fitting.

'not only signals the intention to choose the alpha-shape algorithm, but its value',
'acts as the parameter for the mesh fitting. It can take minutes to calculate a large',
'mesh (e.g. over 10 thousand points), and the result is very sensitive to the',
Copy link
Member

Choose a reason for hiding this comment

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

I'd probably rm the time calc comment:
'It can take minutes to calculate a large mesh (e.g. over 10 thousand points), and the result is very sensitive to the chosen value. '
and save that for a blog post

'chosen value. ',

'If *0*, the convex-hull algorithm is used. It is suitable for convex bodies',
'or if the intention is to enclose the `x`, `y` and `z` point set into a convex',
'hull. However it can not generate concave surfaces.'
Copy link
Member

Choose a reason for hiding this comment

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

I'd take out: 'However it can not generate concave surfaces.' (& save for blog post)

].join(' ')
},

Expand Down Expand Up @@ -106,7 +153,7 @@ module.exports = {
dflt: false,
description: [
'Determines whether or not normal smoothing is applied to the meshes,',
'creating meshes with a low-poly look.'
'creating meshes with an angular, low-poly look via flat reflections.'
].join(' ')
},

Expand Down