godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.07k stars 68 forks source link

Deprecate Camera3D's `PROJECTION_FRUSTUM` projection mode #8536

Open chocola-mint opened 7 months ago

chocola-mint commented 7 months ago

Describe the project you are working on

I was writing unit tests for Camera3D and had to figure out how the "frustum mode" is mathematically defined, so I could come up with appropriate testcases.

Describe the problem or limitation you are having in your project

The "frustum mode" of Camera3D, enabled by setting the projection property to PROJECTION_FRUSTUM, has the following issues:

I've also asked about this on RocketChat and no one really knew how this projection mode is actually defined either. The name is confusing and looking it up yields results about perspective projections (because of course, camera frustums are usually discussed in the context of perspective projections).

For my unit test PR, I eventually opted to just not test this projection mode as it is poorly-defined, but I can imagine unwitting users assuming that this frustum mode would work as expected with every other Camera3D function, only to find themselves with incorrect results.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Completely removing code related to PROJECTION_FRUSTUM might not be realistic in the short term (and however few, there may still be people whose games depend on the buggy implementation). I propose that, for starters, PROJECTION_FRUSTUM should be marked as deprecated in the documentation.

Since frustum_offset is only used by PROJECTION_FRUSTUM, it makes sense to deprecate frustum_offset as well.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Add an additional line in the documentation for ProjectionType denoting PROJECTION_FRUSTUM's deprecated status. Also do the same for frustum_offset.

If this enhancement will not be used often, can it be worked around with a few lines of script?

N/A

Is there a reason why this should be core and not an add-on in the asset library?

N/A

Calinou commented 7 months ago

Frustum projection can do things that neither perspective nor orthogonal can do, so it needs a proper replacement before it can be deprecated.

the frustum_offset property to achieve tilted camera effects like Doom's Y-shearing,

It's also useful for architecture visualization when taking photos of tall buildings (the same principle applies to 3D renders).

AThousandShips commented 7 months ago

See:

chocola-mint commented 7 months ago

I see. I assume the current plan would be to wait until the custom projection mode comes to fruition, and then convert frustum projection to an add-on?

It's still quite unexpected that frustum projection is the only projection that doesn't actually support any of the Camera3D functions related to the camera frustum, though. Putting a small note in the documentation that makes this clear would be appreciated, unless there are still plans to fix the issue.

AThousandShips commented 7 months ago

I don't see the real reason to remove the current functionality, just integrate it

lyuma commented 7 months ago

The frustum projection makes it possible to implement mirrors and portals in a relatively straightforward way. We are using it in our project like that. It seems valuable if for that reason alone.

See the reference code here (the use_screenspace=false case works without engine modifications.) https://github.com/V-Sekai/avatar_vr_demo/tree/master/addons/V-Sekai.xr-mirror

elvisish commented 2 weeks ago

I am using it for Y-shearing exclusively in 3.5 as it is the only possible way, but if it was removed in 4.x I'd no longer be able to. I do think frustrum_offset should be taken into account when raycasting, however, so there's definitely room for improvement, just not removal.