Closed dennemark closed 2 years ago
it's better here in issues, because i keep losing track of the discussions and i try to keep issue count down! :)
I will definitely add that. I see what you mean as the createReconciler
call is buried in Scene, so then it needs to be moved out. I'll need to look at how react-three-fiber exposes their reconciler and allows it to bind to multiple scenes in that kind of global way. Another option is to put it in the context and add a hook that can access it. I wouldn't mind moving away from context since it has other issues at the same time. I'm ok with any breaking changes in that area as I am ready for a v4 anyway with some new ideas for tree shaking. Do you have a code proposal - I will look more at the code tomorrow. I had looked previously into createPortal for DOM for the canvas already as you may have seen in Engine.tsx.
Thanks for bringing this up, that will be a cool addition. You have lots of good ideas - wish I had more time to devote to them!
I am wondering if the reconciler in r3f can live outside react/scene, because threejs in general is not limited to geometries being in a specific scene (we discussed this somewhere else already), so it might be correct, that you create the reconciler together with the scene. - it seems to work also outside of the scene, but i am not sure what happens, if we create multiple scenes. might still work. the advantage would be, that we could have a createPortal function. Thats how it is done within r3f. If however, the reconciler lives within Scene, we would need to expose it via a hook, which is different to the react api.
I thought it could be something like the following code. However, I wasn't able to expose createPortal to use it within storybook. And when I found a workaround, there were the complains, I mentioned in my first post. But I guess it should be a good direction.
import {Node} from "@babylonjs/core/node"
import {ReactNode} from "react"
const reconciler = createReconciler({})
export const createPortal = (children: ReactNode, container: Node) => return reconciler.createPortal(children, container, null, null);
const Scene = ( ) => { the react component.... }
Concerning the context. I am often using zustand. Not sure though if it is usefule for this case, since it lives in global state. But the libraries of daishi (jotai, valtio, zustand) are all pretty nice concerning re-rendering. Not sure if it is necessary for the scene context though, since the scene itself, once created, would not trigger re-renders right?
Thanks for bringing this up, that will be a cool addition. You have lots of good ideas - wish I had more time to devote to them!
Would be nicer if I could make a pull request, instead of only writing down ideas ;) But it came out of a demand and saw that react-three-fiber has it. They have many nice ideas. I was trying to convert the HTML overlay to babylonjs. Still needs some refinement, I hope I can push it soon.
I was trying to convert the HTML overlay to babylonjs. Still needs some refinement, I hope I can push it soon.
Yess!! That would be really nice to add as well. If you want to start a new issue/branch - would be happy to look even at a draft version 😄
OK @dennemark, don't know if you have time to try it out, but my brain is a bit fried from work today! I took a quick look and I will take a better look tomorrow. There is already a reconciler created and the createReconciler
is basically just calling createContainer
an an existing Reconciler. There were actually weird issues in an older version where I was creating new reconcilers each time a scene was created. Here is where the change should go (I think).
https://github.com/brianzinn/react-babylonjs/blob/master/src/render.ts
I think we can assume people are not using primary renderer (or add an optional parameter like this) and in render.ts:
// can maybe add typings on container
export function createPortal(children: React.ReactNode, container: any, usePrimary: boolean = false): React.ReactNode {
return (usePrimary ? ReconcilerPrimary : ReconcilerSecondary).createPortal(children, container, null, null);
}
if you get it running a storybook would be cool!
@brianzinn thanks a lot! Unfortunately I could not have a look at it today, too. Will need to have a closer look next week! But now I understand that you have two reconcilers! ( need to read, why you use two renderers ) I could imagine that it will work. Looking forward to try!
The reason for 2 is just to allow somebody to use the primary renderer, which when I added that feature was an experimental. In other words, typically this reconciler is running as the secondary one (and DOM is the primary one). You can read more here: https://github.com/facebook/react/blob/d483463bc86555decb3e8caa18459d1d0f7c0148/packages/react-reconciler/README.md#isprimaryrenderer
I think the current issue is that the public instance is not a "fiber". I will try to add a property to access to proper fiber - I'm working through the new typings on HostConfig<>
as well.
alright @dennemark -so, that latest version seems at least to "work" (with some odd hacks in place) and now to work backwards from there to a clean solution. I feel like prepareMount
should have let me reverse what is happening for getPublicInstance
. One major difference between r3f and react-babylonjs is that I am returning the native object from the engine (originally with no modifications/metadata), while r3f stores that fiber metadata on the three object itself. Until looking at what is happening now I actually liked my way better, but now I need to see what is going on to continue thinking that way - especially considering what I did in the storybook ['__rb_createdInstance']
:
{(transformNodeRef.current) &&
createPortal(<box position={new Vector3(0, 1, 0)}>
<standardMaterial diffuseColor={Color3.Blue() } specularColor={Color3.Black()} />
</box>, transformNodeRef.current['__rb_createdInstance'])
}
Hi, it looks pretty good! I made a pull request to fix the hack. If it is correct, you might laugh, because it was pretty simple ;) #166
Concerning the functionality, i realise the createPortal hook is not fully what I was searching for. I realise I have the same issue as with the html component. As soon as I send the ref outside of the Engine, it will always be null. I was hoping I could send data from somewhere else into the viewport though. Since my interface is living next to the viewport. A bit like this pseudocode:
<InterfaceContainer>
<PortalToBJS toNode=MyNode>
<SomeMesh>
</PortalToBJS>
</InterfaceContainer>
<Engine>
<MyNode>
//Portal inserts SomeMesh here
</MyNode>
</Engine>
What I came up with so far, is placing the children of PortalToBJS into a store (zustand). PortalToBJS does not return the children, so it does not complain. And then I read from the store in a another PortalExit component and place the children within the react tree. As I stated in the html #165 pr , this feels a bit hacked though, with unwanted side effects.
I am not sure if a context bridge might work here, I will try it out later.
@dennemark the storybook is published with latest changes (ci)! https://brianzinn.github.io/react-babylonjs/?path=/story/gui--html-text
@brianzinn thank! text floats in upper left corner in my case. i will have a look at it tomorrow! also am trying out some other promising approach for a kind of portal. hope to it will work properly!
great. if it's easier for you to test in an external project let me know if publishing a new NPM would help. Cheers.
I guess this one can be closed considering #167
Related approaches:
Hi,
I was wondering if it is possible to use the reconciler function to create a portal. Looking at how react-three-fiber is doing it, it seems to be a simple function However, currently the reconciler is not exposed. The createReconciler function lives within Scene and it does not return the reconciler itself. I managed to get access to it, but it does not seem to be that easy :/ Getting error
Unexpected Fiber popped.
andExpected host context to exist
Even if I place the createReconciler function outside of Scene, so that it would be called only once, it leads to the same error.
Would it be straightforward and worth to implement?
For completeness, I was trying to achieve something like:
(I just realized, I should have posted it in disccusions....)