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

add orthographic camera support #157

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Algorush
Copy link

@Algorush Algorush commented Apr 2, 2024

@Avnerus, hello! It will be great if you review this, as I did this PR using chatGPT help :)
It works well with combined 3DStreet Editor + aframe-tiles-loader-component. Will be able provide a link to example later

@Avnerus
Copy link
Collaborator

Avnerus commented Apr 3, 2024

Thank you @Algorush (and gpt ;) )
It looks like adding orthographic camera support is a bit more tricky.
It requires making a change to the base tile library, loaders.gl/tiles - you can see here that they have already been preparing for it.
You can see an example of an orthographic implementation in the NASA loader here and here.
Once support is added to loaders.gl, three-loaders-3dtiles would just have to pass an isOrthographic flag to that library.
Let me know if you would like to make a pass at it, you could submit a PR to loaders.gl where I am also a maintainer.
Thank you again!

@Algorush
Copy link
Author

Algorush commented Apr 9, 2024

Let me know if you would like to make a pass at it, you could submit a PR to loaders.gl where I am also a maintainer

yes, I'll try to do this.
I have a question about the ortho camera support approach.
Here in loaders.gl they proposes to create this._getOrthograhicScreenSpaceError() function for the ortho camera.
But as far as I understand the code for ortho camera support in the NASA lib, they calculate and use pixelSize value for ortho camera and sseDenominator for perspective camera. Should we use same approach in loaders?
I looked in more detail. In essence, the formula for calculating pixelSize and SSE is the same. Sse is calculated only for height. And pixelSize is also calculated by sse for width and the maximum value between sse for width and height is taken. So I will use this approach too

@Avnerus
Copy link
Collaborator

Avnerus commented Apr 10, 2024

Hi, this is great that you are going to add this feature!

Should we use same approach in loaders?

I think that makes sense.
About the development, I think it would be easiest to add another example to the 3d-tiles example library (source here). You could add "Melbourne (Photogrammetry): Orthographic".

The 3d tiles loader has a frameState variable (accessible to the sse calculation function) that describes the current view.
The frameState contains a viewport reference that is a copy of the Deck.gl viewport (the renderer that uses loaders.gl natively and the one used in the examples).
When Deck.gl is initialized with an Orthographic view, it should set viewport.orthographic to true. Meaning that in the tiles function you should be able to read frameState.viewport.orthographic and if it is true, you calculate the error using pixelSize.

In three-loader-3dtiles, I created a "fake" viewport to comply with the protocol. So in theory, once orthographic tiles work in the deck.gl example, they should work also in three.js just by setting orthographic: true.

Once you have the loaders.gl example working, you can create a PR there and I will follow with the change to three-loaders-3dtiles.

Good luck to all!

@Avnerus
Copy link
Collaborator

Avnerus commented May 31, 2024

Hi @Algorush,
Have you been able to get this working? If not, I might be able join the effort.

@Algorush
Copy link
Author

Hi @Algorush, Have you been able to get this working? If not, I might be able join the effort.

Hi @Avnerus, yes I'm still going to do it. I apologize for the promise and such a delay. I'll start this weekend and maybe do it. Thank you for reminding

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.

2 participants