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

Implement occlusion culling. #48050

Merged
merged 2 commits into from
Apr 27, 2021
Merged

Conversation

JFonS
Copy link
Contributor

@JFonS JFonS commented Apr 20, 2021

Added an occlusion culling system with support for static occluder meshes. It can be enabled via Project Settings > Rendering > Occlusion Culling > Use Occlusion Culling.

Occluders are defined via the new Occluder3D resource and instanced using the new OccluderInstance3D node. The occluders can also be automatically baked from a scene using the built-in editor plugin.

Missing work before 4.0:

Possible improvements (may be done before 4.0, PRs welcome):

Future work (after 4.0):

  • Support for simple dynamic occluders (quads, boxes, spheres).

Occlusion culling working in the editor:
occ

Occlusion buffer debug view:
occ2

@JFonS JFonS added this to the 4.0 milestone Apr 20, 2021
@JFonS JFonS requested a review from a team April 20, 2021 17:19
@JFonS JFonS requested review from a team as code owners April 20, 2021 17:19
@Calinou
Copy link
Member

Calinou commented Apr 20, 2021

Support for simple dynamic occluders (quads, boxes, spheres).

Without this feature, is it still possible to procedurally place occluders in a statically generated procedural scene? That means, without relying on editor baking.

@JFonS
Copy link
Contributor Author

JFonS commented Apr 20, 2021

Support for simple dynamic occluders (quads, boxes, spheres).

Without this feature, is it still possible to procedurally place occluders in a statically generated procedural scene? That means, without relying on editor baking.

Yes, occluders can be added/removed/updated at any time. I consider them "static only" because any one of these changes will trigger a rebuild of embree's BVH.

The rebuild happens in the background and may take some frames to complete, so it's not suitable for dynamic objects on screen, but changing off-screen occluders (i.e. loading/unloading world chunks, or generating the world at the start of a level) should be fine.

@Zylann
Copy link
Contributor

Zylann commented Apr 20, 2021

Very minor, but any reason to use filtered texture to display the debug buffer? Maybe using nearest neighbor would more precisely represent its state? Also, it could be transparent so we can see the scene behind

@@ -0,0 +1,49 @@
/*************************************************************************/
Copy link
Member

Choose a reason for hiding this comment

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

Maybe the module should just be called embree since it's embree?

ClassDB::bind_method(D_METHOD("set_indices", "indices"), &Occluder3D::set_indices);
ClassDB::bind_method(D_METHOD("get_indices"), &Occluder3D::get_indices);

ADD_PROPERTY(PropertyInfo(Variant::PACKED_VECTOR3_ARRAY, "vertices"), "set_vertices", "get_vertices");
Copy link
Member

Choose a reason for hiding this comment

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

I would probably make this non editor editable or you can make it explode :P

return use_occlusion_culling;
}

