Geometry: Move computeLineDistance() to Line/LineSegments by Mugen87 · Pull Request #13147 · mrdoob/three.js

Mugen87 · 2018-01-21T13:06:24Z

This PR moves .computeLineDistance() to Line and adds support for (non-indexed) BufferGeometry. The combination of LineDashedMaterial and BufferGeometry does 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)

Mugen87 · 2018-01-21T13:07:28Z

examples/canvas_lines_dashed.html


 				var geometryCube = cube( 50 );

-				geometryCube.computeLineDistances();


BTW: With CanvasRenderer it is not necessary to compute the line distances for dashed lines...

Mugen87 · 2018-01-21T13:19:07Z

LineSegments is also supported with this PR. Like mentioned in the docs .computeLineDistance() calculates for each point in the geometry the distance to the very beginning of the line. This also works for LineSegments objects.

Mugen87 · 2018-01-21T13:24:55Z

I wonder if we could "hide" the call of .computeLineDistance() inside the renderer. This would avoid issues like https://discourse.threejs.org/t/linedashedmaterial-does-not-work/1640

So if the renderer detects an instance of LineDashedMaterial in a line object, it checks the line distance data and generates them if necessary.

WestLangley · 2018-01-21T15:34:07Z

src/objects/Line.js

+
+			if ( geometry.isBufferGeometry ) {
+
+				// we assume non-indexed geometry


Don't you think you should handle indexed geometry gracefully?

You're right. It should be better with 0286198

WestLangley · 2018-01-21T15:35:45Z

src/objects/Line.js

+
+					}
+
+					lineDistances.push( distance );


I wish we did not always create these JS arrays unnecessarily. It is a pattern we use over-and-over...

WestLangley · 2018-01-21T15:42:23Z

LineSegments is also supported with this PR.

To do so, the distances need to be calculated differently for line segments.

Mugen87 · 2018-01-21T15:48:07Z

What would be different? I thought we calculate for each point the distance to the beginning of the line...

WestLangley · 2018-01-21T15:57:40Z

I thought we calculate for each point the distance to the beginning of the line...

We should be calculating the cumulative length.

Mugen87 · 2018-01-21T16:07:45Z

Ah, now i understand. When using LineSegments we should only consider the distances between the start and end point of each segment. So let's overwrite .computeLineDistance() in LineSegments.

WestLangley · 2018-01-21T16:52:09Z

src/objects/Line.js

+
+					geometry.lineDistances[ i ] = distance;
+
+				}


geometry.lineDistances[ 0 ] = 0; for ( var i = 1, l = vertices.length; i < l; i ++ ) { geometry.lineDistances[ i ] += vertices[ i - 1 ].distanceTo( vertices[ i ] ); }

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 · 2018-02-07T11:44:38Z

Related: #8494

When the PR is merged, there is one less reason for users to convert their lines based on BufferGeomery to Geometry if they want to use LineDashedMaterial. The conversion is not supported anyway.

mrdoob · 2018-02-09T21:59:30Z

Thanks!

WestLangley · 2018-02-13T23:50:39Z

N.B. LineDashedMaterial does not work correctly with LineLoop because there is no distance computed for the last leg.

Mugen87 · 2018-02-14T14:52:56Z

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 LineLoop, right?

WestLangley · 2018-02-14T17:51:26Z

Right. That's the problem.

Geometry: Move computeLineDistance() to Line

4b82ecc

Mugen87 commented Jan 21, 2018

View reviewed changes

Mugen87 added 2 commits January 21, 2018 14:13

Exampels: Fix copy/paste error

c0e5d25

Examples: Improved webgl_lines_dashed.html

e463b8c

WestLangley reviewed Jan 21, 2018

View reviewed changes

Line: Handle indexed geometry more gracefully

0286198

Mugen87 added 2 commits January 21, 2018 17:17

LineSegments: Added .computeLineDistances()

2fbec23

LineSegments: Fix log message

0ec8b93

WestLangley reviewed Jan 21, 2018

View reviewed changes

Line/LineSegments: Refactoring

e1391a9

Mugen87 changed the title ~~Geometry: Move computeLineDistance() to Line~~ Geometry: Move computeLineDistance() to Line/LineSegments Jan 21, 2018

Docs: Improve description for Line.computeLineDistances()

ccfee63

mrdoob added this to the r91 milestone Jan 21, 2018

Tests: Clean up

aeb4865

mrdoob modified the milestones: r91, r90 Feb 9, 2018

mrdoob merged commit e3cea7e into mrdoob:dev Feb 9, 2018

FishOrBear mentioned this pull request Apr 8, 2018

<threejs> Geometry: Move computeLineDistance() to Line DefinitelyTyped/DefinitelyTyped#24803

Merged


		var geometryCube = cube( 50 );

		geometryCube.computeLineDistances();


		if ( geometry.isBufferGeometry ) {

		// we assume non-indexed geometry

Uh oh!

Conversation

Mugen87 commented Jan 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mugen87 Jan 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mugen87 commented Jan 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mugen87 commented Jan 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WestLangley Jan 21, 2018

Choose a reason for hiding this comment

Uh oh!

Mugen87 Jan 21, 2018

Choose a reason for hiding this comment

Uh oh!

WestLangley Jan 21, 2018

Choose a reason for hiding this comment

Uh oh!

WestLangley commented Jan 21, 2018

Uh oh!

Mugen87 commented Jan 21, 2018

Uh oh!

WestLangley commented Jan 21, 2018

Uh oh!

Mugen87 commented Jan 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WestLangley Jan 21, 2018

Choose a reason for hiding this comment

Uh oh!

Mugen87 Jan 21, 2018

Choose a reason for hiding this comment

Uh oh!

Mugen87 commented Feb 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrdoob commented Feb 9, 2018

Uh oh!

WestLangley commented Feb 13, 2018

Uh oh!

Mugen87 commented Feb 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WestLangley commented Feb 14, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mugen87 commented Jan 21, 2018 •

edited

Loading

Mugen87 Jan 21, 2018 •

edited

Loading

Mugen87 commented Jan 21, 2018 •

edited

Loading

Mugen87 commented Jan 21, 2018 •

edited

Loading

Mugen87 commented Jan 21, 2018 •

edited

Loading

Mugen87 commented Feb 7, 2018 •

edited

Loading

Mugen87 commented Feb 14, 2018 •

edited

Loading