Support thick lines #1889
Open
Support thick lines #1889
+176
−65
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/approve |
axelboc
commented
Oct 2, 2025
| extends BufferGeometry<Record<'position', BufferAttribute>> | ||
| implements H5WebGeometry | ||
| { | ||
| class LineConstantGeometry extends ThreeLineGeometry implements H5WebGeometry { |
There was a problem hiding this comment.
I recommend reviewing the diff of LineGeometry first 😉
axelboc
commented
Oct 2, 2025
| // Parts of `LineGeometry#setPositions` that don't need to be called on every update | ||
| // https://github.com/mrdoob/three.js/blob/40728556ec00833b54be287807cd6fb04a897313/examples/jsm/lines/LineSegmentsGeometry.js#L97 | ||
| const { array } = this.positions; | ||
| const instancedBuffer = new InstancedInterleavedBuffer(array, 2 * 3); // two sets of `xyz` coords per line segment |
There was a problem hiding this comment.
In LineGeometry, the positions buffer attribute has to contain line segments rather than connected points. It is wrapped in an InstancedInterleavedBuffer, which is exposed via two InterleavedBufferAttribute: instanceStart and instanceEnd, which provide LineMaterial with access to the start and end points of the line segments.
Comment on lines
+48
to
+51
| if (index >= this.length - 1) { | ||
| this.setInvalidSegment(posIndex); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Skipping the last data point, since it's the end of the second last data point's segment.
|
|
||
| const nextValue = ordinates[index + 1]; | ||
|
|
||
| if (ignoreValue?.(value) || ignoreValue?.(nextValue)) { |
There was a problem hiding this comment.
Don't render the segment either if:
- its start and/or end value is ignored
- its computed positions are not finite (see further down).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At long last, fix #1817
I've decided to completely replace
LineandLineBasicMaterialwithLine2andLineMaterialfromthree/addons/lines, since they no longer require installing a separate package and seem to now be official add-ons rather than just examples.Technically, since the viewer doesn't use this feature yet, there's a slight impact on memory with no actual gain in
@h5web/app— indeed, the position buffers are now twice as long because the new geometries (based onLineGeometry) require line segments rather than points. However, I think it's an acceptable compromise considering that the alternative would probably be to duplicate theLinecomponent and its two (for now) geometries.It's worth noting that by default,
LineMaterialassumes that the specifiedlinewidthis in CSS pixels, which means that it remains constant regardless of zoom level. This is exactly what we want, which is great. It's possible to change this behaviour by setting theworldUnitsboolean prop, but I didn't document it as I'm not sure there's a valid use case for it.Screencast.from.2025-10-02.12-46-18.webm
The new constant interpolation added a bit more work, but it was manageable.
So overall, the only consumer-facing changes for now are as follows:
[DataCurve, Line]Add newwidthprop to control the thickness of the line[DataCurve, Line]PropmaterialPropsnow accepts properties of Three'sLineMaterialinstead ofLineBasicMaterial[Line]Accept extra props from Three'sLine2instead ofLineEDIT I tried to add support for dashed lines. The idea is to compute the cumulative distances to the start and end of every segment. The problem, which we had identified a long time ago, is that these distances are in world space so the size of the dashes and gaps increase when zooming.
Line2solves this problem for the line width, butLineSegments2doesn't solve it for dashes... and I'm not fond of the idea of updating the geometry on every R3F frame, obviously. So I'm putting this work on the back burner.