-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Rename PerspectiveFrustum.fov to PerspectiveFrustum.fieldOfView #5650
Comments
If we're breaking the FOV API, we could consider changes to behavior as well. Originally, this was specified as But recently, I've had complaints that this behavior is not as friendly to wide-screen displays. Some displays and desktop arrangements typically fill the full height consistently, as the height is relatively smaller, and the width is more variable, with side UI panels coming and going affecting the width. Having the FOV based on this larger variable-width dimension is bad because the effective vertical FOV ends up changing in a jarring way when side panels come and go. It would be more pleasing to the eye for these side panels to simply crop or un-crop the display, changing the horizontal FOV without changing the vertical FOV. How to implement this without re-introducing the original FOV problem is up for debate. One idea I had would be to go back to a |
Here's a more recent idea: We change The existing read-only I haven't tried any of this to see what it "feels" like to a user. Obviously a good bit of hands-on usability testing would be needed before any one strategy was accepted. |
I created a branch I didn't attempt to do the full rename described above, as that's actually a larger change and I wanted to get other peoples' opinions on this behavior first. The rename will be useful though for making this a true "breaking" change with a new API. The old behavior can be replicated by setting Here's a Sandcastle demo with an FOV HUD that works in both
|
Latest stable Cesium: Sandcastle demo This branch: Sandcastle demo Try resizing the inset Cesium window horizontally and vertically. In master, FOV X and FOV Y both max out at 60 degrees. On this branch, FOV X maxes out at 89.95, and FOV Y maxes out at only 45. |
I'm all for the new behavior. I don't have a widescreen monitor to test with, but I tried both demos with a squashed view and like the new way. |
@emackey do you plan to make a pull request for this? |
I do. I'm just swamped with other stuff right now. I have a |
See https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Core/PerspectiveFrustum.js#L42
Also search to see if
fov
is used anywhere else in the public Cesium API. The abbreviation is fine in the private interfaces and implementations.The text was updated successfully, but these errors were encountered: