Toggle v-if on a Tres component declared in a separate SFC makes it detach from its parent

bperel commented 5 months ago

Describe the bug

I have a scene where the rendering of a pyramid TresGroup is conditioned by a boolean named show, using v-if. This TresGroup is the root node of a SFC, and this SFC is included in the root scene within another TresGroup named mainGroup.

When I toggle the show boolean, I would expect the pyramid to be rendered within its parent group as the show value is toggled on and off. Instead, on the first render the pyramid is rendered within its parent as expected, but on the subsequent renders it gets detached from it and is instead rendered within the root scene, causing its position to be wrong because the group that it's supposed to belong to has a position offset. When adding a watch:

watch(
    () => props.show,
    () => {
      if (groupRef.value) {
        console.log(
          "My parent is",
          groupRef.value.parent.type,
          groupRef.value.parent.name
        );
      }
    }
  );

I can indeed see that the parent of the pyramid changes:

My parent is Group mainGroup
My parent is Scene <empty string>
My parent is Scene <empty string>
My parent is Scene <empty string>
...

If the pyramid TresGroup is directly declared within the root canvas element, the pyramid is rendered within its group as expected, and the console logs show that the parent of the pyramid didn't change indeed (link):

My parent is Group mainGroup
My parent is Group mainGroup
My parent is Group mainGroup
...

Reproduction

https://play.tresjs.org/#eNqVVm1v2zYQ/iucgiEOZkvKS7NOc4IsbhF0WNogybeqH2iJtphQJEHSdozA/31HUpKptGlcw4DNu4fPHe+OvHuO7hXR/0gZLxckyqKxLhSVBmliFhIxzOdneWR0Hp3nnNZSKIOe0SXWtLircClW11gO0d3t1eVEMKHuJC7IEH0W94ITUEnK52iDZkrUaN9UipD9vwMea3qC+RLrDnRhQPagk0KoPvaLmlIzEdwowX4Ap4QboXs7FJl1ODhcoLtZK1zTstHFSbO2IdjPec4LwbVBskGdWabBc2EPmKH9KQPUEOlKrDJk1IJsDuweCNgnbohaYjYYHKCzc/Sc84YiBuGCxHYLsP32vXQzRIdpmjoib3zOAAkMCBWMYOWiC8b33h99uJy82x9ajXYZ0N4LJ8FMVjhDM8y0F3gIZOJ+LUn2MnEWIRZGLiCwbfayl9m0ILNNZ9bPLqg3ENhx4usGqgQWhtSSYUNghdA4yPJyNKW8hIqaM1dRCD5Of0OUlqQwdEkmuCYKo0wKTQ0VHNBf/xoi+/2WRyhpt/ULohNbtisloHh7DKOjIYJvaik4WABZjSl3SBA1uzuGa6KrQNZIL8XTFRE1MWqNMqzm2jIfQu7gC8Tn4+QFKmQ474jvheDXEB5FMQsRkGsbdiBldF4ZW2h51AcYhbmWWEG19xWZgGRRs4bNafy+v63x63XTASAMRHtNstattnDdGqKW2doN5HYZpsixuhCH2flAlc204Jj9Z8/ZT1Tq8nRi85RRuFAcVPZUh/ERiAqszcgXdWPGG/H1BYJxEhQfLLVZM/u3MjWDWp2Kcu2vVQ35ozxDKVQvQhKXpStuv6yIdSyzl/J3J1jR0lTdepPzPSh/T/QGFqEpLh7nEARejpo3ZA/uuqeBe+McjIaRf5tGNZbxgxYc3mJHnzcKeIIzb9DKwmfSKvKoMkbqLEmKksP+kjC6VDEnJuGyTkL4xXF8Gh8mJdUmsWJA55G75T1i/6D+CrffAfR/xumWvhH3rZgVwY8S811877AXJ3HaOd4K45raPSG3bTO78FrcBTCepuDvH0TXYRigFSw02Tm+AfziMI3fASM8dOQprvvRbXBQwYqUv0DsN/yMGoCjktR0B9IWag9/Ep8mjE5DSssIpbmBmjQautGMzl9UZCFqSRlRX6S9tf3KxIyJ1b9O1rUlt6cixeMP5A/6ybt8A8UC7RPCvU0l3FFivPrj3WfyBP87ZS3KBWuS84rylmjBFtZHD7uEOwhuBzjn7Sd3weD63+uPT+7FaQ7Vc9RAB3XSr+0T2dWaF3xzgXN4l47JT6K0PdlxfBwEPJhE3hjGEAoHnSFaYVNU7bzj7OcRvDFugvDjjBJSw1BRkhnl5Mauxo0/fpaZCgGjBm8O3DxV2ijf5cHF88FBj9E+avIWpiw/IbVK58nAs/hJyJl23aEh7wYkH0g6Q4OWzE9FB1ultwauxUzMG9o2/tdreLltM0R0exf8p88Xe1hsk7gDzE4IW5Q9mP8HWep+rPStsccPIssRnUHimu4IobIDUGO3Pwb1OrAfnGDUem3mOA7noYAhbPLb5t027X537iz2m3WvjUab/wFDwfX9

Steps to reproduce

No response

System Info

TresJS playground
Both TresJS 3 and 4 have the issue

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.

andretchen0 commented 5 months ago

@bperel Thanks for opening these issues!

Since you've been actively filing issues, it would be especially helpful to have the reproductions in a StackBlitz rather than a playground. Among other things, StackBlitz makes running/testing against the reproductions much easier, so we can spend more time on bugs.

bperel commented 5 months ago

@bperel Thanks for opening these issues!

Since you've been actively filing issues, it would be especially helpful to have the reproductions in a StackBlitz rather than a playground. Among other things, StackBlitz makes running/testing against the reproductions much easier, so we can spend more time on bugs.

Alright, I'll try to create the repro in Stackblitz this weekend

Sea-DH1 commented 5 months ago

This is created based on the demo above.

https://stackblitz.com/edit/stackblitz-starters-ntyud7?file=src%2FApp.vue

andretchen0 commented 5 months ago

Thanks to both of you! @bperel @Sea-DH1

bperel commented 5 months ago

Thank you @Sea-DH1 ! And here is the link to the working alternative: https://stackblitz.com/edit/stackblitz-starters-6u9dcf?file=src%2FApp.vue

garrlker commented 5 months ago

Thank you for the reproduction! I was able to figure out what was going on and I've created a PR(#717) with the fix

special thanks to @andretchen0 He asked a question about this in discord that lead me to the solution

garrlker commented 5 months ago

@bperel @Sea-DH1

The PR has been merged and a fix for this issue will go out in the next release

Thanks again for the issue and repro!

garrlker commented 5 months ago

@bperel @Sea-DH1 The fix was released this morning is @tresjs/core version 4.0.2

Please give that a try and report back when you get a chance 🙂

bperel commented 5 months ago

@bperel @Sea-DH1 The fix was released this morning is @tresjs/core version 4.0.2

Please give that a try and report back when you get a chance 🙂

I confirm that it's fixed! Thank you so much for the quick resolution :-)

Tresjs / tres