onBeforeCompile prop of material doesn't work

[enpitsuLin]

Describe the bug

For class Material we have onBeforeCompile method, R3f provide onBeforeCompile prop to use that (example), and I think Tresjs should have something like this.

I tried to use @beforeCompile will warn

and onBeforeCompile both don't work, I guess this might be a bug or this is a feature doesn't implement yet?

I provide a reproduction with a Codesandbox copy from the official playground, just run and see no stdout which should have

Reproduction

https://codesandbox.io/p/sandbox/hopeful-goodall-y86d74?file=%2Fsrc%2FApp.vue%3A44%2C9

Steps to reproduce

No response

System Info

No response

Used Package Manager

npm

Code of Conduct

• [X] I agree to follow this project's Code of Conduct
• [X] Read the Contributing Guidelines.
• [X] Read the docs.
• [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• [X] The provided reproduction is a minimal reproducible example of the bug.

[alvarosabu]

Try <Material on-before-compile="[...args]" />

[enpitsuLin]

@alvarosabu onBeforeCompile is a function that was not iterable

I assign it to the property manually then it works.

watchPostEffect(() => {
  if (materialRef.value) materialRef.value.onBeforeCompile = onBeforeCompile;
});

[alvarosabu]

@Tinoooo would make sense that we map on events to native vue events?

[Tinoooo]

@Tinoooo would make sense that we map on events to native vue events?

@alvarosabu Yes, I think this could be a handy feature ☺️

[ThimoDEV]

@Tinoooo , @alvarosabu Where would I have to start searching to map those events. Is that a responsibility of the custom renderer?

[andretchen0]

Just chiming in.

Ran into this as well, but in my case with onBeforeRender. I think it'd be handy to be able to set the callback via props.

[andretchen0]

@ThimoDEV

Where would I have to start searching to map those events.

I'd start with patchProp in src/core/nodeOps.ts.

[andretchen0]

Looking into this, with a scene like ...

<Object3D :on-before-render="console.log">
  ...
</Object3D>

... and stepping through src/core/nodeOps.ts, you end up at this line.

Here it is in the debugger, with some watch expressions on the right showing the pertinent values:

It's calling ...

node[finalKey](value)    // i.e., object3d['onBeforeRender'](console.log)

... but what's desired here (at least in this issue, if not generally) is setting ...

node[finalKey] = value   // i.e., object3d['onBeforeRender'] = console.log

Context

Looking through the git history, that chunk of code has been calling rather than setting ever since it was added to the codebase. Here are the relevant commits:

• https://github.com/Tresjs/tres/commit/fa072cddc0fca4c1862a49fa2877d3ef5c96dbd2
• https://github.com/Tresjs/tres/commit/c07d963bd50910f9df519db05ed7f1a496ff03cc

Questions

• @alvarosabu , do you happen to remember what the use case is for calling here, rather than setting?
• If there's a use case for calling, can we carve that out with a conditional and otherwise set? Or vice-versa?

[andretchen0]

Context

I came across at least one common use case for calling here.

Something like <PerspectiveCamera :look-at="[1, 2, 3]" />. Under the hood, that needs to call lookAt().

Notably, this doesn't appear to work in R3F/Drei. In R3F, you have to get a reference to the camera and call lookAt(). That's probably a more coherent approach, since templates are supposed to be declarative.

But since we're already calling we should probably continue to call, at least where it's obvious that setting isn't desired.

Fix?

To accommodate setting functions, in src/core/nodeOps.ts, we could do something like changing this:

      if (isFunction(target)) {
        // don't call pointer event callback functions
        if (!supportedPointerEvents.includes(prop)) {
          if (Array.isArray(value)) { node[finalKey](...value) }
          else { node[finalKey](value) }
        }
        return
      }

... to ...

      if (isFunction(target)) {
        // don't call pointer event callback functions
        if (!supportedPointerEvents.includes(prop)) {
          if (isFunction(value)) {
              // NOTE: Both target and value are functions. Set target to value.
              node[finalKey] = value
          }
          else if (Array.isArray(value)) { node[finalKey](...value) }
          else { node[finalKey](value) }
        }
        return
      }

Breaking

That will break code like:

<Object3D :traverse="myTraverseCallback" />

But to me anyway, this clearly not declarative and doesn't belong in a template.

However, if we wanted to, we could work around it for this case and the other on... callbacks by being more specific with the predicate:

          if (isFunction(value) && finalKey.startsWith('on')) {
              node[finalKey] = value
          }

That keeps almost all of the current functionality in place, but it seems overly specific, making it hard to intuit without digging into the source.

---

Feedback?

Any thoughts here?

[enpitsuLin]

So it's some thing like that we are calling lootAt() and traverse() by props rather method on instance, is there other case like that?

we could work around it for this case and the other on... callbacks by being more specific with the predicate:

I think it's best solution.

In my opinion, almost use case about on... property in Object3D should be used to be assigned, not to calling them, it was a callback runs inner that can be override by user

[andretchen0]

So it's some thing like that we are calling lootAt() and traverse() by props rather method on instance, is there other case like that?

I haven't found others exactly like that in Mesh. But similar to that is:

<TresMesh :position="[1, 2, 3]" />

In order to pretend that's declarative, we have to call rather than set behind the scenes. Something like:

mesh.position.set(... [1, 2, 3])

and not

mesh.position = [1, 2, 3]

[andretchen0]

I think it's best solution.

I'll submit a PR.

[enpitsuLin]

seem done, it's closed