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] Add OccluderShapePolygon #57361

Merged
merged 1 commit into from
Feb 1, 2022
Merged

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Jan 28, 2022

Add OccluderShapePolygon, glue to Occluder and gizmos etc.

Working in a similar way to the OccluderShapeSphere, the polygon occluder allows specifying convex polygon occluders. They are considerably more versatile than spheres, as they fit to many more level designs.

These can be edited in a similar fashion to Portals, by dragging handles for points, and specifying the orientation using the Node transform.

Additionally each polygon can have a single "hole". I've used the name hole to distinguish from portal, which has a very specific meaning in the Room system, and it would be confusing having two things with the same name, although they perform an analogous function.

Screenshot from 2022-01-26 11-28-37

Notes

  • Both polygons and holes must both be convex.
  • You can cunningly combine a polygon and hole to create a concave occluder.
  • Under the hood it is possible to have more than one hole, but I'm not quite sure how easy this will be editor wise, and efficiency wise (versus having 2 OccluderShapePolys). I'm currently undecided whether to allow e.g. 2 holes per poly, which we may get away with, depending on feedback during a beta.
  • Under the hood the Occluder polygons share the same backend as is used for Occluder Meshes, which are generalized sets of polygons baked automatically from user geometry. The meshes are considerably more complex, so I have separated the simpler polys into this PR in the interest of getting it merged quicker and allowing more time to finalize the meshes.

The occluder polygons require considerably less manual setup than rooms & portals, they can be added to a game level in a matter of minutes (although you may want to spend longer tuning their position etc to get the best results).

The flipside, as with occluder spheres, is that they only operate to cull objects from rendering. They do not yet affect VisibilityEnablers (although that may be added in a later PR), and they cannot be used to throttle performance in the way that rooms can.

As with Occluder spheres, they can be used dynamically, although changing the transform of the poly is considerably cheaper than changing the points of the poly itself. This should match most use cases though.

2022-01-28.17-04-16.mp4

Main desert map from "Wrought Flesh" - by Miziziziz

2022-01-30.14-55-51.mp4

Under the hood

The poly occlusion engine is fairly intricate. It first has to transform polys into screen space in order to provide a screen space metric for goodness of fit of each poly according to screen area. Larger and closer polys, that are facing the viewer are ranked higher. The best 8 (by default) polys are chosen as occluders, in order to make the best use of processing. The max active polys can be changed via a project setting as with spheres.

Once the polys are identified, they are clipped against each other. Polys (and holes) that are obscured by closer polys can be removed at this stage. Culling planes are precalculated for each polygon and hole.

Finally at runtime AABB of objects can be tested against these culling planes rapidly.

@Calinou
Copy link
Member

Calinou commented Jan 28, 2022

Does this PR supersede #52347?

@lawnjelly
Copy link
Member Author

Does this PR supersede #52347?

Yes I'll close that as the new PR for meshes will be quite a bit different.

@lawnjelly lawnjelly force-pushed the occ_poly_only branch 3 times, most recently from a87356f to 9115f15 Compare January 30, 2022 14:27
@lawnjelly lawnjelly changed the title [WIP] [3.x] Add OccluderShapePoly [3.x] Add OccluderShapePoly Jan 30, 2022
@lawnjelly lawnjelly marked this pull request as ready for review January 30, 2022 14:27
@lawnjelly lawnjelly requested review from a team as code owners January 30, 2022 14:27
@@ -426,6 +427,7 @@ void _err_print_index_error(const char *p_function, const char *p_file, int p_li
#define CRASH_NOW() \
if (true) { \
_err_print_error(FUNCTION_STR, __FILE__, __LINE__, "FATAL: Method failed."); \
_err_flush_stdout(); \
Copy link
Member

Choose a reason for hiding this comment

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

That makes sense, though it could have been better suited in a dedicated PR (and it should be fixed in master too).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes this crept in because it was downsized from a larger PR, and doing the flush here was very obviously needed during development as otherwise it crashes before we see the error messages.

Can move this to a separate PR if desired, and will do one for 4.x as it will be good to fix there too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Have removed this now to go in separate PR.

@akien-mga
Copy link
Member

We avoid shortening identifiers in the API, and this likely shouldn't be an exception either, so it should be OccluderShapePolygon. (In practice it should maybe even be PolygonOccluderShape as we usually put the derived type last, but since there's an OccluderShapeSphere precedent, I guess that shed has been painted :))

@lawnjelly
Copy link
Member Author

Will change, should just be a search and replace. 👍

@lawnjelly lawnjelly force-pushed the occ_poly_only branch 4 times, most recently from bdb1dc6 to fd9bf73 Compare February 1, 2022 11:12
@akien-mga akien-mga changed the title [3.x] Add OccluderShapePoly [3.x] Add OccluderShapePolygon Feb 1, 2022
Add OccluderShapePolygon, glue to Occluder and gizmos etc.
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.

Looks good 👍

@akien-mga akien-mga merged commit b6dbff7 into godotengine:3.x Feb 1, 2022
@akien-mga
Copy link
Member

Thanks!

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.

3 participants