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

[3.x] Implementation of new Spatial Camera type: Oblique #56664

Closed
wants to merge 2 commits into from

Conversation

qaptoR
Copy link

@qaptoR qaptoR commented Jan 10, 2022

allows the specification of a world plane for near clipping;
applies matrix manipulation to generate oblique near plane.


A partial solution to this proposal: godotengine/godot-proposals#2713
**Note: It is by no means a complete solution and should not necessarily be taken as a reason to close the proposal. This PR is merely a straightforward solution for implementing the need for oblique clipping planes to implement a portal mechanic.

Not exactly what is requested, but this implementation covers essentially the most common case the exposed camera matrix would be used for according to the community. At least until more common uses of an exposed matrix are determined to push for a more broad solution.

Thus, this feature is necessary for implementing a 'portal' effect based on the most popular feature tutorials. For example, this often cited video by Sebastian Lague:
https://www.youtube.com/watch?v=cWpFZbjtSQg

Benefits of this solution...
Uses the function 'set_oblique_plane_from_transform' to convert the origin and forward vector into a camera-space Plane normal and distance-offset. Or alternatively, the position and normal can be provided independently of each other.

Exposes member variables in the editor for the plane normal, position, and offset data which supports editing of the plane in world space providing an easy way to visualize the clipping plane by making adjustments in the inspector.

