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

Implement custom geometry API #24

Open
fuddl opened this issue Mar 21, 2021 · 6 comments
Open

Implement custom geometry API #24

fuddl opened this issue Mar 21, 2021 · 6 comments

Comments

@fuddl
Copy link
Contributor

fuddl commented Mar 21, 2021

@mithi next I'll try to implement the API into bare-minimum-3d. I guess it should look like this:

renderScene(viewSettings, sceneSettings, sceneOptions, data3d, plugins)
@fuddl
Copy link
Contributor Author

fuddl commented Mar 22, 2021

@mithi I suppose we need a way to tell the DataRenderer.render() which projection method should be used. Currently this is determined by it's (hardcoded) type.

How about we instead ask if the data is shaped like a point or a line. Eg: does it have a property x or x0? The line case seems rather exotic to me. Did you plan other primitive types?

In order to make my triangles example work, all I needed to do was:

    render(data: Array<Data3dSpecs>): Array<Data2dSpecs> {
        console.debug(data)
        return data.map((element: Data3dSpecs) => {
            switch (element.type) {
-               case DataSpecType.polygon:
-               case DataSpecType.points:
-                   return this._projectPolygonOrPoints(element)
                case DataSpecType.lines:
                    return this._projectLines(element)
+               default: 
+                   return this._projectPolygonOrPoints(element)
            }
        })
    }

What do you think? should _projectPolygonOrPoints be used as the default?

@mithi
Copy link
Owner

mithi commented Mar 23, 2021

I think it is better to be more explicit with the types and to be more explicit on the algorithm that will be used based on the type.

Also I think it would be better, if DataRenderer is not modified at all inspired by the open-closed principle. The code might be easier to maintain if it's closed for modification but open for extension..

It might be better if the pseudo code could be something like this:

// plugin = {
//  Renderer: (The actual renderer)
//  types: (an array of types it can render)
// }

const renderScene = (
  viewSettings: ViewSettings,
  sceneSettings: SceneSettings,
  sceneOptions: SceneOptions,
  data3d: Array<Unknown>,
  plugins: Array<Unknown>
)  {

  // .... initial computations 

  const bareMinimumDataRenderer = new DataRenderer(/*...arguments...*/ )

  const models = data.map((element: Unknown) => {
    if ([DataSpecType.polygon, DataSpecType.points, DataSpecType.lines].includes(element.type)) {
      // this could be optimized by adding the function:
      // bareMinimumDataRenderer.renderSingle(element)
      return bareMinimumDataRenderer.render([element]) 
    } 
    
    // the bareMinimumDataRenderer does not know how to render this type of data
    // use one of your customRenderers here
    for (let plugin of plugins) {
      if(!plugin.types.includes(element.type)) {
        continue
      }
      return new plugin.Renderer(/*...arguments...*/).render(element)  
    }
    // write an error here `unhandled datatype: ${element.type}`
  })

  // more code here....

  return { container, data: [...sceneData, ...models] }
}

@mithi
Copy link
Owner

mithi commented Mar 23, 2021

Did you plan other primitive types?

I only planned one other type which is a 3d circle.. see also: #6

@mithi
Copy link
Owner

mithi commented Mar 23, 2021

@fuddl

If you also think that you need to reuse the the built DataRenderer, feel free to modify bare-minimum-3d to expose it
so that your custom plugin / data renderer could use it. ie modify
https://github.com/mithi/bare-minimum-3d/blob/master/src/index.ts

import { renderScene } from "./utils.js"
import DataRenderer from "./data-renderer"
export { DataRenderer }
export default renderScene

@fuddl
Copy link
Contributor Author

fuddl commented Mar 23, 2021

you are suggesting plug-ins should look like this?

const trianglesPlugin = {
  triangles: {
    types: ['points'],
    Renderer: (element, transforms) => {
      const { size, color, opacity, id } = element
      return element.x.map((x, i) => (
        <Triangle
          {...{
            x,
            y: element.y[i],
            size,
            color,
            opacity,
            id,
            i,
            transforms
          }}
          key={`${id}-${i}`}
        />
      ))
    }
  }
}

and their invocation should look like this?

const { container, data } = renderScene(
  viewSettings,
  sceneSettings,
  sceneOptions,
  [{
    id: 'my-trangle',
    type: 'triangles',
    opacity: 1.0,
    color: 'white',
    size: 2,
    x: [42],
    y: [23],
    z: [47],
  }],
  [trianglesPlugin]
)

@mithi
Copy link
Owner

mithi commented Mar 23, 2021

you are suggesting plug-ins should look like this?

No.

renderScene converts 3d data to 2d data.

For example, this is the input

const dataInput3d = [{
        color: "#FC427B",
        opacity: 1,
        size: 14,
        id: "head",
        type: DataSpecType.points,
        x: [0.0],
        y: [100.0],
        z: [100.0],
    }]

this would be the output

const dataOutput2d = [{
            color: "#FC427B",
            opacity: 1,
            size: 14,
            id: "head",
            type: "points",
            x: [-20.91252502759967],
            y: [-93.39172830586264],
            z: [100] // having a z is a stupid bug, please ignore, I will fix this soon
        }
]

And this 2d data is the one you feed to BareMinimum2d ie

const { container, dataOutput2d } = renderScene(
        viewSettings,
        sceneSettings,
        sceneOptions,
        dataInput3d
    )

<BareMinimum2d {...{container, data: dataOutput2d} />

But that's how it works...

It's alright if you think it's too much work to make it work... don't feel obligated to continue implementing this feature!

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

No branches or pull requests

2 participants