Skip to content
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

New Meshline #11040

Closed
wants to merge 14 commits into from
Closed

New Meshline #11040

wants to merge 14 commits into from

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Mar 21, 2017

This PR introduces a new and flexible way of drawing lines via Screen-Space Projected Lines. The implementation is based on https://github.com/spite/THREE.MeshLine by @spite.

Demo

You can create a mesh line like this:

var line = new THREE.MeshLine( geometry, material );
  • geometry can be a Geometry or BufferGeometry. Users can provide vertices (position) and color data (for vertex colors).
  • material must be a MeshBasicMaterial. It's the only material that is supported right now. You can use three new material properties to control the visual appearance of the line: meshLine(like morphTargets) , meshLineWidth, meshLineSizeAttenuation. Other material properties like color, transparent or opacity also work.
  • Besides, there is a third and optional parameter of MeshLine that can be used to create nice curve shapes (widthCallback).

As soon as we commit to a final API design and naming conventions, i will also create some docs with a subsequent PR.

There is still an open issue. I'm not sure how to handle the situation if the geometry of a mesh line changes. MeshLine always needs to process the given geometry to prepare the data for the vertex shader. The original MeshLine project does this with a separate method setGeometry. But i wonder if we can find a way that fits better to the library conventions and is maybe more convenient.

@mrdoob
Copy link
Owner

mrdoob commented Mar 21, 2017

You're such a life saver! ❤️

@mrdoob
Copy link
Owner

mrdoob commented Mar 21, 2017

What do you think about make this transparent though? So if THREE.Line's linewidth > 1 then WebGLRenderer uses this code. Otherwise it goes through the usual path.

We'll have to lose widthCallback but, at the end of the day, I think it would be great if THREE.Line linewidth just worked.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 21, 2017

So if THREE.Line's linewidth > 1 then WebGLRenderer uses this code. Otherwise it goes through the usual path.

After some research i was not able to find a nice integration of both approaches. But to be honest, i don't know all parts of WebGLRenderer in detail. If there is a smart solution to do this, i'm of course open to it. But currently it seems to me, that the code in WebGLRenderer will be more complicated if we mix the rendering of line primitives and screen-space projected lines.

Therefore i would propose something different 😅.

Line, LineLoop and LineSegments should be exclusively used for rendering line primitives. Because the usage of gl.lineWidth() is not reliable (reference), i would remove linewidth from LineBasicMaterial and LineDashedMaterial and render all line primitives with the default line width of 1. If users still apply linewidth > 1 to a Line* material, three.js could log a warning to use MeshLine in combination with MeshBasicMaterial instead.

@fernandojsg
Copy link
Collaborator

So if THREE.Line's linewidth > 1 then WebGLRenderer uses this code. Otherwise it goes through the usual path.

As this is screen space implementation it won't work properly when using WebVR. That's why I needed to create a actual geometry mesh line for a-painter.
I'd planned to send a PR to add it, we could probably use three of them:

  • If non-vr and Linewidth == 1 -> THREE.line
  • If non-vr and LineWidth > 1 -> THREE.MeshLineSS
  • If vr -> THREE.MeshLine.

