solid-three icon indicating copy to clipboard operation
solid-three copied to clipboard

next `solid-three`

Open bigmistqke opened this issue 2 years ago • 7 comments

This branch continues the context-and-proxies-branch from @vorth in which he took the proxy-implementation from @nksaraf solid-three-chess branch of react-three-fiber.

This branch' intention is to remove all the dependencies and artefacts from react, remove the dependency on zustand as an internal state manager in favour of solid/store and streamline the code, while keeping as close as possible to r3f's v9 codebase.

what has been done

  • fixed the broken event-system
  • remove all references to useRef .current
  • remove dependency on zustand
    • useThree works different then r3f's, while they have selectors to subscribe to store-updates, with our useThree is just a wrapper around useContext(ThreeContext), as we don't have to worry about unnecessary re-renders.
    • a lot of code in core now uses solid-store specific code to update the store, so the distinction between core and solid directory is further made unclear.
    • it simplified some of the internal code, since we don't have to const state = store.getState() and the like.
    • store.subscribe() has been replaced by createEffect(on(() => store, () => {…})) which I am not 100% sure is the same. store. subscribe might be deep-tracking bc off the whole immutable-thing.
  • I added an optimization to applyProps to prevent unnecessary creations/cleanups of effects with mapArray(() => Object.keys(props), ...) and checking if prop is possibly reactive (=== a getter or function)
  • refactors/cleanups:
    • the code in solid/index.ts is moved to solid/renderer.ts. core/renderer.ts is removed. It contained some types it itself had inherit from the removed reconciler.ts, they have been moved to three-types.ts.
    • index.ts, ~solid/index.ts~ (directories are merged, see below) and core/index.ts now only export files
    • removed secondary implementation of applyProps
    • removed unused three-types (remains of r3f's main branch)
    • replaced our custom three-types with r3f's v9 branch' implementation (it used to be done manually, but in the meanwhile they also figured out how to infer it from three.js)
    • our .prettierrc had the line "plugins": ["prettier-plugin-tailwindcss"] which caused my prettier to not function. I replaced our .prettierrc with the one of r3f to prevent unnecessary differences between codebases.
    • went ahead and merged the core and solid-repo back into core, more closely mimicking r3f's project-structure
    • renamed core/components to core/proxy (bc I thought it was a better parallel with reconciler and i preferred how it looked lol)
    • refactored the codebase to mimick r3f's codebase as close as possible

reflections

~Currently our codebase is somewhere between r3f's main branch and v9. For example, Instance is typed like the main branch, but we have Stages like v9. Probably these changes happened after nikhil made his implementation (found pr when they made the change). Idk if we should pull everything closer to v9 or not. I think yes, but my previous attempts have all failed.~

Instance has been ported to current v9 implementation. Our codebase is now up-to-date.

~As I said above; since we now have solid-store-related code throughout the codebase idk if it still makes sense to make the distinction between the two. I think mb it's best to just bring it back into 1 repo and try to mimic the structure as closely to r3f as possible, so we can at least manually compare the two when needed.~

I went ahead and merged those directories

I think it would probably be good to try out some more complex examples, to test out this version. Maybe port the examples of r3f.

bigmistqke avatar Jul 16 '23 20:07 bigmistqke

I refactored the type of Instance as described in this commit Now I think our codebase is pretty much aligned with the current v9-branch of r3f.

I also added r3f as a sub-module into the repo, so that it's easy to compare them.

I personally think syncing their repo automatically with ours will be difficult to do, as there are little changes throughout the codebase. Instead I tried to mimmic their codebase as much as possible with ours so we can diff manually (I have been using the vs-extension Diff Folders to do that, it's quite handy).

bigmistqke avatar Jul 18 '23 10:07 bigmistqke

[for documentation purposes]

The initial way how children were attached to parents, in this branch, was done with the use of context, from child to parent: each parent would wrap its children with a <Context.Provider value={() => Instance}>{props.children}</Context.Provider/> and each child would check this context with the use of the useParent-hook and attach itself to this parent. This however broke the expectation that only returned jsx would attach itself.

const Noop = () => {
  const mesh = new <T.Mesh/>
  return </></>
}

would actually connect the mesh to its parent.

To fix this I adjusted the code (see b6cbde4) to create the hierarchy from parent-to-child instead of from child-to-parent.

Instead of using context we read the props.children, from the parent, and attach the children to the parent accordingly. This way only returned jsx becomes a part of the tree.

bigmistqke avatar Jul 20 '23 17:07 bigmistqke

I am looking to use this repo to create a solidjs renderer for pixi js and this maintenance and solid-js refactoring is massively appreciated. Looking forward to seeing where this PR goes :)

connorgmeehan avatar Jul 22 '23 06:07 connorgmeehan

[for documentation purposes]

  • we move away from v9 to v8 since it has a more stable api, on advice by @codyjasonbennett.
    • v8: Stages are removed
    • v8: loop-functions are scoped inside createLoop
    • v9: Instance is still typed like v9 see
    • v9: T is still typed dynamically, with custom code to account for overloaded methods see
  • I have been working on the drei port at solid-drei which helped this port too:
    • Portal is fixed
    • useLoader has been improved
    • mechanism in place to extend T with namespace SolidThree and SolidThree.IntrinsicElements (for three-elements and extend()) and SolidThree.IntrinsicComponents (for custom solid-three components like T.Suspense and T.Primitive)
  • Improve Suspense:
    • r3f has nice loader patterns in which their useLoader throws immediately on creation and triggers suspense-boundaries. This way it's always guaranteed that their resources are always loaded. It has the downside that no resources are parallelized (afaik). Solid's way of doing Suspense is fundamentally different, so our previous implementation caused some friction when using Suspense.
    • To mimick more closely r3f behavior of suspending creation of three-elements until suspended boundary is resolved, while retaining parallel resource loading some work has been done:
      • <T.Suspense/>: includes a context with a { resolved, addResource }-value
      • onSuspense hook: to read the suspense-context
      • createThreeResource(): a wrapper around createResource that context.addResource)
        • useLoader uses createThreeResource internally, all of drei's loaders are based on this primitive.
        • <T.Primitive/> and createThreeComponent both contain code that read suspenseContext from useSuspense and that can suspend three-element creation in case !suspenseContext.resolved.
      • Timeline of creation of a <T/>-component or a <T.Primitive/>:
        • an initial inspection-run:
          • We do not yet create the three-element
          • We resolve the children and read the props.
          • We do not yet connect/attach the children nor apply the props
        • after initial run:
          • if no resource was read during this initial run:
            • create three-element immediately
            • apply props
          • if a resource was read during this initial run:
            • if component is sibling of a T.Suspense-context:
              • effect/onMount suspended until Suspense is resolve
              • suspend creation until suspenseContext.resolved
              • applyProps and manageChildren is suspended until element is created
            • if component is sibling of a regular Suspense:
              • effect/onMount suspended until Suspense is resolve
              • create element immediately
              • attach children immediately, but the root is added to the scene only when Suspense is resolved
              • apply props with undefined for the resource (ignoring the prop), the prop is applied once it is resolved (potentially even before the Suspense is resolved)
            • if component is not a sibling of any type of suspense:
              • create element immediately
              • apply props with undefined for the resource (ignoring the prop)
      • When the children are resolved, they go through the same process, causing this implementation to resolve the tree depth-first: we walk to the leafs first and then build from the leafs up to the root, and finally attach the tree to the rest of the scene. Before the implementation the tree was resolved top-down, which was the cause of earlier Suspense-inconsistencies where a <T.Mesh/> would already be attached to the scene while its <T.Material/>-child would have a resource.

