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

Imperative coordinates manipulation #61

Closed
mszekiel opened this issue Sep 30, 2023 · 7 comments
Closed

Imperative coordinates manipulation #61

mszekiel opened this issue Sep 30, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@mszekiel
Copy link

I've been exploring this library, and it's been a fantastic experience so far. I've noticed that updating object coordinates in the scene via props for moving objects (not mentioning even the interpolation now) can lead to frequent re-renders, impacting performance. To optimize this, I was thinking of leveraging the useFrame to directly manipulate the object coordinates. But after looking into the current implementation this doesn't sound as easy thing. Is it even possible in current state of mapbox layers? As I unterstand in general each object is new three scene.

I'm curious to hear your thoughts on this and whether it aligns with the library's development goals. I appreciate the hard work you've put into this library, and I'm excited to see how it continues to evolve. 😊

@RodrigoHamuy
Copy link
Owner

Hey, thanks! Each <Coordinates> component is a new Three Scene, but updating lat/longs is just updating the camera matrices. React does struggle in general to do props updates for animations, so as long as these are received as props is going to be slow due to React's nature. But we can expose a hook where you can alter these values without triggering react re-renders. I will add it to the backlog! Thanks for the suggestion :)

@forerunrun
Copy link

forerunrun commented Oct 9, 2023

Each <Coordinates> component is a new Three Scene

Hey @RodrigoHamuy, this gives more insight into the issue I'm encountering, what's the basis of creating a new scene per coordinate? could it be an option to apply matrix transformations to added objects individually and use map.getLayer("THREE-Scene") to manage and manipulate one single three scene and camera? this way you'd open up the possibility of applying matrices to vectors, groups, geometries, meshes etc.. while preserving the current functionality of coordinate positioning... the coordinates component could simply be a group that nests it's children inside the scene with the correct transforms applied... it'd be really great to be able to set vector3's from coordinates so that objects can be created with attributes relative to map space coords...

EDIT: the scene of a layer in mapbox can be retreived with the following...

const threeLayer = map.getLayer('3d-model')
console.log(threeLayer.implementation.scene)

@RodrigoHamuy
Copy link
Owner

Fair point. I first did indeed tried that: Each <Coordinates> had a parent Object3D to which I set its matrix to account for the coordinates it is set on, and take that part of the calculation out of the camera matrix. But I was struggling to make it work.

But apart from my struggle xD the other reason is that the current solution brings one good benefit imo:

It keeps the understanding of the Up vector simple.

If for example you use Environment Maps to reflect ground and sky, the Up vector only works in a point and what you see becomes progressively wrong as you move away from that point. For kilometers the difference can pass unnoticed, but will look very wrong if this Environment Map is shared between two points on opposite sides of the world. One would get the sky when looking up, while the other will get the floor when looking up.

@forerunrun
Copy link

forerunrun commented Oct 10, 2023

It keeps the understanding of the Up vector simple.

OK yes that sort of makes sense, i think something like the following could also calculate the up vector for an object positioned on any lat lng coordinate...

function calculateUpVector(latitude, longitude, radius) {

    const x = radius * Math.sin(latitude) * Math.cos(longitude);
    const y = radius * Math.sin(latitude) * Math.sin(longitude);
    const z = radius * Math.cos(latitude);

    const objectPosition = new THREE.Vector3(x, y, z);
    const sphereCenter = new THREE.Vector3(0, 0, 0);

    const forward = new THREE.Vector3().copy(objectPosition).sub(sphereCenter).normalize();
    const right = new THREE.Vector3(0, 1, 0).cross(forward).normalize();
    const up = forward.clone().cross(right).normalize();

    return up;
}

this could be even more optimized by reusing some of those vector3's...

I kind of see what you mean by the Environment Maps issue, that's a tricky one because fmu the mapbox camera is being "rotated" or transformed under the "sphere" rather than the sphere being rotated around it's own polar / azimuth axes so would make sense that the under side would reflect the env maps floor, rather than sky, for instance if the env map was half "day" and half "night" this would reflect a real world scenario, as of course, one side of the planet is always unilluminated at any point in time...

EDIT: there is also the option here of applying the env map to another larger sphere and having that sphere copy the cameras decomposed matrix rotation if you wanted to preserve the sky always being "up"...

@RodrigoHamuy
Copy link
Owner

Yes, exactly. Altering a parent matrices requires a more complex env map if for example you just want the usual ground+sky, while current solution you can just drop <Environment> from drei and it should work as usual.

Because of time limitations, I don't think I will be changing the strategy from updating only camera matrices to updating both camera matrices + parent objects, but happy to receive PRs with suggestions. Ideally, if we ever implement that, it should be a separated component so users can pick which one they want depending on their needs, since both have different pros and cons.

I will be adding coordinates manipulation imperatively though as soon as I have some spare time, hopefully a quick win 🤞

@RodrigoHamuy
Copy link
Owner

You can now use the function coordsToVector3, for example to manually set an object to a particular coordinate (imperatively). Cc @mszekiel @forerunrun

@mszekiel
Copy link
Author

mszekiel commented Mar 7, 2024

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

3 participants