void Viewport::set_occlusion_rays_per_thread(int p_rays_per_thread) {
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 this should probably be a global setting for all viewports, since it does not make much sense to change. like RenderingServer::viewport_set_occlusion_rays_per_thread(amount) and it works for all viewports. Default value goes into a GLOBAL_DEF().

@@ -304,6 +305,8 @@ class Viewport : public Node {
ScreenSpaceAA screen_space_aa = SCREEN_SPACE_AA_DISABLED;
bool use_debanding = false;
float lod_threshold = 1.0;
bool use_occlusion_culling = false;
int occlusion_rays_per_thread = 512;
Copy link
Member

Choose a reason for hiding this comment

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

Again would probably make this global and just a good enough default, not much of a point per viewport.

@@ -44,6 +44,13 @@ struct SpatialIndexer;
class World3D : public Resource {
GDCLASS(World3D, Resource);

public:
enum OcclusionCullingBuildQuality {
OCCLUSION_CULLING_BUILD_QUALITY_LOW = 0,
Copy link
Member

Choose a reason for hiding this comment

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

I also would make this a global setting that you can only access via RenderingServer. Not much of a point to make per viewport, and since you will most likely never change (and keep quality to high).

for (uint64_t i = p_from; i < p_to; i++) {
bool mesh_visible = false;

if (cull_data.scenario->instance_aabbs[i].in_frustum(cull_data.cull->frustum)) {
if (cull_data.scenario->instance_aabbs[i].in_frustum(cull_data.cull->frustum) && !_occlusion_cull(cull_data, i, cull_data.cam_transform.origin, inv_cam_transform, z_near)) {
Copy link
Member

Choose a reason for hiding this comment

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

Give this is called so much, I would do it inside the if() and only if a bool is enabled or something (that is set globally or in the cull data), else it will most likely have a performance hit.

#endif
}

void RendererSceneCull::render_camera(RID p_render_buffers, Ref<XRInterface> &p_interface, XRInterface::Eyes p_eye, RID p_camera, RID p_scenario, Size2 p_viewport_size, float p_screen_lod_threshold, RID p_shadow_atlas) {
void RendererSceneCull::render_camera(RID p_render_buffers, Ref<XRInterface> &p_interface, XRInterface::Eyes p_eye, RID p_camera, RID p_scenario, RID p_viewport, Size2 p_viewport_size, float p_screen_lod_threshold, RID p_shadow_atlas) {
Copy link
Member

Choose a reason for hiding this comment

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

Keep in mind you not always have a viewport (as an example, when rendering reflection probes) so dont expect it to be present always. In those cases, occlusion culling should not be used.

virtual void occluder_initialize(RID p_occluder);
virtual void occluder_set_mesh(RID p_occluder, const PackedVector3Array &p_vertices, const PackedInt32Array &p_indices);

class RendererSceneOcclusionCull {
Copy link
Member

Choose a reason for hiding this comment

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

I would just make all of this inline, not virtual. I dont think we will have more than one implementation. If we do then we see how to do it, but for now I think its not worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now those are virtual because there is the "dummy" default implementation and the actual working implementation is loaded from the module. I can change it to just have a pointer and check whether it's null or not before all the calls, other than that I don't know how to make it an optional thing.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see better now how this works. IMO I would make the HZB code a bit more encapsulated, and would make the is_occluded() function FORCE_INLINE for performance. Also you can probably move these classes out to their own header/cpp to make it cleaner.

@@ -590,6 +594,8 @@ class RenderingServerDefault : public RenderingServer {
FUNC2(viewport_set_msaa, RID, ViewportMSAA)
FUNC2(viewport_set_screen_space_aa, RID, ViewportScreenSpaceAA)
FUNC2(viewport_set_use_debanding, RID, bool)
FUNC2(viewport_set_use_occlusion_culling, RID, bool)
FUNC2(viewport_set_occluison_rays_per_thread, RID, int)
Copy link
Member

Choose a reason for hiding this comment

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

I would make it a single value for all viewports, to make the API more straightforward.

Copy link

@wajrou wajrou Apr 21, 2021

Choose a reason for hiding this comment

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

typo occluison -> occlusion 9 times in the file

virtual void scenario_set_debug(RID p_scenario, ScenarioDebugMode p_debug_mode) = 0;
virtual void scenario_set_environment(RID p_scenario, RID p_environment) = 0;
virtual void scenario_set_fallback_environment(RID p_scenario, RID p_environment) = 0;
virtual void scenario_set_camera_effects(RID p_scenario, RID p_camera_effects) = 0;
virtual void scenario_set_occlusion_culling_build_quality(RID p_scenario, ScenarioOcclusionCullingBuildQuality p_quality) = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Again would make it single API for all viewports, no need to tweak per viewport and this value will most likely never change from high.

@JFonS
Copy link
Contributor Author

JFonS commented Apr 20, 2021

Very minor, but any reason to use filtered texture to display the debug buffer? Maybe using nearest neighbor would more precisely represent its state? Also, it could be transparent so we can see the scene behind

No specific reason, it was just the default values of the "copy to screen" code. I will take a look if there are better options.

@Calinou
Copy link
Member

Calinou commented Apr 20, 2021

Very minor, but any reason to use filtered texture to display the debug buffer? Maybe using nearest neighbor would more precisely represent its state? Also, it could be transparent so we can see the scene behind

See #45081. Making it translucent is a good idea 🙂

return !visible;
}

RID RendererSceneCull::RendererSceneOcclusionCull::HZBuffer::get_debug_texture() {
Copy link
Member

Choose a reason for hiding this comment

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

I would probably avoid creating this every frame, if this is what you are doing, you can keep it cached.

@JFonS JFonS requested a review from a team as a code owner April 23, 2021 13:56
Added an occlusion culling system with support for static occluder meshes.
It can be enabled via `Project Settings > Rendering > Occlusion Culling > Use Occlusion Culling`.

Occluders are defined via the new `Occluder3D` resource and instanced using the new
`OccluderInstance3D` node. The occluders can also be automatically baked from a
scene using the built-in editor plugin.
- All cpp files listed in `modules/raycast/godot_update_embree.py`
- All header files in the directories listed in `modules/raycast/godot_update_embree.py`

The `modules/raycast/godot_update_embree.py`script can be used to pull the
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
The `modules/raycast/godot_update_embree.py`script can be used to pull the
The `modules/raycast/godot_update_embree.py` script can be used to pull the

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Damn, I almost had a perfect "thirdparty bureaucracy" run this time :_(

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

I'll cleanup the SCsub to sync with changes done in 3.x and c7b53c0, but it's fine as is for now.

@akien-mga akien-mga merged commit 95cfce6 into godotengine:master Apr 27, 2021
@akien-mga
Copy link
Member

Thanks!

This was referenced May 1, 2021
@Calinou
Copy link
Member

Calinou commented Jun 30, 2021

Very minor, but any reason to use filtered texture to display the debug buffer? Maybe using nearest neighbor would more precisely represent its state? Also, it could be transparent so we can see the scene behind

For the record, changing

sampler.mag_filter = RD::SAMPLER_FILTER_LINEAR;
sampler.min_filter = RD::SAMPLER_FILTER_LINEAR;
to RD::SAMPLER_FILTER_NEAREST makes the occlusion culling buffer use nearest-neighbor filtering instead of linear filtering:

image

@clayjohn
Copy link
Member

@Calinou But be careful, that changes the default sampler to use nearest neighbor so many post processing effects will break.

@Calinou
Copy link
Member

Calinou commented Jun 30, 2021

@Calinou But be careful, that changes the default sampler to use nearest neighbor so many post processing effects will break.

Indeed, we should have a way to override this for specific effects somehow.

@clayjohn
Copy link
Member

@Calinou you can make another sampler for the specific effect. I did that for SSAO.

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.

7 participants