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 new Camera3D type: Oblique #89140

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

Conversation

qaptoR
Copy link

@qaptoR qaptoR commented Mar 4, 2024

Closes: godotengine/godot-proposals#501
Partial solution to this proposal: godotengine/godot-proposals#2713

This PR should not necessarily be a reason to close the proposal 2713, it is merely a straightforward implementation of oblique near plane clipping to implement a portal--or other similar--mechanic.

This PR is essentially an update to my previous PR: #64876
The main difference is that shadows were fixed.

This other PR attempts to completely solve proposal 2713 mentioned above, but apparently is having issues with shadows. Considering that its commits try to implement custom projections by adding a new camera type like with my PRs, there is a good chance the way I fixed shadows could also work there too: #84454

The solution was that along with the main Oblique camera projection a separate, normal Perspective camera projection needed to be included with the CameraData used in the RenderSceneCull::_render_scene() that was used explicitly for lighting and shadows.

However, regarding the potential question of whether one PR should be included over the other as a solution, clearly the more general solution can see a wider range of uses. However, I believe this implementation of an Oblique Near Plane camera still has merit based on the majority use case of creating a portal mechanic, etc where oblique near clipping is necessary.

This is because the interface for the Oblique camera included in this PR has been simplified so that users are only required to provide at a minimum the transform of the plane they wish to use as the near clipping plane of the camera using set_oblique_plane_from_transform().

Additionally, because all of the required math (especially the matrix math) is happening in C++, the effect is as efficient as possible, without requiring steps to first camera.get_projection, then doing matrix manipulation, then camera.set_projection all in GDScript.

