feat: add `<Fbo />` abstraction by kekkorider · Pull Request #228 · Tresjs/cientos

kekkorider · 2023-09-21T19:36:24Z

This is basically copypasted from Drei 😀

At the moment, when you head over /abstractions/use-fbo the console shows this error:

That's because here I'm using useTresContext() to get info about the size and viewport.
Any help on how to use useTresContextProvider() would be much appreciated 🥸

Closes #149

netlify · 2023-09-21T19:36:29Z

✅ Deploy Preview for cientos-tresjs ready!

Name	Link
🔨 Latest commit	`54b3730`
🔍 Latest deploy log	https://app.netlify.com/sites/cientos-tresjs/deploys/652165692da6af00087fa9ed
😎 Deploy Preview	https://deploy-preview-228--cientos-tresjs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

JaimeTorrealba · 2023-09-21T20:58:55Z

Since useTresContextProvider() is a provide/inject value, I believe you can't use outside setup context

Maybe we can convert this one into a renderlessComponent
You can base on the StatsGL component, for example.

JaimeTorrealba

Look good :)

JaimeTorrealba · 2023-09-21T20:59:43Z

+} & WebGLRenderTargetOptions
+
+export function useFBO(
+  width?: number | FBOSettings,


Width could be FBOSettings ?

That comes from Drei.

IMHO this is in case you do not specify the size (i.e. fullscreen) and you provide the settings object as first parameter.

JaimeTorrealba · 2023-09-21T21:01:17Z

-    :material-uniforms-mieCoefficient-value="props.mieCoefficient"
-    :material-uniforms-mieDirectionalG-value="props.mieDirectionalG"
-    :material-uniforms-sunPosition-value="sunPosition"
+    :material-uniforms-mie-coefficient-value="props.mieCoefficient"


This will break this abstraction @alvarosabu, what is needed here to use the linter propertly? a git pull??

Don't blame me, I simply ran pnpm lint --fix 😄

I know heheh this was an issue in others PRs too

@kekkorider please merge last main with the eslint over-rule and set the tmeplate of this file to:

<primitive :object="skyImpl" :material-uniforms-turbidity-value="props.turbidity" :material-uniforms-rayleigh-value="props.rayleigh" :material-uniforms-mieCoefficient-value="props.mieCoefficient" :material-uniforms-mieDirectionalG-value="props.mieDirectionalG" :material-uniforms-sunPosition-value="sunPosition" :scale="props.distance" />

@alvarosabu I just rebased and force-pushed (I'm used to work this way instead merging main).
After running pnpm lint --fix everything was ok as expected.

JaimeTorrealba · 2023-09-21T21:03:20Z

+  // const viewport = useThree((state) => state.viewport)
+  const viewport = { dpr: 1.5 }
+
+  const _width = typeof width === 'number' ? width : size.width * viewport.dpr


We could pass default values in the props or in the parameters,
but these types of validations (for type) are not necessary, for that is typescript :)

Yeah I was thinking the same thing, but I was having a lot of errors in the editor (despite everything was working).
So I copypasted this stuff as-is from Drei.

Let's make it works and then we see if we can clean something :)

kekkorider · 2023-09-21T23:23:55Z

Now we have something 🥸

fbo.mp4

alvarosabu

Excellent first contribution @kekkorider 👏🏻

Just change the naming convention to Fbo rather than useFbo since is not composable and change the file from a ts to a vue component to ensure better reactivity

alvarosabu · 2023-09-22T08:55:26Z

+  settings?: WebGLRenderTargetOptions
+}
+
+export const UseFBO = defineComponent<UseFboProps>({


Since this is a component rather than a composable I would probably avoid using use prefix and turn this file into a SFC vue component called <Fbo /> rather than a ts component.

This will make things easier for the props.

Also is not reactive, if any of the props change the target is not disposed and created again.

@kekkorider take a look at this component https://github.com/Tresjs/cientos/blob/main/src/core/staging/ContactShadows.vue let me know if you need help.

JaimeTorrealba · 2023-09-22T12:46:59Z

+      <TresBoxGeometry :args="[1, 1, 1]" />
+      <TresMeshBasicMaterial
+        :color="0xff8833"
+        :map="fboRef?.texture || null"


Question, this way of using is good, but it feels a little odd...

Since it is useful to use a renderless component, could this be made with slots?

Something like:

Is that possible, did you find comfortable? The FBO is use only in :map property?
What do you think, guys?

@JaimeTorrealba how would it look if done via slots for example?

In the code could be something like:

const slots = useSlots()
const material = slot.default()[0].children.material // and here you access the material of the slot component
//and you can assign the "texture" created directly, obviously some validations are needed first

Just by the record, I'm not demanding a change, you did this abstraction @alvarosabu you know what is better, is just I feel this way of usage a little weird, you don't. Other solutions are considered too

That's not what I mean actually, you mention passing the fboRef as a map it's odd. What would be the alternative?

const slots = useSlots()
const material = slot.default()[0].children.material

Here you are mentioning the possible implementation, what I mean is how would this look in the template? Like this?

<Fbo> <TresMesh> <TresBoxGeometry :args="[1, 1, 1]" /> </TresMesh> </Fbo>

kekkorider · 2023-09-26T09:04:28Z

I refactored everything to make it a renderless component following the new naming convention as @alvarosabu requested.

Basically it works, but whenever I change a prop the box turns black (no map set).
I'm logging the UUID of the FBO texture in the browser console and it stays the same no matter what.

JaimeTorrealba

@alvarosabu when you have time please check this one, when the components have WebGlRenders I couldn't make then reactive... Example of that is the Reflector, so please if you know a way to achieve this, comment us

alvarosabu · 2023-09-28T10:00:44Z

@kekkorider only missing the docs and add the composable as well in them (the composable needs to be used inside of a child component since it needs the context of TresCanvas)

kekkorider · 2023-09-28T14:50:56Z

@alvarosabu awesome! I will work on it next week. 🍻

kekkorider · 2023-10-05T12:49:52Z

@alvarosabu @JaimeTorrealba I guess we're there 🥳

alvarosabu · 2023-10-06T04:45:04Z

Hey @kekkorider amazing job! There seem to be some conflicts with main, could you please check it?

kekkorider · 2023-10-06T13:04:17Z

Hey @kekkorider amazing job! There seem to be some conflicts with main, could you please check it?

@alvarosabu I just rebased and removed the conflicts 🙌

JaimeTorrealba

Great work man, the component looks nice, I'll include it for the next v3.5 of cientos
Thanks you
I've just some little changes, docs related, and one question

kekkorider · 2023-10-06T16:53:11Z

@JaimeTorrealba I just fixed everything you asked 🙌

JaimeTorrealba

It has been a difficult one :D. But great work.

I'll not merge until @alvarosabu check this one but see everything fine

alvarosabu

Brilliant work @kekkorider @JaimeTorrealba

alvarosabu · 2023-09-27T06:49:09Z

+      <TresBoxGeometry :args="[1, 1, 1]" />
+      <TresMeshBasicMaterial
+        :color="0xff8833"
+        :map="fboRef?.texture || null"


That's not what I mean actually, you mention passing the fboRef as a map it's odd. What would be the alternative?

const slots = useSlots()
const material = slot.default()[0].children.material

Here you are mentioning the possible implementation, what I mean is how would this look in the template? Like this?

<Fbo> <TresMesh> <TresBoxGeometry :args="[1, 1, 1]" /> </TresMesh> </Fbo>

kekkorider added the abstraction label Sep 21, 2023

kekkorider requested review from JaimeTorrealba and alvarosabu September 21, 2023 19:36

kekkorider self-assigned this Sep 21, 2023

kekkorider changed the title ~~Add useFBO() abstraction~~ feat: add useFBO() abstraction Sep 21, 2023

JaimeTorrealba suggested changes Sep 21, 2023

View reviewed changes

kekkorider force-pushed the feature/149-add-useFBO branch from 4a632f0 to f3a00a1 Compare September 22, 2023 08:50

alvarosabu suggested changes Sep 22, 2023

View reviewed changes

JaimeTorrealba reviewed Sep 22, 2023

View reviewed changes

kekkorider changed the title ~~feat: add useFBO() abstraction~~ feat: add <Fbo /> abstraction Sep 26, 2023

kekkorider requested review from JaimeTorrealba and alvarosabu September 26, 2023 17:33

JaimeTorrealba reviewed Sep 27, 2023

View reviewed changes

kekkorider force-pushed the feature/149-add-useFBO branch from c17039a to 023422b Compare October 4, 2023 22:05

kekkorider marked this pull request as ready for review October 5, 2023 12:44

kekkorider requested a review from JaimeTorrealba October 5, 2023 12:44

kekkorider added 6 commits October 6, 2023 13:27

[broken] add useFBO

f8e3cf3

Use useTresContext in useFBO

e97cb3d

Lint

97ba472

Refactor to renderless component

9a97aaa

Lint

5e8e04e

Setup docs page

9bb8483

kekkorider and others added 14 commits October 6, 2023 13:29

Refactor to renderless component

b37bd13

Test with Alvaro

3c74131

fix: check for .value in depth check

bbd087e

fix: depth was setting depthBuffer

404b182

feat: added sizes as default if width and height are not explicity set

ca56d65

feat: add composable + component approach

4e0408b

chore: forcing rename of folder for casing issues

8767bfa

chore: rename back

7ee0a18

Update docs

4971fb8

Finalize docs of <Fbo />

e7b334b

Finalize docs

eccead2

Fix default values for width and height

789629c

Fix typo

07a4a11

Update global components

81acaed

kekkorider force-pushed the feature/149-add-useFBO branch from 48749bd to 81acaed Compare October 6, 2023 13:02

JaimeTorrealba suggested changes Oct 6, 2023

View reviewed changes

Comment thread docs/guide/abstractions/fbo.md Outdated

Comment thread docs/guide/abstractions/fbo.md Outdated

Comment thread docs/guide/abstractions/fbo.md Outdated

Comment thread docs/components.d.ts Outdated

kekkorider added 5 commits October 6, 2023 16:48

Remove docs/components.d.ts

49c717c

Use separate pages for <Fbo /> and useFBO

0add19d

Import demos

33002ae

Lint UseFBODemo.vue

2a5da98

Refactor UseFBODemo

38e76aa

kekkorider requested a review from JaimeTorrealba October 6, 2023 16:53

Add warning about usage in child component

ac994e0

JaimeTorrealba approved these changes Oct 6, 2023

View reviewed changes

JaimeTorrealba reviewed Oct 6, 2023

View reviewed changes

Comment thread docs/guide/abstractions/use-fbo.md

chore: fix route for examples on playground and color of cube

54b3730

alvarosabu approved these changes Oct 7, 2023

View reviewed changes

alvarosabu merged commit 6efc076 into main Oct 7, 2023

Uh oh!

Conversation

kekkorider commented Sep 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Sep 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for cientos-tresjs ready!

Uh oh!

JaimeTorrealba commented Sep 21, 2023

Uh oh!

JaimeTorrealba left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kekkorider commented Sep 21, 2023

Uh oh!

alvarosabu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kekkorider commented Sep 26, 2023

Uh oh!

JaimeTorrealba left a comment

Choose a reason for hiding this comment

Uh oh!

alvarosabu commented Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kekkorider commented Sep 28, 2023

Uh oh!

kekkorider commented Oct 5, 2023

Uh oh!

alvarosabu commented Oct 6, 2023

Uh oh!

kekkorider commented Oct 6, 2023

Uh oh!

JaimeTorrealba left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kekkorider commented Oct 6, 2023

Uh oh!

JaimeTorrealba left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alvarosabu left a comment

Choose a reason for hiding this comment

kekkorider commented Sep 21, 2023 •

edited

Loading

netlify Bot commented Sep 21, 2023 •

edited

Loading

alvarosabu commented Sep 28, 2023 •

edited

Loading