Geometry: Move computeLineDistance() to Line/LineSegments #13147
Conversation
examples/canvas_lines_dashed.html
Outdated
|
|
||
| var geometryCube = cube( 50 ); | ||
|
|
||
| geometryCube.computeLineDistances(); |
There was a problem hiding this comment.
BTW: With CanvasRenderer it is not necessary to compute the line distances for dashed lines...
|
|
|
I wonder if we could "hide" the call of So if the renderer detects an instance of |
|
|
||
| if ( geometry.isBufferGeometry ) { | ||
|
|
||
| // we assume non-indexed geometry |
There was a problem hiding this comment.
Don't you think you should handle indexed geometry gracefully?
There was a problem hiding this comment.
You're right. It should be better with 0286198
src/objects/Line.js
Outdated
|
|
||
| } | ||
|
|
||
| lineDistances.push( distance ); |
There was a problem hiding this comment.
I wish we did not always create these JS arrays unnecessarily. It is a pattern we use over-and-over...
To do so, the distances need to be calculated differently for line segments. |
|
What would be different? I thought we calculate for each point the distance to the beginning of the line... |
We should be calculating the cumulative length. |
|
Ah, now i understand. When using |
src/objects/Line.js
Outdated
|
|
||
| geometry.lineDistances[ i ] = distance; | ||
|
|
||
| } |
There was a problem hiding this comment.
geometry.lineDistances[ 0 ] = 0;
for ( var i = 1, l = vertices.length; i < l; i ++ ) {
geometry.lineDistances[ i ] += vertices[ i - 1 ].distanceTo( vertices[ i ] );
}There was a problem hiding this comment.
That did not work for me since geometry.lineDistances[ i ] needs to be initialized with the previous value. Otherwise the summation does not work.
geometry.lineDistances[ 0 ] = 0;
for ( var i = 1, l = vertices.length; i < l; i ++ ) {
geometry.lineDistances[ i ] = geometry.lineDistances[ i - 1 ];
geometry.lineDistances[ i ] += vertices[ i - 1 ].distanceTo( vertices[ i ] );
}
Mugen87
changed the title
|
Related: #8494 When the PR is merged, there is one less reason for users to convert their lines based on |
|
Thanks! |
|
N.B. |
|
Even if we compute the distance from the last vertex to the first one, how would we put this value in the lineDistances array/attribute? Normally we have for each vertex a respective distance value. This seems not to be true for |
|
Right. That's the problem. |
This PR moves
.computeLineDistance()toLineand adds support for (non-indexed)BufferGeometry. The combination ofLineDashedMaterialandBufferGeometrydoes work now. This feature was frequently asked by users, e.g.https://discourse.threejs.org/t/computelinedistances-with-buffergeometry/644 or
#7013
The implementation is based on @WestLangley's suggestion, see #7013 (comment)