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

feat(Sparkles): add Sparkles #253

Merged
merged 14 commits into from
Oct 26, 2023
Merged

feat(Sparkles): add Sparkles #253

merged 14 commits into from
Oct 26, 2023

Conversation

andretchen0
Copy link
Contributor

@andretchen0 andretchen0 commented Oct 14, 2023

Screen.Recording.2023-10-14.at.18.54.14.mov

Here's a look at <Sparkles> to get your feedback.

Playground

Playground available at:

pnpm run playground

[...]/staging/sparkles

Sparkles

Thoughts on <Sparkles /> visuals, props, etc. are welcome here.

Gradients

See below

ShaderData

See below

@stackblitz
Copy link

stackblitz bot commented Oct 14, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link

netlify bot commented Oct 14, 2023

Deploy Preview for cientos-tresjs ready!

Name Link
🔨 Latest commit 173f481
🔍 Latest deploy log https://app.netlify.com/sites/cientos-tresjs/deploys/65399550bef8a700089c62d3
😎 Deploy Preview https://deploy-preview-253--cientos-tresjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@andretchen0
Copy link
Contributor Author

andretchen0 commented Oct 14, 2023

Gradient<T> discussion

Looking for feedback.

On Discord @alvarosabu asked to continue discussion of Gradients here.

Overview

Sparkles uses a new data type, Gradient<T>. Please have a look: /src/staging/Sparkles/Gradient.ts

You can find them used as props in /src/stage/Sparkles/component.vue. These are all gradients, for example:

  colorSequence: () => ['purple', 'blue', 'green', 'red'],
  alphaSequence: () => [[0.0, 0.0], [0.10, 1.0], [0.5, 1.0], [0.9, 0.0]],
  offsetSequence: () => [[1.0, 1.0, 1.0]],
  surfaceDistanceSequence: () => [0.05, 0.1, 0.15],
  sizeSequence: () => [0.0, 1.0],
  noiseSequence: () => [1.0, 1.0, 1.0],

Types of Gradient

In keeping with Tres' flexible params, it accepts a few different types:

  • T - a single value
  • [T, T, T, T] - values at evenly spaced stops
  • [[number, T], [number, T], [number, T]] - values at defined stops

So you could set a color gradient like this:

  • 'red'
  • ['red', 'green', 'blue']
  • [[0.1, 'red'], [0.2, 'green'], [0.9, 'blue']]

Respectively, those would look like this if represented graphically:

Screenshot 2023-10-14 at 19 13 58

Flexibility

The inputs are flexible, but there's a fair amount of code necessary to disambiguate and normalize the gradients, e.g., functions like, normalizeFlexibleVector3Gradient. I think that's ok, as we already have functions like normalizeVectorFlexibleParam, but I thought it was important to point out.

Order

Note that for defined stops, the order is:

stop position, value

This is how the CSS canvas addColorStop is ordered., though I find it a bit counter-intuitive personally.

@andretchen0 andretchen0 changed the title feat(Sparkles) feature(Sparkles) Oct 14, 2023
@andretchen0 andretchen0 changed the title feature(Sparkles) feat(sparkles) Oct 14, 2023
@andretchen0 andretchen0 changed the title feat(sparkles) feat(Sparkles): add Sparkles Oct 14, 2023
@andretchen0
Copy link
Contributor Author

ShaderData discussion

This is another rather large module created for <Sparkles />, but potentially useful elsewhere, so I'd like your feedback.

<Sparkles /> needs a way to send Gradient props to its shader. I've taken this approach.

On startup and for each prop change:

  • Convert Gradient<T> to rgba(T.x, T.y, T.z, 255), where all values are Uint8's – 0 to 255
  • For each prop:
    • Use the HTML canvas' addColorStop gradient to add the gradient data
    • fill() the next row on the canvas with the color gradient
  • Update a THREE.texture with the canvas' imageData
  • Update the shader uniform

In the shader:

  • Sample the texture and convert the data back to the (more or less) original values.

So, there's a lot going on.

For setup, I've included a builder. It's my hope that this makes it easier to guide the user through setup using Intellisense, as only a few choices are available in each step:

Untitled.mov

The class-based interface that underlies the builder is also available for those who prefer it:

new ShaderData([
  new ShaderDataEntryGradientScalar(refs.alphaSequence, 'alphaSequence', 0, 1),
  new ShaderDataEntryGradientTresColor(refs.colorSequence, 'colorSequence'),
], 256)

In the shader

texture

ShaderData has a useTexture method that sends back a reactive texture.

ShaderProps

ShaderData.useTexture() also sends back a ShaderProps: an object that makes it easier to retrieve data from the texture in the shader. Here's an example:

TS:

const { texture, shaderProps } = new ShaderDataBuilder().add.[...].build().useTexture('uInfo')

GLSL string in TS:

`
float surfaceDistance = ${shaderProps.use('surfaceDistanceSequence').at('mix(a, b, t)').get()};
`

which gets string substituted into this GLSL:

float surfaceDistance = ((texture2D(uInfo, vec2(mix(a, b, t), 0.009765625)).x));

Why bother?

Without ShaderProps, the user must know:

  • the texture row number for the pixels for 'surfaceDistanceSequence'
  • the total number of rows in the texture
  • the 'dimensions' of the original data
  • how to convert from a 0-1 value back to the original scale/offset of 'surfaceDistanceSequence'.

That's a real pain, and it's why I took the time to write ShaderProps.


Feedback

Any feedback you have is welcome. This is a lot of code. It could surely use revisions and DX improvements. I'd also be open to taking a different approach here, if someone knows of a better one.

this.resolution = resolution
}

useTexture(uniformName = 'uInfo') {
Copy link
Member

Choose a reason for hiding this comment

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

Is the param uniformName gonna be used?

Copy link
Contributor Author

@andretchen0 andretchen0 Oct 16, 2023

Choose a reason for hiding this comment

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

Yeah. It's used for the ShaderProps methods. For example, here:

gl_FragColor = ${shaderProps.use('colorSequence').at('vProgressColor').getRaw()};

Behind the scenes, shaderProps uses uniformName to sample the texture. Here's what ends up in the shader:

gl_FragColor = (texture2D(**uInfo**, vec2(vProgressColor, 0.001953125)).rgba);

If there's no name set, then ShaderProps doesn't know where to look for the texture uniform.

}
}

