three.js
three.js copied to clipboard
Camera: Added fovMode
Similarly to the Camera's Sensor Fit in Blender, this lets the user configure the in which dimension the fov is being applied: vertical
, horizontal
or auto
. I'm also proposing to set it to auto
by default.
This affects mobiles specially. Previously the examples looked like this:
![Screen Shot 2020-06-10 at 2 38 00 PM](https://user-images.githubusercontent.com/97088/84321492-38926980-abae-11ea-9127-79194e65a1d0.png)
With the new default it looks like this:
![Screen Shot 2020-06-10 at 2 37 48 PM](https://user-images.githubusercontent.com/97088/84321508-41833b00-abae-11ea-97f3-d4df8f6a8a32.png)
Which I think is what you would expect when the render looks like this in landscape:
![Screen Shot 2020-06-10 at 2 43 08 PM](https://user-images.githubusercontent.com/97088/84321862-f0277b80-abae-11ea-913d-2855d4464c39.png)
Not sure about the name fovMode
. Any suggestions?
/fyi @elalish
Yeah, that seems like a good one to me. I might introduce that configuration to model-viewer as well.
@WestLangley Any chance you can help with the correct hFov
and vFov
formulas?
I like this change 👍
The name might be a big vague, though. When I saw the title of the PR I though this was going to be a function to help adjust the view of the camera to fit an object in the view.
Maybe something like setFovFit
or setFovAxis
?
Renamed it to fovMode
but fovFit
may also work? fovAxis
also sounds good to me.
Can you explain what camera.fov
represents now? It used to be the vertical field-of-view in degrees. Is it still the vertical-field-of-view?
Also, how would you describe what getEffectiveFOV()
returns?
Can you explain what
camera.fov
represents now? It used to be the vertical field-of-view in degrees. Is it still the vertical-field-of-view?
camera.fov
is the field-of-view in degrees but, when fovMode
is auto
it's vertical or horizontal depending on the aspect ratio.
Also, how would you describe what
getEffectiveFOV()
returns?
Not sure what that method is for 😅
Not sure what that method is for
It is for when camera.zoom
!== 1. TBH, I was unaware we were no longer using it.
I personally favor this approach compared to #21190. To me, it's okay if the final semantics of fov
depends on another property. According to this PR, it's the field-of-view in degrees. Whether or not it is the vertical or horizontal depends on fovMode
. That seems understandable.
Sidenote: Couldn't we remove getEffectiveFOV()
? The method is neither used in the examples nor in the editor. If users still require this value, they can compute it on app level.
I do not think fov
should have multiple meanings, so I am not in favor of this.
The API we have works fine.
I support the solution I proposed in https://github.com/mrdoob/three.js/pull/21190#issuecomment-774738902. Alternatively,hfov
could be implemented with a getter/setter, but that is an implementation detail.
@Mugen87 said:
Couldn't we remove getEffectiveFOV()? The method is neither used in the examples nor in the editor.
I suggest we avoid using "it's not used in the examples" as a justification to remove a feature. There are multiple methods that are not used in the examples.
The examples exist to demonstrate three.js features. Add an example, instead of removing the feature.
I suggest we avoid using "it's not used in the examples" as a justification to remove a feature. There are multiple methods that are not used in the examples.
Fair enough! I should have also mentioned that I've never seen the method used in the wild for the last couple of years.
Fair enough! I should have also mentioned that I've never seen the method used in the wild for the last couple of years.
Granted.
It may have been used in CanvasRenderer
, or by users currrently to calculate the visible height at a given distance.
Thing is, I expect most users do not have the math skills to derive the formula used in the method. That may be a reason to justify retaining it.
To compute projection matrix with .fov as horizontal instead of vertical
this.perspectiveCamera.aspect = w / h;
this.perspectiveCamera.updateProjectionMatrix();
if(this.fovAxis==='horizontal'){
let e = this.perspectiveCamera.projectionMatrix.elements;
e[5]=this.perspectiveCamera.aspect;
e[0]=1;
}
Sorry if this is off-topic but I just implemented this for a friend, and then found this thread, so I thought I'd drop it in here.
For the record, I still prefer what I said previously.
What is fovMode ?deeply explanation, please.
@Ben6jamin Please ask for help/explanations at the forum.
@Ben6jamin You can also ask to https://chat.openai.com/chat but the answer may be kind of made up 😁
![Screen Shot 2022-12-07 at 11 51 43 AM](https://user-images.githubusercontent.com/97088/206076451-76d8861e-0bae-4bc6-9650-5a529f89dc18.png)
@WestLangley Seems like you were the only person opposing this change. Any chance you have changed your mind?
I prefer https://github.com/mrdoob/three.js/pull/21190#issuecomment-774738902 instead of FOVmode
.
Actually, horizontalFov
could be implemented as a getter/setter. (That is how we handle SpotLight.power
.) I like that approach.
What I'm proposing here is to follow Blender's Sensor Fit
feature:
It would be set to auto
by default, which may break things but doing camera.fovMode = 'vertical'
should fix it.
The benefit is that models will fit in any viewport by default without the developer having to know what horizontal/vertical fov is. They will not need to understand these concepts unless they have to.
What you're suggesting is forcing new users to understand these concepts from the get go. Any projects that new users do will result in models not fitting the viewport in mobile phones until they understand horizontal/vertical fov concepts. And chances are they never will...
This change would result in better looking content by default and I can't imagine any side effects, but I would like to hear any side effects you are seeing and why you're blocking it 🤔
I am not blocking it. I don't like blockers. I am just expressing my preference.
Why can't camera.fov
mean just one thing?
I think the answer is, it can. I think you can implement your auto-fit feature without redefining the meaning of camera.fov
.
With the getter/setter
I proposed, users could do camera.hFov = value
if they want. Or, as you prefer, the fov could be set for them automatically by your sensor-fit feature.
I expect it is the feature that you really care about, not the parameterization. At least I hope so. :-)
@mrdoob It is late here. Perhaps I am misunderstanding what you mean by fovMode
...
I am not blocking it. I don't like blockers. I am just expressing my preference.
It's probably a language thing but if you say you prefer a different approach to me it sounds that you're not okay with what's being proposed and the different approach should be used instead.
Then the issue gets put on hold until I have time to understand your reasoning and try to reach an agreement.
What happened here is that many other issues piled up on top and I forgot about it.
I expect it is the feature that you really care about, not the parameterization. At least I hope so. :-)
I care about the quality of the content being produced and the user experience.
If I go to a website that uses three.js on my mobile and the model doesn't fit in the viewport (many of the examples have this issue) my first reaction is trying to think if there's anything the library could do so the developer doesn't need to know/worry about this in order to provide a good experience.
@mrdoob It is late here. Perhaps I am misunderstanding what you mean by
fovMode
...
Currently we have this:
The idea is to change the current behavior so instead of a "vertical field of view" it uses the largest dimension of the viewport by default.
The user could then control the behavior by setting fovMode
to auto
(default), horizontal
or vertical
.