Frustum culling #1642

nikhilghosh75 · 2024-04-26T03:42:54Z

Implemented Frustum Culling into the renderer. Currently only applies to the world renderer. The camera has a frustum data structure to allow it to cull objects outside the frustum.

An additional stresstest has been created for the renderer to test out frustum culling.

Additional fixes to loading files on windows that have the special \r character are also included in this pull request.

Fixed merge conflict

Fixed styling

Fixed various checks

heinezen · 2024-04-27T01:46:00Z

Nice, I'll take a look over the weekend :)

heinezen · 2024-04-29T18:48:27Z

libopenage/renderer/camera/camera.cpp

 this->look_at_scene(Eigen::Vector3f(0.0f, 0.0f, 0.0f));

 resources::UBOInput view_input{"view", resources::ubo_input_t::M4F32};
 resources::UBOInput proj_input{"proj", resources::ubo_input_t::M4F32};
 auto ubo_info = resources::UniformBufferInfo{resources::ubo_layout_t::STD140, {view_input, proj_input}};
 this->uniform_buffer = renderer->add_uniform_buffer(ubo_info);

+ // Make the frustum slightly bigger than the camera's view to ensure objects on the boundary get rendered
+ float real_zoom = 0.7f * this->default_zoom_ratio * this->zoom;


The factor used here (0.7f) should be configurable.

Also, you don't need to calculate the frustum here, since line 30 moves the camera (and calls move_to)

heinezen · 2024-04-29T18:50:54Z

libopenage/renderer/camera/camera.h

+static const float near_distance = 0.01f;
+static const float far_distance = 100.0f;
+


These should be configured for the frustum class and not in the camera file.

heinezen · 2024-04-29T18:53:46Z

libopenage/renderer/camera/camera.cpp

+ float real_zoom = 0.7f * this->default_zoom_ratio * this->zoom;
+ frustum.Recalculate(this->viewport_size, near_distance, far_distance, this->scene_pos, cam_direction, Eigen::Vector3f(0.0f, 1.0f, 0.0f), real_zoom);


same here for the factor.

heinezen · 2024-04-29T18:54:03Z

libopenage/renderer/camera/camera.cpp

+ float real_zoom = 0.7f * this->default_zoom_ratio * this->zoom;
+ frustum.Recalculate(viewport_size, near_distance, far_distance, scene_pos, cam_direction, Eigen::Vector3f(0.0f, 1.0f, 0.0f), real_zoom);


same here for the factor.

heinezen · 2024-04-29T19:09:58Z

libopenage/renderer/camera/camera.h

+ /**
+ * Is frustum culling enabled? If true, perform frustum culling.
+ * If false, all frustum checks will return true
+ */
+ bool frustum_culling;
+
+ /**
+ * The frustum used to cull objects
+ * Will be recalculated regardless of whether frustum culling is enabled
+ */
+ Frustum frustum;


I thought about this for a while and I think it's better if the frustum is not a camera member for now. Instead, a frustum should be created and returned by the camera using a get_frustum(..) function. The reasoning behind this is that frustum culling will likely be more configurable if the Frustum is directly exposed. Accessing it through the camera class can make it harder to activate/deactivate when debugging. This means more frustums will be created at first (once per frame), but I think this is the better short-term option right now.

To create the frustum, you can add a function like this:

/** * Get a frustum for this camera * * @param scalefactor Resize the frustum to ensure objects on the boundary get rendered */ Frustum get_frustum(float scalefactor)

scalefactor would be the variabe that should be configurable that I mentioned earlier (the 0.7f one).

heinezen · 2024-04-29T19:28:55Z

libopenage/renderer/resources/parser/parse_sprite.cpp