class Texture {
Copy link
Member

Choose a reason for hiding this comment

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

mmm, I would probably name it different to avoid conflicts with three Texture class, would be possible to extend the three one with this implementation our they are completly different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point.

They're different. I'll figure out a different name.

export type GradientScalar = Gradient<number>
export type GradientVectorFlexibleParams = Gradient<VectorFlexibleParams>

export function normalizeColorGradient(
Copy link
Member

Choose a reason for hiding this comment

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

This utils are gold, 🤔 @andretchen0 do you see them usable in other context than the sparkles?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you see them usable in other context than the sparkles?

I can imagine some, but we should probably wait until we have some concrete needs before making Gradient<T> available.

At the moment, the module is just a few types and functions. To be generally useful, it'd probably need to offer some classes, otherwise, users have to carry the NormalizeConfig from Gradient around.

Representing:

const g:Gradient<number> = [999, 1000]

Normalizing to the form [ [ stop, value ], [ stop, value ], ... ]:

normalizeScalarGradient(g) === [[0.0, 999], [1.0, 1000]]

General use

To be generally useful, it'd probably need some classes. For a TresColor gradient, that might look like:

new GradientColor(unnormalized:Gradient<TresColor>)

class GradientColor would encapsulate the NormalizeConfig<TresColor, Color> from Gradient.ts. Otherwise, users have to carry the NormalizeConfig around, or know where to find it.

Then GradientTresColor could offer a get(stop:number) => Color, which would be useful generally, I think.

I didn't do that here because it requires an extra class and an extra step for get. Those weren't necessary for Sparkles.

Currently

Because Sparkles just needs an image texture, it just skips the general get<T>(stop) above and uses the canvas API for rasterization.

But that means converting all input values to canvas rgba(255, 255, 255, 255) for storage in the texture, then converting back when reading the texture. (The ShaderProps class handles those conversions for Sparkles.) The conversion to rgba and back results in data loss that's fine for Sparkles but probably not for some other applications.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved Gradient to utils.

I think we should hold off on ShaderData until there are other needs. At least, it's not entirely clear to me exactly what it should look like in the end. If we have another use, maybe that will clarify things.

Copy link
Member

@alvarosabu alvarosabu left a comment

Choose a reason for hiding this comment

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

Regarding Gradient #253 (comment)

Looks powerful but I see it actually as something usable in way more other use cases than just Sparkles, for now I would take it out of the sparkles directory and add it to utils or at core level. (same for get2DCanvas)

We also should probably add unit tests to these ones.

What is your opinion on this, do you think it could be used in future abstractions?

Regarding ShaderData #253 (comment)

I'm not gonna lie, I understood half of it 😅 but I understand its purpose, IMHO it would facilitate future shader's related work so following the same idea as the previous topic, do you see it usable outside of the context of Sparkles?

If yes, maybe makes sense to take it out of the sparkles directory and add it to utils or at the core level as well

The demo is amazing, I fell in love with the effect. I guess I will wait for the docs to understand all the parameters well.

@andretchen0
Copy link
Contributor Author

Regarding Gradient #253 (comment)

Looks powerful but I see it actually as something usable in way more other use cases than just Sparkles, for now I would take it out of the sparkles directory and add it to utils or at core level. (same for get2DCanvas)

We also should probably add unit tests to these ones.

What is your opinion on this, do you think it could be used in future abstractions?

I think they can be generally useful, but I'd like to have some more concrete cases before making them more general. I can imagine a few different approaches to generalizing Gradient<T>, but each approach involves tradeoffs – extra space, slower execution, lossy data. So I guess I'd like to see what the real needs are before generalizing.

Regarding ShaderData #253 (comment)

I'm not gonna lie, I understood half of it 😅 but I understand its purpose, IMHO it would facilitate future shader's related work so following the same idea as the previous topic, do you see it usable outside of the context of Sparkles?

Yeah, it's a big chunk of code. I think it's generalizable, but it's probably best not to generalize until there's another concrete use.

The demo is amazing, I fell in love with the effect. I guess I will wait for the docs to understand all the parameters well.

Good to hear! I'll work on the docs then!

src/core/staging/Sparkles/get2DCanvas.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,23 @@
export default function get2DCanvas(width = 256, height = 256) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

get2DCanvas sounds like an existing canvas is returned. But a new one is created. Shall we call this function make2DCanvas? What do you think? This might be a personal thing 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make2DCanvas is fine with me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fwiw, I removed this function altogether and created a <canvas /> inline.

@andretchen0
Copy link
Contributor Author

andretchen0 commented Oct 23, 2023

We also should probably add unit tests to these ones.

That's a good idea, but I haven't added the unit tests yet. There's currently no testing framework in Cientos, as far as I can see.

I see that Leches uses vitest. Would you like me to add that to Cientos and submit a PR?

Regarding ShaderData #253 (comment)

I removed a lot of the code from ShaderData – notably the ShaderProps class and various helpers that I was using to add strings to the shader. The result is that there's a lot less "magic" and less code – so hopefully a good thing, in the end.

I guess I will wait for the docs to understand all the parameters well.

Docs are now available: guide/staging/sparkles.html

@andretchen0 andretchen0 marked this pull request as ready for review October 23, 2023 11:35
@alvarosabu
Copy link
Member

Awesome thanks @andretchen0 regarding unit test. Let's add vitest in the scope of another PR and then create a follow-up tasks to add test to this feature

@alvarosabu
Copy link
Member

Screenshot 2023-10-24 at 16 39 43

@andretchen0 mmm, the demo is broken on Netlify, do we need to build the lib before?

@andretchen0
Copy link
Contributor Author

andretchen0 commented Oct 24, 2023

@andretchen0 mmm, the demo is broken on Netlify, do we need to build the lib before?

Thanks. I'll check it out!

@andretchen0
Copy link
Contributor Author

@alvarosabu

Ok, things are working on Netlify now.

I'm not sure exactly what changed. I deleted node_modules and dist, and ran

pnpm i && pnpm run build && pnpm run docs:build && pnpm run docs:preview

Strangely, pnpm run docs:dev didn't show the same problems locally.

@alvarosabu
Copy link
Member

True locally they were working

Copy link
Member

@alvarosabu alvarosabu left a comment

Choose a reason for hiding this comment

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

Amazing job @andretchen0 can't stop playing with it hahaha

@alvarosabu alvarosabu merged commit cbcc9b9 into main Oct 26, 2023
6 checks passed
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.

3 participants