feat: lensflare#185
Conversation
✅ Deploy Preview for cientos-tresjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
JaimeTorrealba
left a comment
There was a problem hiding this comment.
First let me tell you thanks for this amazing contributions, I believe almost all the changes are just minimal stuff
Seems really cool, the effect
Even so, in addition to the requested changes:
All the props should be reactive,
I was trying to changes the seeds from 0 to 10000, but I can't see any visual changes
Have you tried adding your own textures¿?
We could consider clean a little to the component and extract some logic into other files, what do you think?
The final result could be something like:
abstractions/
LensFlare/
index.vue
Utils (or any other name) // And here put some logic so we
Sorry for the time to response is a big component, but I'm excited to add this to Cientos
Also, I'm on the official TresJs Discord for a more quick respond
|
|
||
| const lerp = THREE.MathUtils.lerp; | ||
|
|
||
| const TEXTURE_PATH = 'https://raw.githubusercontent.com/andretchen0/tresjs_assets/' + |
There was a problem hiding this comment.
Send me the PR for this in the assets repo I'll approve it :)
There was a problem hiding this comment.
Sure thing. I might need to change some images so I'll keep this in my fork for now. But once it's ready for review, I'll submit a PR to the assets repo.
Thanks for the feedback. I'll go through it and make the changes. |
|
I've made most of the requested changes above. Let me know about the remaining issues and we'll move forward. You can check out the playground at: http://localhost:5173/abstractions/lensflare Fwiw, I had earlier included |
JaimeTorrealba
left a comment
There was a problem hiding this comment.
I like the re-organization of the files, we're close to finish this :)
New props: color, distance, size, textureAs mentioned, the user can now put these props directly on the component: color, distance, size, texture. They serve as defaults.
It's handy in a case like this: Or: Edit: removed the edge cases from the code. |
|
I've added the docs at http://localhost:5173/guide/abstractions/lensflare.html Still waiting on Tresjs/assets#7 but once that's merged and the texture paths are updated here, this is ready to go, unless there are more requested changes. |
|
@andretchen0 perfect, I already approved the Tresjs/assets#7 |
|
I've updated the paths to point at Tres Assets. The Tresjs/assets readme says:
I use those assets as defaults for the Lensflare. That means they will likely be used in Cientos' users' software. Is that ok, or should they be placed somewhere else? |
|
Yes, it's fine to use the assets as textures in our components BTW all the images you have used are free of charge right? There is no license problem on them |
|
I have to say, incredible work!!! It's a huge contribution When you feel ready, please remove the draft-status and let's merge it Also, if you want, the demo component for lensFlare will be really useful in the Playground let me know if you would like to add it there (after finalizing this PR of course) |
Yes. I made them using GIMP and the canvas API. They are free to use and unencumbered by licenses. |
Sure, that would be great. Would you like me to fork the playground repo? |
|
Yes, but you're now part of the Tres org right? |
|
Oh, sorry. I had to fork tres-assets because I wasn't part of that team. Ok. I'll just create a branch then. Thanks! |
@JaimeTorrealba
Here's the Lensflare as it stands, for discussion.
Playground
If you start the playground, the lensflare demo is at http://localhost:5173/abstractions/lensflare
If you refresh the demo, the large "star" in the middle gets a new seed, so it'll look slightly different.
Lensflare
src/code/abstractions/Lensflare.vuecould shed about 200 LOC if we dropped the seedable pseudorandom generation. About 150 LOC is setting up then selecting the seeded pseudorandom defaults.But those 200 lines make it a lot easier for the user to add a Lensflare.
As mentioned here, a user can currently do this
and it'll produce a Lensflare with about 25 elements based on the seed – always the same for the same seed.
This
will replace the seeded first element's texture with the image at the passed url.
If we remove the pseudorandom generation, the user will have to do something like:
Note
The THREE Lensflare isn't really meant to be updated. It makes its elements private.
I've made the elements reactive to prop changes by hanging on to their references, but updating them is a rather slow process. User input has to be merged with seeded defaults or a default element, then that has to be merged with the THREE Lensflare's elements.
It may be slow if done every frame. Since that's not the way the THREE implementation appears to be meant to work, I didn't worry about it.
RandUtils
For
src/utils/RandUtils, I reused the parts of THREE'sMathUtils, but made them use the seedable pseudorandom generator. If a module usesRandUtilsand always calls methods the same way, it'll always produce the same numbers, array samples, etc. I think that could be pretty handy for a project like Cientos, but I don't want to force it on anyone. FWIW, I got the idea from Apple Motion, which takes random seeds for a lot of its objects.