- // Skip empty lines and comments
- if (line.empty() || line.substr(0, 1) == "#") {
+ // Skip empty lines, lines with carriage returns, and comments
+ if (line.empty() || line.substr(0, 1) == "#" || line[0] == '\r') {


same here, these should be removed by the string splitter

heinezen · 2024-04-29T19:29:05Z

libopenage/renderer/resources/parser/parse_terrain.cpp

- // Skip empty lines and comments
- if (line.empty() || line.substr(0, 1) == "#") {
+ // Skip empty lines, lines with carriage returns, and comments
+ if (line.empty() || line.substr(0, 1) == "#" || line[0] == '\r') {


heinezen · 2024-04-29T19:29:13Z

libopenage/renderer/resources/parser/parse_texture.cpp

@@ -56,6 +56,11 @@ std::string parse_imagefile(const std::vector<std::string> &args) {
 // it should result in an error if wrongly used here.

 // Call substr() to get rid of the quotes
+ // If the line ends in a carriage return, remove it as well
+ if (args[1][args[1].size() - 1] == '\r') {


heinezen · 2024-04-29T19:29:20Z

libopenage/renderer/resources/parser/parse_texture.cpp

- // Skip empty lines and comments
- if (line.empty() || line.substr(0, 1) == "#") {
+ // Skip empty lines, lines with carriage returns, and comments
+ if (line.empty() || line.substr(0, 1) == "#" || line[0] == '\r') {


heinezen · 2024-04-29T19:31:32Z

libopenage/renderer/stages/world/render_stage.cpp

@@ -58,6 +58,10 @@ void WorldRenderStage::update() {
 std::unique_lock lock{this->mutex};
 auto current_time = this->clock->get_real_time();
 for (auto &obj : this->render_objects) {
+ if (!obj->within_camera_frustum(this->camera)) {
+ continue;


This will still render the object btw, it just skips the update :D

I'll have to fix this in a separate PR as it is not possile right now to remove specific objects from the render pass.

heinezen · 2024-04-29T19:34:08Z

I only found a few minor issues with this and a have restructering reques. It already looks quite nice! You also have to rebase and resolve the merge conflicts.

heinezen · 2024-04-29T19:39:03Z

copying.md

@@ -153,6 +153,7 @@ _the openage authors_ are:
 | Astitva Kamble | askastitva | astitvakamble5 à gmail dawt com |
 | Haoyang Bi | AyiStar | ayistar à outlook dawt com |
 | Michael Seibt | RoboSchmied | github à roboschmie dawt de |
+| Nikhil Ghosh | NikhilGhosh75 | nghosh606 à gmail dawt com


I think the CI fails because you are missing an | at the end of the table. Then the mailmap entry is also not necessary.

TheJJ

can't we calculate a containing circle for each object on creation and when resizing it? then we only have to check one coordinate/distance for each object to cull it.

heinezen · 2024-05-02T18:16:10Z

can't we calculate a containing circle for each object on creation and when resizing it? then we only have to check one coordinate/distance for each object to cull it.

I think that would be a bit more complicated since we would have to calculate it for every animation sprite and not just per object. And then we also have to apply zoom scaling...

nikhilghosh75 and others added 11 commits April 25, 2024 23:14

Fixed bug with opening files with \r on windows

2738b1c

Implemented frustum culling

5e4a50c

Implemented stresstest 1

7b7383a

Added additional comments to the frustum culling logic

96a7fa6

Merge pull request #1 from nikhilghosh75/master

68a91a9

Fixed merge conflict

Fixed styling

5e9dd26

Merge pull request #2 from nikhilghosh75/master

de32bb9

Fixed styling

Fixed various checks

11be4a8

Merge pull request #3 from nikhilghosh75/master

6dd2172

Fixed various checks

Added name to mailmap

ef03a27

Added name to copying.md

64a2e82

heinezen requested changes Apr 29, 2024

View reviewed changes

heinezen reviewed Apr 29, 2024

View reviewed changes

heinezen added area: renderer Concerns our graphics renderer nice new thing ☺ A new feature that was not there before lang: c++ Done in C++ code labels Apr 29, 2024

TheJJ reviewed May 2, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Frustum culling #1642

Frustum culling #1642

nikhilghosh75 commented Apr 26, 2024

heinezen commented Apr 27, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen Apr 29, 2024

heinezen commented Apr 29, 2024 •

edited

heinezen Apr 29, 2024

TheJJ left a comment

heinezen commented May 2, 2024

		static const float near_distance = 0.01f;
		static const float far_distance = 100.0f;

		float real_zoom = 0.7f * this->default_zoom_ratio * this->zoom;
		frustum.Recalculate(this->viewport_size, near_distance, far_distance, this->scene_pos, cam_direction, Eigen::Vector3f(0.0f, 1.0f, 0.0f), real_zoom);

		float real_zoom = 0.7f * this->default_zoom_ratio * this->zoom;
		frustum.Recalculate(viewport_size, near_distance, far_distance, scene_pos, cam_direction, Eigen::Vector3f(0.0f, 1.0f, 0.0f), real_zoom);

Frustum culling #1642

Are you sure you want to change the base?

Frustum culling #1642

Conversation

nikhilghosh75 commented Apr 26, 2024

heinezen commented Apr 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heinezen commented Apr 29, 2024 • edited

Choose a reason for hiding this comment

TheJJ left a comment

Choose a reason for hiding this comment

heinezen commented May 2, 2024

heinezen commented Apr 29, 2024 •

edited