Git Product home page Git Product logo

Comments (5)

jtbandes avatar jtbandes commented on June 19, 2024 1

with the right docs and warnings, having a separate prop might be easier to understand

yeah, agreed if we have both docs and warnings, this should be fine.

  1. Make a story on the cameraState module that allows passing matrices directly

we'll have to see about this organization, since if the matrices are a separate prop from cameraState it doesn't make as much sense to group them with the cameraState stories...but they are camera-related, so maybe it's fine. You might find some opportunities to clean up the organization while you're there.

Looking forward to your branch!

from webviz.

jtbandes avatar jtbandes commented on June 19, 2024

This definitely seems like a good feature to add. Let's think about the API a bit more — some of the properties in cameraState would be replaced by the view matrix and some by the projection matrix, right?

  • view comes from: target, targetOrientation, targetOffset, distance, phi, thetaOffset
  • projection comes from: perspective, distance, fovy, near, far

I was thinking it might be nice to group these properties in with the cameraState, but now I realize if both matrices are specified then there's nothing left in cameraState (except that keyboard/mouse controls depend on the values in cameraState, but you said you'd probably want to disable those). I'd be a bit hesitant to keep adding new props to Worldview since it already has so many, but maybe we can change the type of cameraState to include both alternatives CameraState | CameraMatrices? Then it's also more clear that you can't specify both the matrices and the other cameraState properties.

Do you think you (or other users) would want to supply one of these matrices but not the other?

As an point of reference, the XRView interface defined in the WebXR spec provides a projectionMatrix and a transform (which has a position+orientation, and also a combined matrix property).

cc @brianc @vidaaudrey who worked with raw view/projection matrices for a Worldview VR project :)

from webviz.

colinterface avatar colinterface commented on June 19, 2024

Thanks for the feedback! 😸

Just spent some time poking at it this morning and I think I have a plan for moving forward.

I'd be a bit hesitant to keep adding new props to Worldview since it already has so many

I totally see your concern. I think it's a matter of documentation though. There are 3 groups of users who all want a simple explanation (and ideally an example) of the props they need to use.

  1. Someone who just wants to use worldview's internal camera state. They'll want to know that this is the default option, that they can use the default mouse and keyboard controls, how to customize them if they want to, and how they can provide an initial camera state if they need to.
  2. Someone who wants control of the camera state, but doesn't want to deal with matrices. They'll want to know about the cameraState object and how each parameter works. They'll also want to know that they need to use the onCameraStateChange callback to update their camera state so they can use the built in camera controls.
  3. Someone who wants to use their own view and projection matrices. I believe this person will accept that they'll have to provide their own camera controls, though they might be interested in getting the input events and mapping them the same way they would if using the built in camera controls…

Personally I don't need Worldview to manage input events… that's firmly outside of the scope of what I want it to do for me. But it's definitely convenient if we want this to be a one stop shop for building a 3D application. That's a whole other discussion though.

but maybe we can change the type of cameraState to include both alternatives

Turning cameraState into a union type is definitely one option, but I think with the right docs and warnings, having a separate prop might be easier to understand… implementation-wise it's not a huge difference. My gut reaction is generally to avoid union types where possible.

Do you think you (or other users) would want to supply one of these matrices but not the other?

I think in most cases if someone is using matrices, they'll probably provide both. CameraStore provides valid defaults for any values missing in the cameraState object. We could do a similar thing for cameraMatrices, along with a console warning.

As a point of reference, the XRView interface defined in the WebXR spec provides a projectionMatrix and a transform

Using the name transform instead of view makes sense in the context of the XRView interface because it's a transform for the view. But I think this shape works well for our context:

const cameraMatrices = { 
  view, 
  projection 
};

Dealing with view and projection matrices is common in regl and the greater WebGL ecosystem. People using this advanced feature will know that they map to the projection * view part of their vertex shader.

Anyway, it seems like we'll be able to reach alignment one way or another and at this point I'll just need to make a branch for more feedback. Here's my current plan:

  1. Make a story on the cameraState module that allows passing matrices directly
  2. Update the cameraState documentation to clearly lay out the 3 scenarios described above
  3. Update the Worldview component, the CameraStore class, and the camera selectors to support the new behavior.
  4. Open a PR for further discussion.

We have a sprint coming up where we're focusing on FE infra. I'll plan to tackle this during the week of Dec 9. Open to discussing more here but probably won't be able to do much until then.

Does that sound like a reasonable plan?

from webviz.

colinterface avatar colinterface commented on June 19, 2024

Hey there, we haven't forgotten about this. We have a fork over at https://github.com/hoverinc/webviz/

As we continue to integrate Worldview into our apps, we are finding more tweaks we need to make.

I think our plan to merge might have been a bit premature. It's still something we would like to do, but it might make sense to wait until our changes have stabilized a bit more.

from webviz.

janpaul123 avatar janpaul123 commented on June 19, 2024

Cool, feel free to make a PR if you think it's stable enough now. 😄

from webviz.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.