Portal Effect using perspective camera...
Screenshot 2022-01-09 113017
Each portal has a camera which displays on the alternate portal's display. The perspective camera can't see through the obstacle its parent portal is placed on (its position is relative to the player's position and the other portal).

Portal Effect using Oblique camera...
Screenshot 2022-01-09 112903
The oblique camera uses the portal's global transform to adjust the camera's near clipping plane to be parallel with the portals facing direction. Thus, obstacles between the camera and portal are no longer visible.

Oblique Camera Member Variables...
Screenshot 2022-01-10 031700

Simple Video Demonstration

**Note: This PR is now covered for 3.x (here) and master branch from this PR: #64876

@qaptoR qaptoR requested review from a team as code owners January 10, 2022 11:20
@Calinou Calinou added this to the 3.5 milestone Jan 10, 2022
Comment on lines +156 to +168
/* clang-format off */
q.x = ((
p_oblique_plane.x < 0 ? -1 :
p_oblique_plane.x > 0 ? +1 : 0
) + matrix[2][0]
) / matrix[0][0];

q.y = ((
p_oblique_plane.y < 0 ? -1 :
p_oblique_plane.y > 0 ? +1 : 0
) + matrix[2][1]
) / matrix[1][1];
/* clang-format on */
Copy link
Member

Choose a reason for hiding this comment

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

I think you can use the SGN macro to get -1, +1, and 0. The clang-format comments can then be removed hopefully.

Copy link
Author

Choose a reason for hiding this comment

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

I will try this right away

Copy link
Author

Choose a reason for hiding this comment

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

I don't think SGN will work here... should I opt for intermediary variables?
#define SGN(m_v) (((m_v) < 0) ? (-1.0) : (+1.0))

Copy link
Member

@timothyqiu timothyqiu Jan 10, 2022

Choose a reason for hiding this comment

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

Ah, please don't change the macro. That'll break compatibility I think.

But the clang-format tweak still seems awkward to me, I'm not sure if SGN works for your logic.

Copy link
Author

Choose a reason for hiding this comment

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

sorry, I wasn't suggesting changing the macro, just including it to show it doesn't have the necessary 'zero if zero' branch.

This is the adapted algorithm used in my PR:
http://terathon.com/code/oblique.html

Specifically, he uses an inline function:
image

which you can see does greatly simplify the expression:
image

Copy link
Member

Choose a reason for hiding this comment

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

In master, it's:

#ifndef SIGN
#define SIGN(m_v) (((m_v) == 0) ? (0.0) : (((m_v) < 0) ? (-1.0) : (+1.0)))
#endif

We could backport SIGN with the behavior of 0.0 returning 0.0 and keep SGN the way it is.

Copy link
Author

@qaptoR qaptoR Jan 10, 2022

Choose a reason for hiding this comment

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

That seems like a reasonable option, especially given that according to Wikipedia, the signum (aka sign) function should return zero if zero

image

There's also this answer from stackoverflow, which might be worth looking into as an optimization: https://stackoverflow.com/a/4609795/11622959
image

Copy link
Author

@qaptoR qaptoR Jan 10, 2022

Choose a reason for hiding this comment

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

I think the real question is "is there a distinct difference"? any mathematicians kicking around?

Looking around I found this collection of articles. I mention it not just because of their crazy bit-twiddling, but because it shows there are legitimately multiple types of sign-checking, hence it makes sense why SGN does not currently return zero. It probably wasn't needed more generally.

http://graphics.stanford.edu/~seander/bithacks.html#:~:text=your%20target%20architecture.-,Compute%20the%20sign%20of%20an%20integer,-int%20v%3B%20%20%20%20%20%20//%20we

Copy link
Author

Choose a reason for hiding this comment

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

We could backport SIGN with the behavior of 0.0 returning 0.0 and keep SGN the way it is.

just to clarify: are you suggesting that I do this for my PR, as in add that Macro and utilize it to clean up the expression?
Or to do that as another PR, and clean up the expression with the assumption that the new macro will be available when necessary?

@qaptoR
Copy link
Author

qaptoR commented Jan 10, 2022

It appears the builds that are failing because of an issue with adding PROJECTION_OBLIQUE, which I didn't handle in 'editor/spatial_editor_gizmos.cpp'.

When I've fixed that (and the other things brought up in the review) and whatever else comes up from these tests... do I just amend the commit and push again?

@timothyqiu
Copy link
Member

Yes, amending & force push does the work.

@qaptoR qaptoR force-pushed the oblique_camera branch 3 times, most recently from a35671a to 402ac69 Compare January 10, 2022 16:30
@aaronfranke
Copy link
Member

Shouldn't the oblique near plane be a Plane type instead of a normal and position?

@qaptoR
Copy link
Author

qaptoR commented Jan 10, 2022

Shouldn't the oblique near plane be a Plane type instead of a normal and position?

In the visual_server this is true, and how it is implemented in the 'camera' representation there.
But in the editor it is more relevant to associate the plane as a normal and position as a traditional Plane has the value d along with a normal which is the distance of the plane from the origin, whereas for this camera its global_transform is the origin, and we need the global position of the plane to calculate the relevant camera-space d value.

@SaracenOne
Copy link
Member

SaracenOne commented Jan 10, 2022

I've wanted to add this to the camera for a while now to do this very effect, so I'm glad someone's implementing this. Exposing it as oblique clipping planes seems like it should be sufficent enough for the actual effect, but yeah, seeing a demo of a working implementation of portals/mirrors would help sell this.

The only other thing that I would suggest should be considered as further features might be a way to manipulate the other clipping planes too. The reason you may want control over the other clipping planes too is that if you're looking through a mirror/portal, while you're essentially placing a flipped fake camera mirroring you own camera behind it, and the camera frustum is much wider than the confine of the portal, you actually want to be able to cull everything outside of the confines of the rectangle you're rendering through. This shouldn't affect the ability to actually create the effect itself, but being able to clip unwanted geometry would likely make the effect more optimized. I'm not sure, but it might even be necessary to give a way to handle the view and clipping frustum seperately. You may also want a way to override its position for other forms of occlusion culling too (the room/portal system, ect.), since the portal camera will be behind the portal, but as far as occlusion tests go, you would want to consider it aligned with the portal plane for the most efficent render.

Just wanted to my thoughts on this since too. I'm glad you're implementing this. A bunch of us really want to make use of this feature too.

@qaptoR
Copy link
Author

qaptoR commented Jan 10, 2022

I can put together a demo video if that helps others.

@SaracenOne I believe what you're talking about is exactly what is wanted from this proposal: godotengine/godot-proposals#2713. There are a lot more capabilities provided by exposing the camera-matrix, however it's a bigger challenge than the scope of this PR. Currently the visual_server_scene class utilizes the camera_matrix and it would take some effort to restructure how that information is carried around.

I hadn't actually thought to limit the sides of the frustum as a way of culling unnecessary geometry from view (which might result in a performance gain).

@SaracenOne
Copy link
Member

SaracenOne commented Jan 10, 2022

Hmm, yeah, that sounds about right. So, I'll suggest to continue the PR as is, and maybe one thing I could do is look a bit more into the idea of exposing the full camera matrix, but target it strictly as a future 4.x feature rather than encouraging broadening the scope of this PR too much. Figuring out the math for oblique projection is valuable either way.

@qaptoR
Copy link
Author

qaptoR commented Jan 10, 2022

I just want to point out the people who did the heavy lifting when it comes to this PR.
I mention them here with links to their work: godotengine/godot-proposals#2713 (comment)

Igor Kosyanenko, who attempted this before me but got stuck when there was warping in the portal image. EDIT: The fix for that is the FOV of the skybox should be the same as the portal camera (which should be the same as the player camera); this is a short video showing the problem, and the solution: https://youtu.be/hchttF-iN7Y

Eric Lengyel, whose name you'll actually find in the Camera_Matrix::set_oblique() function, since it's his mathematical research which Igor founded his work on.

I'm merely carrying the torch by continuing the work and stitching it all together, because I needed it for my game project, and hopefully it will help others until such a time as there is a better solution.

@qaptoR
Copy link
Author

qaptoR commented Jan 11, 2022

I've put up a Simple Demonstration to showcase the effect an oblique near clipping plane has when creating a portal effect.

@TokageItLab
Copy link
Member

There is a problem #57404 with Automatic LOD in Orthogonal mode. It may need to consider a same problem.

@qaptoR
Copy link
Author

qaptoR commented Feb 1, 2022

There is a problem #57404 with Automatic LOD in Orthogonal mode. It may need to consider a same problem.

Is that problem in the Orthogonal mode related to the math for determining the matrix?

This solution copies all the code from the Projection camera and just calculates new clipping planes using the matrix, otherwise it's no different from projection in a significant way.

Is there a test-project for reproducing the error with the Orthogonal camera that can be utilized to test this Oblique camera?

@TokageItLab
Copy link
Member

In #57404, it seems that the distance calculation was broken and solved. Perhaps #54885 will be relevant in the future. Please see it.

@greenrazer
Copy link

I'm currently working on portals in my game and I'm using this PR for the near clipping plane. I'd really like to see this get merged at some point. Great work qaptoR!

@akien-mga akien-mga added this to the 3.4 milestone Jul 2, 2022
@akien-mga akien-mga marked this pull request as draft July 2, 2022 22:14
@akien-mga
Copy link
Member

akien-mga commented Jul 2, 2022

For the record, this is being discussed further in the relevant proposal: godotengine/godot-proposals#2713 (comment)

For now I marked this PR as a draft as it targets the 3.4 branch and can't be merged as is. First we'll need to reach a consensus on what to implement in the proposal, and then if the implementation in this PR is approved, it should be rebased against the 3.x and master branches.

(I assigned the 3.4 milestone to make it clear that this PR is targeting the 3.4 branch - but that does not mean that we intend to merge this for a 3.4.x release. That stable branch is only receiving bug fixes.)

@qaptoR
Copy link
Author

qaptoR commented Aug 6, 2022

Thank you for the clarification. This was my first PR and I was still learning so much about contributing (still am really). I've been side-tracked on other projects lately, having put the 3D game I created this PR for on hold to work on a few smaller projects before I tackle something as ambitious as it was.

However, I am happy to and will make an effort to prepare this to target 3.x and master branches, as I am keen to keep one foot rooted deep in engine/tool development. If anyone wants to collaborate as well it would be great to start growing community connections; I always tend to work better with someone to pass around ideas with.

allows the specification of a world plane for near clipping;
applies matrix manipulation to generate oblique near plane.
@qaptoR qaptoR changed the base branch from 3.4 to 3.x August 24, 2022 12:24
@qaptoR
Copy link
Author

qaptoR commented Aug 24, 2022

I have, to the best of my present knowledge, retargeted this PR for 3.x...

I cherry-picked the commit from 3.4 to 3.x, fixed merge conflicts, then force-pushed the local 3.x to this remote PR branch once I had tested that it compiled and worked as before in engine.

if there is anything else to be done, I'll focus on getting the PR to work for master branch now.

@qaptoR
Copy link
Author

qaptoR commented Aug 28, 2022

not sure what the next step is. I can see a button marked "ready for review".
However there is one function 'camera::set_oblique_plane_from_transform' which was intended to be a straightforward call to generate the oblique plane for a given mesh, using the forward normal as the automatically chosen normal.

image
fig 1: plane-mesh, showing surface-perpendicular normal is upward (green) vector.

However, in this image, while using the XR demo of another contributor to test the camera for mirrors I realized that based on how a plane-mesh instantiates, it is actually the 'upward normal', or 'y component' of the basis which should be used as the default.

Looking into why I chose the forward normal, I can see that it was because for godot 3.4, a quad-mesh was essentially a plane-mesh, but instantiated 'upright' with the forward normal perpendicular to the surface, and so was the default choice of mesh for a portal/mirror.

image
fig 2: Quad mesh in default instantiation pose

Since godot 4.0 has removed the quadmesh as an option (likely for redundancy), I propose the function in question be changed for both 3.x and master PR (but I bring it up here since it really must be done in master regardless) to use the basis.get_colum(1) instead of get_column(2).

@qaptoR qaptoR marked this pull request as ready for review August 28, 2022 21:30
@akien-mga akien-mga changed the title Implementation of new Spatial Camera type: Oblique [3.x] Implementation of new Spatial Camera type: Oblique Sep 2, 2022
@jamesblight
Copy link

@qaptoR Great work with this. However, I think calling this an oblique projection/camera type is misleading and will confuse people. This change appears to implement an oblique view frustum for a perspective camera. Apologies if I'm missing something.

@qaptoR
Copy link
Author

qaptoR commented Mar 31, 2023

I'm going to close this PR, work on a better implementation that can be applied more seamlessly to the camera (that will perhaps work with orthogonal cameras as well, based on the paper my current solution is sourced from). I already have an idea how to do this, but my local repo is too messy that I need to do housekeeping to get a clear mind about what I'm doing moving forward. This project was the first open source contribution I attempted to make and I'm very proud that I had a rough working solution at the time but it was still not a good enough solution IMO for final acceptance.

Moving forward, for my own sanity I will only be targeting the Master branch since at some point 4.0 and master will begin to diverge and I do not intend to keep track of multiple versions since that was one of the major downfalls of this current solution, I felt overwhelmed by having to jump back and forth between master and 3.x.

@qaptoR qaptoR closed this Mar 31, 2023
@qaptoR qaptoR deleted the oblique_camera branch March 31, 2023 23:24
@YuriSizov YuriSizov removed this from the 3.x milestone Dec 2, 2023
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.

10 participants