Cone sizeref fixes #2715
Cone sizeref fixes #2715
Conversation
src/traces/cone/convert.js
Outdated
| coneOpts.coneSize = trace.sizeref || 0.5; | ||
| } else { | ||
| // sizeref here has unit of velocity | ||
| coneOpts.coneSize = (trace.sizeref / trace._normMax) || 0.5; |
There was a problem hiding this comment.
Previously, coneSize under sizemode: 'absolute' was scaled using the max norm of the "scaled" u/v/w vectors (that same dataScale than in #2646). This made sizeref under sizemode: 'absolute' had little meaning.
Now, sizeref under sizemode: 'absolute' is scaled by the "input" u/v/w max norm should make this thing a little less confusing.
There was a problem hiding this comment.
Nice! Two questions:
- When is the
|| 0.5(in either case) necessary? - What if the field is uniformly zero? then
_normMaxwill be0andconeSizewill beInfinity... which I suppose is OK if the end result is no errors and nothing drawn.
There was a problem hiding this comment.
What if the field is uniformly zero?
Oh right, Infinity is truthy. Good call!
There was a problem hiding this comment.
When is the || 0.5 (in either case) necessary?
Currently, the sizeref attribute declaration does not set a dflt field. In practice I guess it does have a hard default of 0.5. But technically under the sizemode: 'absolute', the sizemode default is 0.5 * trace._normMax.
src/traces/cone/attributes.js
Outdated
| 'for vector magnitude 1 is one grid unit.' | ||
| 'Determines whether `sizeref` is set as a *scaled* (i.e unitless) scalar', | ||
| '(normalized by the max u/v/w norm in the vector field) or as', | ||
| '*absolute* value (in unit of velocity).' |
There was a problem hiding this comment.
I wouldn't use the word "velocity" here - you can show many other quantities as vector fields (electric fields, as just one common example). Just say "in the same units as the vector field" or some such.
There was a problem hiding this comment.
oh right, good call!
src/traces/cone/attributes.js
Outdated
| 'Adjusts the cone size scaling.', | ||
| 'The size of the cones is determined by their u/v/w norm multiplied a factor and `sizeref`.', | ||
| 'This factor (computed internally) corresponds to the minimum "time" to travel across two', | ||
| 'two successive x/y/z positions at the average velocity of those two successive positions', |
There was a problem hiding this comment.
two twos is one two too many
|
Great! Smoothing off a few more rough edges with cones. 💃 |
fixes #2700 and #2701 (with the help gl-vis/gl-cone3d#12).
This PR also improves hover on cones by picking the entire cone as opposed to overlaying
gl-scatter3dpoints and using those for picking. Note that, the hover label remain at the same positions as before.I still need to fixup a few tests, but I would appreciate a review of commit 4836861 (where I tried to make sense of
sizemode: 'absolute') and commit gl-vis/gl-cone3d@3b3ef9d (where I impleted a "local" find-vectorScale algo as proposed by @alexcjohnson in gl-vis/gl-cone3d@3b3ef9d)