Comments:

  • I am unsure if the whole suspend-element-creation strategy is 100% necessary, with its custom T.Suspense and createThreeElement, it could very well be an unnecessary complication. The initial inspection-run to collect resources together with regular Suspense, resolving the tree from the bottom-up until the Suspense-boundary, could visually be enough. It might even be more performant, since it spreads out the creation of three-elements and the applying of props.
  • We should investigate more what are the actual benefits of suspending three-element creation with all props defined: I think there could be perf benefits if it's for args (I think it could prevent an additional compilation of a shader) but if they are used as methods of an existing three-object I am not sure if there is any perf benefit at all:
// this could have performance benefits if texture is defined
const material = new THREE.Material({map: texture})

// I am not sure this would, since it would need to compile the shader first anyway
// except if THREE would queueMicrotask the compilation of the shader 🤔
const material = new THREE.Material()
material.map = texture
  • This timeline-management comes with the cost of a createLazyMemo in <T.Primitive/> and all other <T/>-components and the double-run means effects need to be run twice (duh). All values are memo'd with mapArray so I suppose the extra effects won't be too expensive, but there will be some overhead.

bigmistqke avatar Aug 17 '23 13:08 bigmistqke

@connorgmeehan cool! I would be pretty interested if there is a way to generalize this renderer, so you could plug it in other libraries. If you wanna come and hang out with us at discord

bigmistqke avatar Aug 17 '23 14:08 bigmistqke

I removed all the timeline-management code and the suspension of three-element creation as they didn't compose nicely with ordinary jsx-elements and messed up some expectations, p.ex on when refs are resolved:

<T.Mesh ref={mesh}>
  <CustomComponent mesh={mesh}/>
</T.Mesh>

In the above example CustomComponent expects to receive mesh immediately, but with the initial inspection-run this resulted into undefined.

bigmistqke avatar Aug 17 '23 19:08 bigmistqke

@connorgmeehan cool! I would be pretty interested if there is a way to generalize this renderer, so you could plug it in other libraries. If you wanna come and hang out with us at discord

Sorry for the late response. It's funny you say that. That's one of the first things that I did when adapting this code.

Here's an early version of the generalised proxy renderer. I haven't really bothered open sourcing it the up to date code because I'm working in a private monorepo with my app. https://github.com/sanspointes/constructables/ Basically it just lets you create a new root with a context and lets you wrap any classes into a component with automatic prop typing (which is honestly pretty buggy and slow for the typescript compiler). It also prefers defining implied props explicitly (such as position-x: number) rather than at runtime so you get proper typing. Also here's the pixi renderer implementation that uses the generalised renderer.

I am running into performance issues, especially surrounding the way props are tracked when components are created. I would love to combine efforts. :)

Also can you re-share the discord link? I can't open it.

connorgmeehan avatar Oct 06 '23 04:10 connorgmeehan