Actually for me is also confusing the name MeshLine as I expected it to be real geometry mesh instead of a SS representation, so probably renaming it to a more clear name could be nice too.
Another problem I got with Jaumes THREE.MeshLine` implementation is that it lacks of most of the material options that we have for the rest of geometry and that could be applied to an actual geometry mesh line.


var object = scene.children[ i ];

if ( object instanceof THREE.Line ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

THREE.MeshLine

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍


// animate line width

material1.meshLineWidth = 10 * ( 1 + 0.5 * Math.sin( 5 * time ) );
Copy link
Collaborator

Choose a reason for hiding this comment

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

material2.meshLineWidth = 100 * ( 1 + 0.5 * Math.sin( 5 * time ) );

Suggestion. Vary material2 instead, and use OrbitControls. It is easier to zoom and see the subtitles of what is going on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good idea!

Copy link
Collaborator

Choose a reason for hiding this comment

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

100 *
please so it is visible on small devices.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps also set size attenuation on the first one.

What are the units of width?

Copy link
Collaborator Author

@Mugen87 Mugen87 Mar 22, 2017

Choose a reason for hiding this comment

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

Sry, overlooked the 100 * thing. I've also added size attenuation to the first material.

Copy link
Collaborator Author

@Mugen87 Mugen87 Mar 22, 2017

Choose a reason for hiding this comment

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

meshLineWidth should be equal to the value of gl.lineWidth() so it's pixel...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for this. You can change it later if you want. For now, it is helpful. :)

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 22, 2017

I'm not sure about the way how to handle different line implementations. We could create separate entities like THREE.Line or THREE.MeshLine and let the user decide what to use. On the other hand we can have just a single entity (e.g. THREE.Line) and run transparently different implementations dependent on various factors (material properties or VR/non-VR).

@fernandojsg, @WestLangley What's your opinion about this?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 22, 2017

As this is screen space implementation it won't work properly when using WebVR.

What's the actual problem in this case?

That's why I needed to create a actual geometry mesh line for a-painter.

Can you share the code for this? I am interested to see your solution 😊

@WestLangley
Copy link
Collaborator

I am not sure MeshLine is a good name for a screen-space approach. Trying to think of an alternate nomenclature...

  • If non-vr and Linewidth == 1 -> THREE.Line
  • If non-vr and LineWidth > 1 -> THREE.ThickLine ??? or THREE.Line if implementation is transparent
  • If vr -> THREE.Ribbon ???

@mrdoob
Copy link
Owner

mrdoob commented Mar 22, 2017

That's why I needed to create a actual geometry mesh line for a-painter.

Can you share the code for this? I am interested to see your solution 😊

He's probably creating a tube. Which makes me think that we could also create a tube too..

@WestLangley
Copy link
Collaborator

What are the units of width?

meshLineWidth should be equal to the value of gl.lineWidth() so it's pixel...

If it is consistent with THREE.Points, then fine. If not, we should figure out why. Also consider DPR.

@fernandojsg
Copy link
Collaborator

Thinking about names I like @WestLangley proposal for THREE.Ribbon , as it's actually that, not a tube.
I'm not sure about the approach discussed for vr/non-vr and linewidths as they're not really consistent for example regarding materials, so probably I'll just leave it to the user to choose which one they want to use.

I'll try to open a PR for the THREE.Ribbon later this week.

@WestLangley
Copy link
Collaborator

We used to have THREE.Ribbon. I do not recall the implementation details, though. It was removed for some reason...

@spite
Copy link
Contributor

spite commented Mar 22, 2017

It's not a Ribbon, either, if we're being technical.

I'm afraid you're trying to cram way too functionality in Line: WebGL Lines are mathematical lines, and there's the unfortunate problem with lineWidth across platforms, which makes sense, since the definition of a line is precisely that doesn't have breadth or thickness.

That's the main and only thing to solve for a Line replacement (and it could be built into the THREE.Line family transparently). Other stuff, like adding properties from other materials or rendering lines in VR (what does that even mean!), it should just be a Mesh, any Mesh: CurvePath, ShapePath, Ribbon, what have you.

Just my two cents.

@mrdoob
Copy link
Owner

mrdoob commented Mar 22, 2017

Agreed. I think we should leave the VR use case out of this discussion. We don't want to turn THREE.Line into TiltBrush. We just want to have lines with lineWidth > 1, which is something that used to work (OSX only anyway).

@mrdoob
Copy link
Owner

mrdoob commented Mar 22, 2017

After some research i was not able to find a nice integration of both approaches. But to be honest, i don't know all parts of WebGLRenderer in detail. If there is a smart solution to do this, i'm of course open to it. But currently it seems to me, that the code in WebGLRenderer will be more complicated if we mix the rendering of line primitives and screen-space projected lines.

So, unless someone else tries it before me, I'll use this code as reference and try to add support for lineWidth > 1 in WebGLRenderer for r86.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 22, 2017

Sounds good to me. If i can manage it, i'll have a look at the integration of the code into WebGLRenderer in the next days again. For the case i make any kind of progress, i'll post it here.

@fernandojsg
Copy link
Collaborator

Ok I agree, it sound good to me too, better to leave the "ribbon" out from this and just in case add as an external example but nothing more.
Maybe just for the vr case, we could just force use simple lines instead of the SS ones.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 22, 2017

Also consider DPR.

@WestLangley That was a good hint! 👍 The last commit makes the line width of MeshLine consistent to Line.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Apr 16, 2017

Related issues: #10686, #10357

tomka and others added 3 commits April 30, 2017 22:43
This is needed to distinguish WebGLProgram instances that use mesh lines
from those that don't. This is done based on a so called program code,
which is, besides other things, based on the list of known parameter
values for the parameters names from the parameter list. If two
materials only differ in whether they use mesh lines and the one that
doesn't is loaded first, then without this change no new program will be
generated if the material with enabled mesh lines is loaded afterwards,
which effectively disables mesh lines for this material.
WebGLProgram: add meshLine to list of parameter names
@WestLangley
Copy link
Collaborator

When the number of points is reduced, there appears to be a problem with the implementation.

var spline = new THREE.CatmullRomCurve3( points );
var divisions = points.length / 6; // previously points.length * 6

screen shot 2017-05-10 at 2 33 07 pm

@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 10, 2017

I think a too small sampling rate causes issues when calculating the line extrusions in the vertex shader.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented May 17, 2017

see #11349

@Mugen87 Mugen87 closed this May 17, 2017
@Mugen87 Mugen87 deleted the meshline branch May 17, 2017 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants