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

Permit overriding the projection matrix passed to shaders without affecting culling logic. #85529

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

Conversation

lyuma
Copy link
Contributor

@lyuma lyuma commented Nov 30, 2023

Implements the functionality godotengine/godot-proposals#4932 but takes a slightly different approach.

This provides the option to provide a Projection matrix which will be used only for graphics operations, while keeping existing frustum/perspective/orthogonal modes for CPU-side camera logic such as culling.

When this PR is used with the frustum camera projection mode, we were able to build a functional screen-space mirror which efficiently culls geometry (on the CPU) outside of the visible view frustum of the mirror. You can see our addon here https://github.com/V-Sekai/avatar_vr_demo/tree/master/addons/V-Sekai.xr-mirror and clone this repository to try the demo for yourself. Currently, the demo requires XR due to the XR apis for accessing the camera projection being easier to use than traditional viewport APIs (we weren't able to figure out how to access the non-XR viewport reliably from script, but it does work if configured manually)

Status quo (without this PR) strictly using set_frustum projection mode without override_projection. Notice the aliasing artifacts that are caused by rendering from the frustum perspective and then transforming the mirror to screen coordinates. Notice the aliasing artifacts on the back wall due to sampling. These are more noticeable as the player walks around.
mirror_implementation_frustum_mode2

With this PR, using a screenspace oblique frustum traditionally used for planar reflections in override_projection, while using the aforementioned set_frustum projection on the Camera node itself. Everything in the reflection looks pixel-perfect.
mirror_implementation_screenspace_mode2

When I find time, I can attempt to summarize the differences in implementation into a counter-proposal, as well as why I think this is a preferable implementation.

The main difference between this and that proposal 4932 is that this implementation provides the ability to have fine-grained control over geometry culling. For example, as you can see in the attached screenshot, if I extend the mirror mesh (shown with noise dithering outside of the pink mirror border), all geometry instances outside of the frustum are not rendered, despite the matrix set as the override_projection having a full-screen frustum (as this is a screenspace mirror)
godot_mirror_dither_illustration
I walked slightly to the right in this screenshot such that the lamp and right hand were no longer visible in the reflection. Now, you can see here only the left hand is visible in the reflection, and the right hand, lamp, doors, windows and shelves are being frustum-culled. This is exactly what we want.

@lyuma lyuma requested review from a team as code owners November 30, 2023 06:34
@lyuma lyuma force-pushed the override_projection_4.2 branch 2 times, most recently from a58000f to 9490ea3 Compare November 30, 2023 06:54
@lyuma lyuma requested a review from a team as a code owner November 30, 2023 06:54
@AThousandShips
Copy link
Member

@@ -199,6 +199,12 @@
<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="override_projection" type="Projection" setter="set_override_projection" getter="get_override_projection" default="Projection(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)">
If set to a non-zero Projection, overrides the view-projection matrix passed to shaders with this custom matrix. This *only* overrides the projection passed to shaders on the GPU. The [constant PROJECTION_PERSPECTIVE], [constant PROJECTION_ORTHOGONAL] or [constant PROJECTION_FRUSTUM] values are still used for CPU-side operations such as culling, which can be used to provide a separate culling frustum.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If set to a non-zero Projection, overrides the view-projection matrix passed to shaders with this custom matrix. This *only* overrides the projection passed to shaders on the GPU. The [constant PROJECTION_PERSPECTIVE], [constant PROJECTION_ORTHOGONAL] or [constant PROJECTION_FRUSTUM] values are still used for CPU-side operations such as culling, which can be used to provide a separate culling frustum.
If set to a non-zero Projection, overrides the view-projection matrix passed to shaders with this custom matrix. This *only* overrides the projection passed to shaders on the GPU. The [constant PROJECTION_PERSPECTIVE], [constant PROJECTION_ORTHOGONAL], or [constant PROJECTION_FRUSTUM] values are still used for CPU-side operations such as culling, which can be used to provide a separate culling frustum.

Please use Oxford comma.

<param index="0" name="camera" type="RID" />
<param index="1" name="matrix" type="Projection" />
<description>
If set to a non-zero Projection, overrides the view-projection matrix passed to shaders with this custom matrix. This *only* overrides the projection passed to shaders on the GPU. The [method camera_set_frustum], [method camera_set_orthogonal] or [method camera_set_frustum] values are still used for CPU-side operations such as culling, which can be used to provide a separate culling frustum.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If set to a non-zero Projection, overrides the view-projection matrix passed to shaders with this custom matrix. This *only* overrides the projection passed to shaders on the GPU. The [method camera_set_frustum], [method camera_set_orthogonal] or [method camera_set_frustum] values are still used for CPU-side operations such as culling, which can be used to provide a separate culling frustum.
If set to a non-zero Projection, overrides the view-projection matrix passed to shaders with this custom matrix. This *only* overrides the projection passed to shaders on the GPU. The [method camera_set_frustum], [method camera_set_orthogonal], or [method camera_set_frustum] values are still used for CPU-side operations such as culling, which can be used to provide a separate culling frustum.

@@ -89,6 +89,15 @@ void RendererSceneCull::camera_set_frustum(RID p_camera, float p_size, Vector2 p
camera->zfar = p_z_far;
}

void RendererSceneCull::camera_set_override_projection(RID p_camera, const Projection &p_matrix) {
Camera *camera = camera_owner.get_or_null(p_camera);
ERR_FAIL_COND(!camera);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ERR_FAIL_COND(!camera);
ERR_FAIL_NULL(camera);

@fire
Copy link
Member

fire commented Nov 30, 2023

@aaronfranke You make be interested in mirrors in The Mirror.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

fire pinged me to review this, but I am not qualified to review this. Looks cool though.

<member name="override_projection" type="Projection" setter="set_override_projection" getter="get_override_projection" default="Projection(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0)">
If set to a non-zero Projection, overrides the view-projection matrix passed to shaders with this custom matrix. This *only* overrides the projection passed to shaders on the GPU. The [constant PROJECTION_PERSPECTIVE], [constant PROJECTION_ORTHOGONAL] or [constant PROJECTION_FRUSTUM] values are still used for CPU-side operations such as culling, which can be used to provide a separate culling frustum.
To clear the overridden projection, set to [code]Projection(Vector4.ZERO, Vector4.ZERO, Vector4.ZERO, Vector4.ZERO)[/code].

Copy link
Member

Choose a reason for hiding this comment

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

Line breaks in the XML docs are paragraphs, and there should never be any blank lines.

Suggested change

@majikayogames
Copy link
Contributor

Appreciate this PR, I was able to adapt your set_oblique_near_plane function with the camera.set_override_projection function to make properly occluded portals for my game:

https://github.com/majikayogames/portal_demo

However I'm wondering if we need the projection override matrix displayed in the inspector? It may confuse new users.
image

I think this PR more likely to make it in 4.3 if we could have some function only on the RenderingServer to override a camera's projection matrix, hidden from view unless you need it.

@Calinou
Copy link
Member

Calinou commented Jan 29, 2024

Tested locally (rebased on top of master fa48a51), it works as expected.

However, there's no easy way to switch to a custom projection while keeping the existing perspective/orthogonal/frustum mode as a base. This makes configuring it in the editor significantly harder compared to #84454.

Also, the projection override takes up a lot of space in the inspector when you don't need to override it. Therefore, I prefer the inspector design found in #84454.

It has the same rendering issues as #84454 which shows that the issue is with the rendering code, as opposed to the custom projection code. You can use the project I linked in #84454 (comment) for testing purposes.

@lyuma
Copy link
Contributor Author

lyuma commented Jan 29, 2024

The reason I preferred this approach is because I wanted to provide a different culling frustum compared to the visual frustum.

The main difference between this and that proposal 4932 is that this implementation provides the ability to have fine-grained control over geometry culling. For example, as you can see in the attached screenshot, if I extend the mirror mesh (shown with noise dithering outside of the pink mirror border), all geometry instances outside of the frustum are not rendered, despite the matrix set as the override_projection having a full-screen frustum (as this is a screenspace mirror)
godot_mirror_dither_illustration

While you could argue that the above is only an optimization for a specific usecase, it did make it easier for me to verify the correctness of my planar reflection.

I see your point that if culling uses the correct frustum calculated from the projection matrix, there is no advantage to restricting the majority of the graphics code to a specific matrix. If I understand the advantages of #84454, it is that users may want to set a custom projection matrix and not explicitly calculate the frustum bounds that match the projection.

Here is my worry: Unity also has an API to allow an arbitrary projection matrix similar to #84454, though Unity's culling code is not robust and is known to break in some cases when the projection matrix is explicitly set. In fact, in the built-in pipeline at least, using a custom projection is explicitly incompatible with deferred rendering.
And it's not just Unity. Even Unreal 5 only has perspective and orthographic modes, and if you want to override the projection matrix, it looks a lot like this PR: it doesn't affect culling and is only visual. The difference is Unreal does not have a "Frustum" mode which means the only way to implement planar portals is to set a near clip plane explicitly.

So my question is: is opening up the API to allow arbitrary culling projection matrices going to work correctly in all cases? Is there no special case needed in Godot's renderer for ortho / perspective / frustum in occlusion culling, for example?

@majikayogames
Copy link
Contributor

majikayogames commented Dec 9, 2024

If this is still being considered it will need an update to take into account the Z buffer being reversed since 4.3. Breaks compatibility renderer completely right now and everything renders depth sorted backwards without these lines updated:
majikayogames@d7d6e38

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.

6 participants