Alternatively, because the Oblique Near Plane matrix manipulations "... can be applied to any projection matrix, ..." (https://terathon.com/lengyel/Lengyel-Oblique.pdf, pg. 2) it's possible that all camera types (including the other proposed custom projection camera) could benefit from a general is_oblique_near_plane boolean toggle instead of this dedicated oblique camera which only implements the algorithm for a Perspective projection.

Video 1: Current state of personal portal game mechanic
https://youtu.be/A_-2nTyKVkU
Notice that the portal viewports are not blocked by the surface the portal is on (the issue with using a standard perspective camera), and that shadows and lighting appear the same when looking through the portal (as evidenced by walking through the portal where a seamless transition occurs).

Video 2: Previous video showcasing dynamically the difference oblique near plane clipping makes
https://youtu.be/PipVd6wRnMk?si=lCThReaKyO-smBaP
Note that this older video did not properly handle shadows, and the viewport texture color and environment lighting were all wrong giving it that washed out look (both fixed in Video 1).

Image 1: Portal effect using Perspective camera
image

Image 2: Portal effect using Oblique camera
image

Image 3: Oblique camera editor interface
image

core/math/projection.h Outdated Show resolved Hide resolved
servers/rendering/renderer_scene_cull.cpp Outdated Show resolved Hide resolved
servers/rendering/renderer_scene_cull.cpp Outdated Show resolved Hide resolved
servers/rendering/renderer_scene_cull.cpp Outdated Show resolved Hide resolved
scene/3d/camera_3d.h Outdated Show resolved Hide resolved
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, I'm having trouble setting up a planar water reflection with this PR on Linux + NVIDIA.

I get frequent engine freezes (with VK_DEVICE_LOST error and the entire PC appearing to freeze) if invalid transforms are entered in the Oblique camera projection mode, so it's hard to test. I had the same problems when testing #84454.

PS: I wonder if setting projection from a transform should be offered in the editor somehow, so you can set up portals and planar reflections without any script. The RemoteTransform3D node makes it possible to adjust the camera position without a script, but I still need to assign a script to call Camera3D.set_oblique_plane_from_transform(). Ideally, it should be possible to set this up without any script to make this feature more accessible to artists and level designers.

Testing project: test_pr_89140.zip

@qaptoR
Copy link
Author

qaptoR commented Mar 4, 2024

@Calinou
There are two camera member variables: oblique_normal, and oblique_position, which are exposed in the editor for this purpose.
Use the transform origin for position and transform forward vector for the normal.

@Calinou
Copy link
Member

Calinou commented Mar 4, 2024

@qaptoR Can you upload a testing project? I haven't been able to figure it out so far.

core/math/projection.cpp Outdated Show resolved Hide resolved
doc/classes/Camera3D.xml Outdated Show resolved Hide resolved
doc/classes/Camera3D.xml Outdated Show resolved Hide resolved
@qaptoR
Copy link
Author

qaptoR commented Mar 4, 2024

@Calinou
I just got home from classes.

  • I will post a minimum testing project asap.
  • I will also test the project you posted on my system to see if everything works:
    • I'm building/testing on Windows 10, Intel 10900K, Nvidia 3090.
  • then work on the code revisions that have been posted so far.

@qaptoR
Copy link
Author

qaptoR commented Mar 4, 2024

@Calinou
Still going to work on a minimum test project, but in the interim I figured out what was wrong with your test scene.

The path to the Camera3D in water.gd on line 12 should be:
$Water/SubViewport/Camera3D.set_oblique_plane_from_transform(global_transform)

I was unable to open the test scene until this was corrected because it is an @tool script.
From there the oblique camera works correctly for me.

@fire
Copy link
Member

fire commented Mar 4, 2024

@lyuma You were looking into this. #85529

@qaptoR
Copy link
Author

qaptoR commented Mar 4, 2024

@Calinou
Here is a minimum portal test project.
Some things to note about the scene I set up

  • the BackingMesh's are invisible to the portal cameras purposefully because they're mostly there to keep tabs on where the portals are as you move around
  • when you look through the portal at the other portal, the visual distortion that looks like you're painting on the portal with what else is in view is normal because we're not handling recursion
  • there are visual indicators behind each portal so that when you switch the cameras to perspective mode it's obvious what the oblique camera is culling

EDIT:
Fixed a quirk with the test scene where the viewport textures may appear pink when first loaded.

89104_min_test.zip

@qaptoR qaptoR force-pushed the master_oblique_camera branch 2 times, most recently from 2242e08 to cb0a2f4 Compare March 5, 2024 05:39
Copy link
Contributor

@lyuma lyuma left a comment

Choose a reason for hiding this comment

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

Interesting approach. At least in non-XR cases, I could see this being a viable alternative to my PR...

I'm a bit unclear in how to apply this design to XR, since I'll need to copy the projection matrix of the HMD and then make it oblique. See my reference XR mirror implementation

@@ -2591,7 +2617,23 @@ void RendererSceneCull::render_camera(const Ref<RenderSceneBuffers> &p_render_bu
camera->znear,
camera->zfar,
camera->vaspect);
} break;
case Camera::OBLIQUE: {
Quaternion oblique_plane = get_camera_oblique_quaternion(p_camera);
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is a plane, not a quaternion, then the function should be named get_camera_oblique_plane and the return type should be Plane, not Quaternion.

If Godot's Plane type is somehow not suitable for this case, then it should use Vector4 rather than Quaternion.

Copy link
Author

Choose a reason for hiding this comment

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

The drawback to using a plane is that must be converted to quaternion before it can be used in the step p_oblique_plane * (2.0F / p_oblique_plane.dot(q)) because the Plane type does not support mathematical operations.

Quaternion was used because at the time (almost 3 years ago) that this camera was originally implemented the Vector4 type did not exist.

Copy link
Contributor

Choose a reason for hiding this comment

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

must be converted to quaternion before

A conversion should be ok. it should only be one line of code to perform the conversion. performance is not at issue here: it would improve readability/maintainability to avoid use of quaternion.

and the calculation there should be using Vector4, not quaternion.

editor/plugins/gizmos/camera_3d_gizmo_plugin.cpp Outdated Show resolved Hide resolved
Comment on lines +224 to +217
<member name="oblique_normal" type="Vector3" setter="set_oblique_normal" getter="get_oblique_normal" default="Vector3(0, 1, 0)">
The desired normal vector of the world space plane used by an oblique camera as an oblique near clipping plane.
</member>
<member name="oblique_offset" type="float" setter="set_oblique_offset" getter="get_oblique_offset" default="0.0">
The offset value for the oblique plane along its forward vector.
</member>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced about this using a separate projection mode from perspective. Most of the code seems to mimic perspective, just with some extra options. Perhaps we can condition this on whether those extra options are in use (for example, if oblique_offset == Vector3.ZERO and oblique_normal == Vector3(0,1,0) then it is a standard perspective. (is that default value of (0,1,0) correct?)

Copy link
Author

Choose a reason for hiding this comment

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

I've mentioned this in the top PR comment, which is that Lengyel's algorithm can technically be applied to any projection matrix, and it will remain the same except for the changes to the near clipping plane. My suggestions is to add a boolean flag to the camera which appends the matrix manipulation algorithm to the end of all the camera types. Rather than duplicating a single camera type.

This would also further justify the added second shadow_projection included in the CameraData which is used to maintain the shadows and lighting, as it is currently overhead which is only necessitated by the Oblique camera.
If all camera types had the option of Oblique near plane projections then it would be further justified.

Copy link
Contributor

Choose a reason for hiding this comment

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

My suggestions is to add a boolean flag to the camera which appends the matrix manipulation algorithm to the end of all the camera types. Rather than duplicating a single camera type.

it sounds reasonable to me. we just have to be careful not to add overhead in the common case (not using oblique)

@@ -3155,7 +3201,7 @@ void RendererSceneCull::_render_scene(const RendererSceneRender::CameraData *p_c
cull_data.visible_layers = p_visible_layers;
cull_data.render_reflection_probe = render_reflection_probe;
cull_data.occlusion_buffer = RendererSceneOcclusionCull::get_singleton()->buffer_get_ptr(p_viewport);
cull_data.camera_matrix = &p_camera_data->main_projection;
cull_data.camera_matrix = &p_camera_data->shadow_projection;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it correct for culling to use the plain perspective projection?

If the intent is to use this for planar reflections, wouldn't a frustum camera projection be more appropriate for culling purposes?

Copy link
Author

Choose a reason for hiding this comment

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

The oblique camera is only implemented as essentially a 'trick' version of the Perspective camera. This is because it was developed with games that use that camera in mind.

As mentioned in an above comment, the shadow_projection as overhead is only in service to this one camera, for every other type it will be a duplicate of the main_projection, which is why I call it overhead.

I don't know enough about the rendering pipeline or lighting system to understand why using the Oblique projection breaks the shadows (a problem with my first PR for this feature), but my guess is that for every Projection, any attempt to use the Oblique_Projection variation will require the shadows and lighting to use the original *_Projection.

This is why I feel that adding the boolean toggle for all cameras would justify the overhead.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I get the idea of adding a boolean to enable the overhead. I can't entirely say I understand if it's correct for light/shadow to use a separate projection matrix (might it also add complexity to the fragment shader?)

the comment I am trying to make here is that culling could be based on the frustum. I show an example here of how using the frustum projection for culling can be helpful: #85529 (comment)
it would be good to double check if occlusion culling works correctly since the origin point in a classical perspective projection might be outside the room.... while using a frustum might make this check more correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Culling currently assumes the near and far planes are parallel. Passing the non-oblique projection probably avoids some troubles there. Now, the consequence is that objects between the oblique and the non-oblique planes won't be culled on the CPU (they will be on the GPU anyway). This is probably an acceptable trade-off though.

Comment on lines 318 to 320
if (p_flip_fov) {
p_fovy_degrees = get_fovy(p_fovy_degrees, 1.0 / p_aspect);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what p_flip_fov is for

Copy link
Author

Choose a reason for hiding this comment

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

this isn't part of my PR, This is just part of the Projection::set_perspective code. Almost everything about the Oblique camera is a duplication of the Perspective camera because it was the most straightforward way to implement oblique near plane clipping for my project when I originally started. This is why I've made the suggestion to switch to a more general approach using a boolean toggle to enable oblique near plane clipping for any camera projection type.

Comment on lines 321 to 340

real_t sine, cotangent, deltaZ;
real_t radians = Math::deg_to_rad(p_fovy_degrees / 2.0);

deltaZ = p_z_far - p_z_near;
sine = Math::sin(radians);

if ((deltaZ == 0) || (sine == 0) || (p_aspect == 0)) {
return;
}
cotangent = Math::cos(radians) / sine;

set_identity();

columns[0][0] = cotangent / p_aspect;
columns[1][1] = cotangent;
columns[2][2] = -(p_z_far + p_z_near) / deltaZ;
columns[2][3] = -1;
columns[3][2] = -2 * p_z_near * p_z_far / deltaZ;
columns[3][3] = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate code of set_perspective (non-stereo overload)

Note also that for XR usage, it might be good to consider the possibility of supporting this in stereo in the future..

Comment on lines 353 to 356
columns[0][2] = c.x;
columns[1][2] = c.y;
columns[2][2] = c.z + 1.0F;
columns[3][2] = c.w;
Copy link
Contributor

@lyuma lyuma Mar 5, 2024

Choose a reason for hiding this comment

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

I would avoid taking a shortcut here, in case we do want to use this in XR. The math is the same (you'll be subtracting 0 for x, y and w and subtracting -1 for z, just like in your code) but this way it can be more easily refactored for other projection types in the future.

Suggested change
columns[0][2] = c.x;
columns[1][2] = c.y;
columns[2][2] = c.z + 1.0F;
columns[3][2] = c.w;
columns[0][2] = c.x - columns[0][3];
columns[1][2] = c.y - columns[1][3];
columns[2][2] = c.z - columns[2][3];
columns[3][2] = c.w - columns[3][3];

Copy link
Author

@qaptoR qaptoR Mar 5, 2024

Choose a reason for hiding this comment

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

I wasn't taking any shortcuts. These are the exact steps laid out in Eric Lengyel's algorithm (https://terathon.com/code/oblique.html).

I don't know enough linear algebra to determine if your suggestion will observe the same results for all projections, but as I quoted in the top PR comment, Eric's paper suggests that this algorithm is correct for any projection matrix, which I assume means that it can be used confidently unchanged.

This is why I think changing the implementation to a boolean toggle for all camera types (including a possible custom_projection type like the one proposed by PR 84454). Lengyels algorithm is itself basically a short-cut that sacrifices depth precision for oblique clipping with any projection.

Copy link
Contributor

Choose a reason for hiding this comment

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

The generalization is explained tersely here: https://aras-p.info/texts/obliqueortho.html
from what it sounds like, this correction enables the math to work for any projection matrix, not just perspective.

core/math/projection.h Outdated Show resolved Hide resolved
@lyuma
Copy link
Contributor

lyuma commented Mar 5, 2024

another important feature in addition to portals is Planar reflections, which require the ability to flip the final projection matrix (UV space), since this is necessary to preserve chirality of triangles (backface culling) in a mirrored setting.

for example, in my code, I do this by adding a flip in the projection here:

p_cam.set("override_projection",  Projection(Transform3D.FLIP_X) * proj *  Projection(Transform3D.FLIP_X))

and then perform the flip again in the shader to undo this:

		uv = vec2(1.0 - SCREEN_UV.x, SCREEN_UV.y);

Without this flipping, it is quite complicated to implement mirrors: there will need to be some way to communicate that culling is inverted (In Unity users do this with GL.invertCulling = true but inverting UV space is IMHO far simpler and doesn't require changing the renderer.

Just thinking aloud, but this is a problem I ran into, though it was easily solved in my case since I was using the PR with a customizable projection matrix.

@theraot
Copy link
Contributor

theraot commented Mar 6, 2024

An oblique projection is a parallel projection.

Here is a picture from Wikipedia:

image

It is commonly used in technical drawing. But it has also seen use in video game art, as most 2D RPGs use some kind of oblique projection (where you see the top face and the front face on full size). A notable case is EarthBound that uses a cabinet projection.

And thus such projection could be useful for anybody trying to mimic the look of those games with a 3D scenes (consider for example the trick used for The Legend of Zelda: A Link Between Worlds).


But it seems you are not doing that. Instead it seems that what you are doing is an oblique frustum projection. Please call it what it is. PROJECTION_OBLIQUE is wrong.

@qaptoR qaptoR force-pushed the master_oblique_camera branch 2 times, most recently from 1d33af8 to 121ecf5 Compare March 8, 2024 20:38
@qaptoR
Copy link
Author

qaptoR commented Mar 8, 2024

I've updated the PR to reflect the alternative approach to applying the oblique algorithm.
Now, there is a flag use_oblique_frustum which is currently only available for the perspective and orthogonal cameras because as far I am aware the more general algorithm brought to my attention by @lyuma only works for those two projections.

This new approach also reduces the amount of duplicated code that was used for the oblique camera previously.

I've also updated the test project I posted before with a new scene meant to test oblique algorithm on orthogonal cameras.
Some notes about the project in general

  • for both scenes the portal viewport paths need to be reset when the scene is loaded: PortalSide[AB]>mesh>material>shader_parameter/TextureAlbedo>viewport_path
  • the orthogonal_oblique.tscn portal camera frustums seem to be affected in unreliable ways. The boxes/prisms in the scene are placed such that if they are moved further/closer from the position you find them in when the scene loads will result in them being culled by the far plane.
    • on the good side, the near plane does appear to be culling appropriate to the oblique plane

89104_min_test.zip

EDIT:
I think I found the issue. I'll fix it when I get home from class. I'm referencing the wrong index at
projection.cpp:line108 where it should be [0][3] instead of [0][1].

EDIT:
Okay, the columns reference is fixed.

@qaptoR qaptoR force-pushed the master_oblique_camera branch from 121ecf5 to 04ebd2d Compare March 8, 2024 21:01
allows the specification of a world plane for near clipping;
applies matrix manipulation to generate oblique near plane.
@qaptoR qaptoR force-pushed the master_oblique_camera branch from 04ebd2d to d5d2426 Compare March 11, 2024 18:45
@Ribiveer
Copy link

Ribiveer commented Jul 27, 2024

Hi there! I'm currently working on a project that needs portals and mirrors, and I'm in dire need of this feature. I see it's been completely written up and working well, which is awesome! Since it's been a few months since this was finished, I'm wondering: are there's any blocks on merging? I'm quite new to contributing to open-source projects, but if there's anything I can dedicate time to so this feature gets integrated as soon as possible, I'd love to know what needs to be done.

@qaptoR
Copy link
Author

qaptoR commented Jul 28, 2024

@Ribiveer I'm not here to say whether anything can be done sooner or later to merge, I'm new to this process as well, despite having made several PRs over the years, they all end up in stasis and I suspect that is either because a) i don't push the godot team enough to remind them of my features, or b) because my PRs are predominantly new features there is an opposing force to them being merged which is that strive for stability in the core engine which new features tend to disrupt leading to slow integration of an already long backlog.

what I would recommend is getting comfortable building from source, and merging different branches/prs into a single working engine that you call your own. this is what I do, and it means that you can have all the features that you want that are stuck in stasis.

obviously I hope that my work gets included in the engine core eventually and I can be counted officially as a contributor, but I also want the current features of the engine to be robust and bug free, so I am fine with the godot team focusing their effort there.

I am also grateful that you appreciate my work, and I hope that if you do choose to use it still in the interim that it serves well enough for the purpose you intend.

@Ribiveer
Copy link

@qaptoR Thank you for your thoughtful reply! I think I've been delaying the inevitable, getting comfortable building from source is indeed the best strategy here. In the meantime I hope for you that your PR's get approved! I've looked through your repository and they seem like useful additions indeed.

@Ribiveer
Copy link

Ribiveer commented Jul 30, 2024

I've copied this pull request's commit in my fork, and have been testing it today.

It's working extremely well! It surprised me that the given coordinates were in world space. While this perfectly serves most purposes this feature will used for (most of the time it will be used to align the clipping plane with an object in the world), an argument could also be made that, since all other camera parameters are local to the camera, this one should be as well. Personally though, this is perfect for my (and I assume most people's) needs!

It doesn't seem to play nicely with the automatic LOD system, however. Looking at the clipping plane from straight on, Godot assumes a normal distance for meshes. But when looking at the clipping plane at an angle, Godot seems to assume more and more radical distances, causing objects to assume a lower LOD than they should. I looked around and found issue #81995 and #76436 that might be related, but further investigation would be necessary for implementation in Godot.

For my own purposes, this PR perfectly suits my needs, since I am not planning on using Godot's built-in LOD system. For integration into the engine there's either a fix needed for working with the LOD system, or it might actually work itself out when the LOD system gets worked out.

That, or it might have already been worked out in the master branch, and my using of 4.2's stable branch messes something up. Keeping open that possibility!

Thank you so much for making this PR!

@theraot
Copy link
Contributor

theraot commented Aug 17, 2024

Now that 4.3 is out, there is another chance for this feature.

It seems everything from prior reviews has been addressed. So I hope it goes smoothly.

@miskatonicstudio
Copy link

Thank you @qaptoR for this contribution! I am also experimenting with portals and was initially disappointed that Godot doesn't support "oblique projection", only to discover that your PR adds this feature! ❤️ I compiled the engine from source using your branch and it works exactly like expected! I didn't check all the shadow and LOD issues mentioned above, but for my limited experiment, this is exactly what I need 😃

I hope that your PR will be merged soon and Godot will start supporting this awesome feature out-of-the-box 😉

@AThousandShips AThousandShips changed the title Implementation of new Camera3D type: Oblique Add new Camera3D type: Oblique Sep 4, 2024
@Sooshiko
Copy link

This feature would be very helpful for water reflection as you can go through water like a portal even though your position doesn't change. Especially if it works for compatibility renderer as we don't have access to screen_space reflection.

@Riordan-DC
Copy link

Awesome looking PR! Really looking forward to its merge. Fixes this awful artefact with planar reflections. Pink geometry should be clipped by the planar reflection camera which should be possible with an oblique camera.
image

<member name="projection" type="int" setter="set_projection" getter="get_projection" enum="Camera3D.ProjectionType" default="0">
The camera's projection mode. In [constant PROJECTION_PERSPECTIVE] mode, objects' Z distance from the camera's local space scales their perceived size.
</member>
<member name="size" type="float" setter="set_size" getter="get_size" default="1.0">
The camera's size in meters measured as the diameter of the width or height, depending on [member keep_aspect]. Only applicable in orthogonal and frustum modes.
</member>
<member name="use_oblique_frustum" type="bool" setter="set_use_oblique_frustum" getter="get_use_oblique_frustum" default="false">
Toggle for applying oblique near plane frustum culling.
Copy link
Contributor

@Mickeon Mickeon Dec 4, 2024

Choose a reason for hiding this comment

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

Suggested change
Toggle for applying oblique near plane frustum culling.
If [code]true[/code], enables oblique near plane frustum culling.

I would personally elaborate on what this means, as well.

<return type="void" />
<param index="0" name="oblique_transform" type="Transform3D" />
<description>
Sets the [member oblique_normal] and [member oblique_position] values of an oblique projection camera using a the origin and forward vector of the transform argument. For ease of using plane meshes as portals and mirrors.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Sets the [member oblique_normal] and [member oblique_position] values of an oblique projection camera using a the origin and forward vector of the transform argument. For ease of using plane meshes as portals and mirrors.
Sets the [member oblique_normal] and [member oblique_position] values of this camera, through the given [param oblique_transform]'s origin and forward vector. Can be useful for plane meshes used as portals or mirrors. See also [member use_oblique_frustum].

@@ -202,12 +209,24 @@
<member name="near" type="float" setter="set_near" getter="get_near" default="0.05">
The distance to the near culling boundary for this camera relative to its local Z axis. Lower values allow the camera to see objects more up close to its origin, at the cost of lower precision across the [i]entire[/i] range. Values lower than the default can lead to increased Z-fighting.
</member>
<member name="oblique_normal" type="Vector3" setter="set_oblique_normal" getter="get_oblique_normal" default="Vector3(0, 1, 0)">
The desired normal vector of the world space plane used by an oblique camera as an oblique near clipping plane.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the normal vector of the mentioned world space plane be provided in World space or in View space to this function ? Either way this should be clarified.

Copy link
Contributor

@Flarkk Flarkk Dec 5, 2024

Choose a reason for hiding this comment

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

Gave it further thinking, and in the end I believe the oblique plane's normal and position attributes should be expressed in view space for consistency.
In other words these shouldn't make the camera looking different when it moves around (we're setting camera attributes after all, not scene attributes).
You purposefully provide
set_oblique_plane_from_transform() that allows users to set the oblique plane's attributes from world space vectors already.

@fire fire requested a review from lyuma December 4, 2024 20:17
Comment on lines +147 to +150
int dot = int(camera->oblique_normal.dot(camera->oblique_position - camera->transform.origin) >= 0.0f ? 1.0f : -1.0f);
Vector3 cam_space_pos = camera->transform.xform_inv(camera->oblique_position);
Vector3 cam_space_normal = camera->transform.basis.xform_inv(camera->oblique_normal) * dot;
real_t cam_space_dst = -cam_space_pos.dot(cam_space_normal) + camera->oblique_offset;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should be able to do directly :
real_t cam_space_dst = -(camera->oblique_position - camera->transform.origin).dot(camera->oblique_normal) + camera->oblique_offset; in world space.
The inverse transform by basis shouldn't have an influence on the dot product (provided basis is orthonormal, but the transform.basis of a Camera3D is always made orthonormal).
Give it a try and let me know.

Comment on lines +215 to +217
<member name="oblique_offset" type="float" setter="set_oblique_offset" getter="get_oblique_offset" default="0.0">
The offset value for the oblique plane along its forward vector.
</member>
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for having an offset ? Seems like it plays more or less the same role than the znear.
Said differently : what can we achieve with this offset we can't get by just adjusting the camera's znear ?

Comment on lines -2573 to +2594
Projection projection;
Projection main_projection;
Projection shadow_projection;
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested effects (SSR, SSIL, Fog, GI, etc...) to make sure they don't break with the oblique main_projection ? You can use this test scene : reprojection.zip

@@ -2626,7 +2650,7 @@ void RendererSceneCull::render_camera(const Ref<RenderSceneBuffers> &p_render_bu
}

if (view_count == 1) {
camera_data.set_camera(transforms[0], projections[0], false, camera->vaspect, jitter, camera->visible_layers);
camera_data.set_camera(transforms[0], projections[0], projections[0], false, camera->vaspect, jitter, camera->visible_layers);
} else if (view_count == 2) {
camera_data.set_multiview_camera(view_count, transforms, projections, false, camera->vaspect);
Copy link
Contributor

Choose a reason for hiding this comment

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

So the Oblique camera is not supported in XR, right ?
If yes, this should be mentioned somewhere, at least in the documentation

@@ -3155,7 +3201,7 @@ void RendererSceneCull::_render_scene(const RendererSceneRender::CameraData *p_c
cull_data.visible_layers = p_visible_layers;
cull_data.render_reflection_probe = render_reflection_probe;
cull_data.occlusion_buffer = RendererSceneOcclusionCull::get_singleton()->buffer_get_ptr(p_viewport);
cull_data.camera_matrix = &p_camera_data->main_projection;
cull_data.camera_matrix = &p_camera_data->shadow_projection;
Copy link
Contributor

Choose a reason for hiding this comment

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

Culling currently assumes the near and far planes are parallel. Passing the non-oblique projection probably avoids some troubles there. Now, the consequence is that objects between the oblique and the non-oblique planes won't be culled on the CPU (they will be on the GPU anyway). This is probably an acceptable trade-off though.

view_count = 1;
is_orthogonal = p_is_orthogonal;
vaspect = p_vaspect;

main_transform = p_transform;
main_projection = p_projection;
main_projection = p_main_projection;
shadow_projection = p_shadow_projection;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather call it culling_projection as it is what it is used for in your code (except the open question of effects).

Copy link
Contributor

@Flarkk Flarkk left a comment

Choose a reason for hiding this comment

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

Generally speaking I find this PR on the right track.

My key concern is that we add world space attributes to the camera. This means the camera would look different depending on its position.

The pattern used instead in such cases in Godot is to set the path to another node (the portal) as an attribute of the dependent node (the camera), and makes the latter retrieve its transform when needed.

I believe this design would simplify the Camera3D interface even further as only one NodePath field (or equivalent) would be added to Camera3D. We could call it Mirror node or something similar.

Some attention is needed on effects (SSR, Fog, GI, etc...) : do they still work with this PR ?

Also, you should rename the PR as it's not about a new camera type anymore. See Edit below though


Edit - That could even be a new RemoteCamera3D type inheriting Camera3D, that not only takes the portal object as a NodePath but also the main Camera3D. RemoteCamera3D would adjust its position automatically relative to both the mirror/portal's transform and the main camera position, similarly to (but not exactly as) how RemoteTransform3D operates

Comment on lines +218 to +220
<member name="oblique_position" type="Vector3" setter="set_oblique_position" getter="get_oblique_position" default="Vector3(0, 0, 0)">
The world space position of the plane used by an oblique camera as an oblique near clipping plane.
</member>
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally we don't need an oblique position neither (see my comment on the oblique offset). The oblique near plane should be fully determined by the oblique normal and the camera znear value. Of course it would mean more math to synchronize it with the portal position, but less UI clutter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add angled/oblique near and far camera planes