docs: Rotating Triangle Tiles example #2048
Open
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
📊 Bundle Size Comparison
👀 Notable resultsStatic test results:No major changes. Dynamic test results:No major changes. 📋 All resultsClick to reveal the results table (331 entries).
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu. |
|
pkg.pr.new packages benchmark commit |
sharabai
force-pushed
the
feat/rotating-triangle-tiles-example
branch
from
f658ee6 to
5086460
Compare
sharabai
force-pushed
the
feat/rotating-triangle-tiles-example
branch
from
7b8d09b to
2ce6e6c
Compare
Contributor
aleksanderkatan
left a comment
aleksanderkatan
left a comment
There was a problem hiding this comment.
Great first example! 🦋
I tried to be educational in the comments, so many of them are not necessarily issues but stylistic preferences, nits or good practices.
If you have no time to address these comments, we can take over, no problem
| @@ -0,0 +1,49 @@ | |||
| // Generate by AI | |||
Contributor
There was a problem hiding this comment.
Could you find some human made implementation and reference the author instead? Or, if it's just some wikipedia formulas, just remove this line
Comment on lines
14
to
16
| const shiftedColorsBuffer = root.createReadonly(d.arrayOf(d.vec4f, 3), [ | ||
| ...colors, | ||
| ]); |
Contributor
There was a problem hiding this comment.
vectors already are arrays (with some extra steps)
Suggested change
| const shiftedColorsBuffer = root.createReadonly(d.arrayOf(d.vec4f, 3), [ | |
| ...colors, | |
| ]); | |
| const shiftedColorsBuffer = root.createReadonly(d.arrayOf(d.vec4f, 3), colors); |
Comment on lines
+4
to
+5
| "tags": ["experimental", "fragment shader", "instancing"], | ||
| "dev": false |
Contributor
There was a problem hiding this comment.
Suggested change
| "tags": ["experimental", "fragment shader", "instancing"], | |
| "dev": false | |
| "tags": ["experimental", "fragment shader", "instancing"] |
Comment on lines
+17
to
+19
| function getCubicBezierControlPoints() { | ||
| return cubicBezierControlPoints; | ||
| } |
Contributor
There was a problem hiding this comment.
Can this function be inlined?
Comment on lines
+91
to
+96
| const GridParams = d.struct({ | ||
| tileDensity: d.f32, | ||
| userScale: d.f32, | ||
| trianglesPerRow: d.u32, | ||
| triangleCount: d.u32, | ||
| }); |
Contributor
There was a problem hiding this comment.
I think this struct is unused
Comment on lines
+84
to
+85
| let calculatedPosition = rotate(vertexPosition, angle); | ||
| calculatedPosition = std.mul(calculatedPosition, scaleFactor); |
Contributor
There was a problem hiding this comment.
nit: we support infix operators for +-*/
Suggested change
| let calculatedPosition = rotate(vertexPosition, angle); | |
| calculatedPosition = std.mul(calculatedPosition, scaleFactor); | |
| const calculatedPosition = rotate(vertexPosition, angle).mul(scaleFactor); |
Comment on lines
+136
to
+138
| if ((e0 > 0 || e1 > 0 || e2 > 0) && drawOverNeighborsAccess.$ === 0) { | ||
| std.discard(); | ||
| } |
Contributor
There was a problem hiding this comment.
This drawOverNeighbors switch has made this shader a lot more complicated. Without this, we could probably simplify everything to only one straightforward pipeline.
I tried to come up with something to avoid passing so much flat interpolated data and without discard. We could theoretically create a static stencil like the following:
And use it for "odd" triangles, use a negative stencil for "even" triangles, and no stencil for the front layer. Maybe @reczkok can come up with something better.
Comment on lines
+11
to
+17
| const stepRotationAccess = tgpu['~unstable'].accessor(d.f32); | ||
| const shiftedColorsAccess = tgpu['~unstable'].accessor(d.arrayOf(d.vec4f, 3)); | ||
| const animationProgressAccess = tgpu['~unstable'].accessor(d.f32); | ||
| const middleSquareScaleAccess = tgpu['~unstable'].accessor(d.f32); | ||
| const drawOverNeighborsAccess = tgpu['~unstable'].accessor(d.u32); | ||
| const scaleAccess = tgpu['~unstable'].accessor(d.f32); | ||
| const aspectRatioAccess = tgpu['~unstable'].accessor(d.f32); |
Contributor
There was a problem hiding this comment.
tip: There is a pretty small limit of buffers that can be used without bind groups, usually it's only 12 (because this is the minimum limit for a single bind group, and those are bound under the hood). A simple way to avoid it is to group those into structs. In this case, it is not necessary, though it might clean up the code a little bit.
| aspectRatioBuffer.write(aspectRatio); | ||
| } | ||
|
|
||
| let animationDuration = 1000; |
Contributor
There was a problem hiding this comment.
This variable is kept between example switches (triangles -> sth else -> triangles). It's just a number so this probably does not matter.
| // Example controls | ||
|
|
||
| export const controls = { | ||
| 'Tile density': { |
Contributor
There was a problem hiding this comment.
In examples we usually start controls' names with lowercase letters (exception: buttons)
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.
Triangle tiles that rotate.