Skip to content

Conversation

andretchen0
Copy link
Contributor

@andretchen0 andretchen0 commented Feb 6, 2024

Problem

Orthographic camera is not getting updates when screen size changes. So the aspect ratio falls out of sync with window size, resulting in squished/stretched images.

Solution

  • Update orthographic camera aspect when screen size updates
  • Give user a "manual" flag to keep Tres from updating camera

Context

Manual flag in R3F.

* Update orthographic camera aspect when screen size updates
* Give user a "manual" flag to keep Tres from updating camera
@andretchen0 andretchen0 marked this pull request as ready for review February 6, 2024 15:58
Copy link
Member

@alvarosabu alvarosabu left a comment

Choose a reason for hiding this comment

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

Thanks for this @andretchen0. I like the manual prop, I'm thinking how we could add it to the docs so the users are aware of it.

@andretchen0
Copy link
Contributor Author

Thanks for this @andretchen0. I like the manual prop, I'm thinking how we could add it to the docs so the users are aware of it.

Maybe we add an entry in https://docs.tresjs.org/advanced/ ?

I'm not sure many users will want it. It's pretty niche. Afaik, no one had complained that Tres was controlling the <TresPerspectiveCamera /> previously.

I just don't want to force people to drop Tres because their use case requires controlling the camera.

@alvarosabu
Copy link
Member

Thanks for this @andretchen0. I like the manual prop, I'm thinking how we could add it to the docs so the users are aware of it.

Maybe we add an entry in https://docs.tresjs.org/advanced/ ?

I'm not sure many users will want it. It's pretty niche. Afaik, no one had complained that Tres was controlling the <TresPerspectiveCamera /> previously.

I just don't want to force people to drop Tres because their use case requires controlling the camera.

I would say you already found it valuable so let's not wait for the community to request it or complain about it, if someone asks, we can direct the user to that documentation.

Outside of the scope of this PR, @andretchen0 I'm strongly thinking about adding a cookbook to the docs similar to this https://docs.astro.build/en/recipes/., maybe is a good candidate?

@andretchen0
Copy link
Contributor Author

andretchen0 commented Feb 7, 2024

Thanks for this @andretchen0. I like the manual prop, I'm thinking how we could add it to the docs so the users are aware of it.

Maybe we add an entry in https://docs.tresjs.org/advanced/ ?
I'm not sure many users will want it. It's pretty niche. Afaik, no one had complained that Tres was controlling the <TresPerspectiveCamera /> previously.
I just don't want to force people to drop Tres because their use case requires controlling the camera.

I would say you already found it valuable so let's not wait for the community to request it or complain about it, if someone asks, we can direct the user to that documentation.

Outside of the scope of this PR, @andretchen0 I'm strongly thinking about adding a cookbook to the docs similar to this https://docs.astro.build/en/recipes/., maybe is a good candidate?

I'm open to whatever.

We've currently got the Lab. Maybe there should be a "tutorials" or "basics" section there?

Here's one way R3F is doing that, for comparison. They've got a lot of examples and basics on their "Getting started" page. That's probably a good move, since most newcomers to a project like Tres will want to see what it can do visually.

One drawback to the current Lab setup is that there's no way to see the code directly. If we keep the current setup, maybe we add a button to go directly to the particular example on Github, like Three.js examples:

Screenshot 2024-02-07 at 17 25 30 copy

@alvarosabu
Copy link
Member

@andretchen0 hey buddy, can you confirm me this would not need to update any docs? If not then we can merge it already

@andretchen0
Copy link
Contributor Author

It would potentially break an existing orthographic camera that the user was updating themselves — which was really the only choice, previously.

Maybe that needs documentation?

@alvarosabu
Copy link
Member

It would potentially break an existing orthographic camera that the user was updating themselves — which was really the only choice, previously.

Maybe that needs documentation?

I hardly think anyone was doing that until now 😂, it's ok then

@andretchen0
Copy link
Contributor Author

I hardly think anyone was doing that until now 😂, it's ok then

I think you're right. Lol

@andretchen0 andretchen0 merged commit 97dcd7b into v4 Mar 6, 2024
@andretchen0 andretchen0 deleted the fix/camera-aspect branch March 6, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants