Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: Deckgl Fiber Renderer #35

Closed

Conversation

brandonjpierce
Copy link

@brandonjpierce brandonjpierce commented Feb 29, 2024

Original PR on the deck.gl repo visgl/deck.gl#8560

ℹ️ Opening PR early to get initial feedback while we polish up
⚠️ Example dir currently not working, seems to be an issue with luma.gl

Remaining TODOs

  • Implement HTML interleaving (we had some prior art here but removed for the sake of the first pass)
  • Add in a bunch of tests
  • Fix any remaining TS errors
  • Provide more complex examples
  • Detail any known caveats or gotchas
  • Provide example for React driven tooltips

Example

cd examples/website/react-fiber
yarn
yarn start-local

Background

Initial discussion in Slack: https://deckgl.slack.com/archives/C34AC5MSQ/p1689366597250129

My company has a very large and complex React application that also uses Deck.gl. Over time as the complexity and scope of the project grew we were running into issues with the existing @deck.gl/react package due to some of its limitations with children ordering and a few others.

A very popular technique adopted in the three community was to provide a React renderer for ThreeJS so that developers would not need to swap between the two paradigms in their brain. This is the goal of this effort as well. Allow developers to always "think in React".

This deck.gl fiber renderer has been used in a production app for almost half a year now and has unlocked some really incredible techniques for our team.

Some high level concepts:

// all deck.gl layers are supported out of the box, e.g.

<scatterplotLayer />
<tileLayer />
<goeJsonLayer />

// etc..
// Support for custom layers
class MyCompositeLayer extends CompositeLayer {
  ...
}

// inject into renderer
extend({ MyCompositeLayer });

// use in react
<myCompositeLayer />
// can nest layers in multiple views
<DeckGL>
  <mapView id="main>
    <MainApplicationLayers />
  </MapView>
  <Some>
    <Nested>
      <mapView id="minimap>
        <MinimapLayers />
      </MapView>
    </Nested>
  </Some>
</DeckGL>
// Order for layers is based on React tree ordering
// e.g. [bitmapLayer, scatterplotLayer, lineLayer, myCustomLayer]
<MainApplication>
  <bitmapLayer />
  <scatterplotLayer />
  <lineLayer />
  <Some>
    <Nested>
      <Component>
        <myCustomLayer />
      </Component>
    </Nested>
  </Some>
  ...
</MainApplication>
// colocation becomes incredibly easy and allows you to self contain components more
function MyLayer() {
  const [hovered, setHovered] = useState();
  const [clicked, setClicked] = useState();

  const onHover = useCallback((pickInfo) => {
    // additional logic
    setHovered(...)
  });

  const onClick = useCallback((pickInfo) => {
    // additional logic
    setClicked(...)
  });

  // Make a stable dataset for picked object
  const activeData = useMemo(() => [clicked], [clicked]);


  return (
    <>
      <scatterplotLayer
        id="base"
        onClick={onClick}
        onHover={onHover}
        pickable
        ...
      />
      
      <scatterplotLayer
        id="fancy-active"
        data={activeData}
        visible={Boolean(clicked)}
        ...
      />
    </>
  )
}

Change List

  • Adds in a new package called @deck.gl/react-fiber
  • Adds in a new website example called react-fiber

canvas,
layers: [],
onWebGLInitialized: state.setWebgl
// TODO: investigate if we need
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love insight into any tips and tricks this can potentially unlock

root.current = createRoot<HTMLCanvasElement>(canvas);
}

// @ts-expect-error FIXME
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO

} from '@deck.gl/layers';
import type {Instance} from './types';

// IDEA: we can technically move this purely to userland so that a user is defining precisely
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Articulating this more, as a developer I could in theory only extend explicitly what I am using in my app e.g. say I am only using a MapView and ScatterplotLayer:

extend({
  MapView,
  ScatterplotLayer
});

This would allow the bundle size to shrink (in theory) for the package consumer.

// NOTE: may need to attach handlers here
commitMount: noop,

// NOTE: this fires per instance, since deckgl cannot update individual layers
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,304 @@
import {log} from '@deck.gl/core';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging temporary

*/

state.deckgl.setProps({
// TODO: investigate why we need to spread the array here for deckgl to pick up layer changes
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hunch here is that Deck does some internal shallow compare against the layer list array? e.g. doing:

state.deckgl.setProps({
  layers: layerCache,
});

Does not work since we are mutating layerCache directly in some of the other functions

const state = container.getState();

/**
* TODO: implicit view/layer filter logic
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibgreen
Copy link
Contributor

ibgreen commented Mar 7, 2024

@brandonjpierce Sorry for slow response here. I sent you an invite to this repository so that you can merge things yourself.

Copy link
Contributor

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will give this a rubberstamp approval for merging, as long as it doesn't break the repo.

@brandonjpierce
Copy link
Author

@ibgreen thank you! I am working with my team to button up a number of the TODOs we have remaining on this (we have some folks out sick/on PTO right now). Will keep y'all updated with regards to polish and maturity of the PR.

@ibgreen
Copy link
Contributor

ibgreen commented Mar 7, 2024

@ibgreen thank you! I am working with my team to button up a number of the TODOs we have remaining on this (we have some folks out sick/on PTO right now). Will keep y'all updated with regards to polish and maturity of the PR.

Sounds good. A reasonable approach is also to merge this baseline and do incremental improvements from there. Let me know if your team needs additional write permissions to the repo.

@ibgreen
Copy link
Contributor

ibgreen commented Apr 11, 2024

Please reopen if this picks up again.

@kahole
Copy link

kahole commented Jun 17, 2024

This looks very interesting! We have been struggling with some of the same concerns consolidating the React and deck.gl mindsets in our project. Especially when leaning heavily on libraries as React Query and Zustand for managing data and state.
Layer ordering is one concern we share as well in our workaround efforts using Zustand to isolate and store layer updates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants