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

Change Camera properties in Rendering Server to use new Projection class #4932

Open
BastiaanOlij opened this issue Jul 21, 2022 · 3 comments

Comments

@BastiaanOlij
Copy link

BastiaanOlij commented Jul 21, 2022

Describe the project you are working on

Rewrite/cleanup 3D rendering implementations

Describe the problem or limitation you are having in your project

Godots Camera node records properties such as fov, near, far, etc. to setup the camera for a 3D project. This is great for 99% of the users as this presents a nice, simple and easy to understand interface for setting up a camera. For anyone wanting more this however closes the door. Many proposals and PRs have been raised to allow more complex camera configurations but they have all fallen short because all are improvements for the 1% of users that need it, and present a step back for the 99% that do not.

From a technical point of view, one of the main issues here is that any configuration of the Camera results in those settings being communicated to the Rendering Server as is. It is now the rendering server that builds a projection matrix from this data. When an XR Camera is used we even work around this by overriding this projection matrix which is pretty ugly considering.

This proposal attempts to introduce a change that keeps the user facing Camera API as is, but changes the way things work in the background to open doors for the hand full of developers that need more control.

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

Reduz's PR 63219 introduces a key change that allow us to streamline our camera implementation. Our CameraMatrix class is now renamed to Projection and more importantly, it can be encapsulated in a Variant which means that we can send it as a parameter in functions exposed in our ClassDB.

The proposed change is fairly simple. Instead of the Rendering Server taking the camera properties as is, we now store a Projection object. The code for building this Projection object moves into the Camera node. It still takes the same inputs and stores these in the scene, but it then builds the Projection matrix and sends it to the Rendering Server.

Our XRCamera3D node can now change as well and become responsible for obtaining the projection matrices from the XRInterface and communicating these to the rendering server.

To support both traditional and stereo rendering output the new API for the rendering server needs to be able to take an array of projection matrices. The Camera3D node simply sends 1, the XRCamera3D node can sent 2 if applicable. This greatly simplifies the code inside of the rendering server as well.

Users who wish to create more complex Projection matrices (for mirror effects, or other purposes) can just override the default camera logic, or implement their own camera node and create the needed projection matrix with code.

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

There are a number of changes required for this:

  1. In our rendering server camera_set_perspective, camera_set_orthogonal, camera_set_frustum and camera_set_use_vertical_aspect will be replaced by a camera_set_projection method that takes an array of projection matrices
  2. camera_set_transform will be changed to take an array of transforms
  3. Code internal in the rendering server will be changed so the viewport count is now taken from the camera data, the XRInterface overrule code will be removed (we will still keep the XR flag so ensure the viewport outputs to the XRInterface). This change will thus also allow us to render to stereo viewports that do not output to an XR interface (again important for mirrors).
  4. Our Camera3D node will be changed to calculate the projection matrix and communicate it where needed
  5. Our XRCamera3D node will be enhanced to obtain the projection matrices from the XR interface and communicate them to the rendering server as well as calculate the correct transforms.

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

No, this is code to the rendering server

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

No, this is code to the rendering server

@Calinou
Copy link
Member

Calinou commented Jul 21, 2022

Related to #2713.

@BastiaanOlij
Copy link
Author

Just tagging @reduz , @clayjohn and @JFonS, would like to implement this if you guys agree.

@BastiaanOlij
Copy link
Author

BastiaanOlij commented Jul 25, 2022

Just a bit of feedback after talking to Reduz, the biggest issue inside of the Rendering engine that have pushed back a few other attempt is that there are a number of effects that assume standard projection matrices as an optimization as full unprojection can be expensive. There are potentially some other issues where effects will break if a non standard projection matrix is supplied to the rendering engine.

Some we can probably capture and flag as we've already added switches to various effects to switch between the optimized and full unprojection code depending on whether we're rendering multiview or not.

If there are other issues with non-standard projection matrices we'll need to evaluate if it makes sense to address these, or if the workload is bigger then the benefits this has.

I do note that this can also be a warning. With this PR the standard Camera3D will behave as before, the only situation things could go wrong is if a user supplies a custom projection matrix through code (or their own Camera implementation).

edit also note that culling is a potential problem here as culling logic has certain expectations of the view frustum against which we cull. In XR we solve this by combining the frustums of the left and right eye and this combined frustum works properly for our culling logic.

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

No branches or pull requests

2 participants