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

Contact Points and Contact Manifold #1301

Conversation

fallenatlas
Copy link
Contributor

@fallenatlas fallenatlas commented Aug 21, 2024

Description

  • Add Contact Manifold
  • Compute contact points using the Sutherland Hodgman algorithm.
  • Updated collisions sample to show the resulting info on the contact manifold.

(yup, that's all)

To support this:

  • Added a Plane to geom
  • Added the concept of faces and edges to box shape
  • Added utils to geom, which contains functions that don't directly handle intersections.

I'll gladly take any suggestions so Box.hpp doesn't look so ugly.

Resources:
https://research.ncl.ac.uk/game/mastersdegree/gametechnologies/previousinformation/physics5collisionmanifolds/2017%20Tutorial%205%20-%20Collision%20Manifolds.pdf
https://research.ncl.ac.uk/game/mastersdegree/gametechnologies/physicstutorials/4collisiondetection/Physics%20-%20Collision%20Detection.pdf
Also used the structures to hold the date in Box2D/Avian for reference.

Checklist

  • Self-review changes.
  • Evaluate impact on the documentation.
  • Add entry to the changelog's unreleased section.

Copy link
Contributor

github-actions bot commented Aug 21, 2024

PR Preview Action v1.4.7
🚀 Deployed preview to https://GameDevTecnico.github.io/cubos/preview/pr-1301/
on branch gh-pages at 2024-09-09 19:29 UTC

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/4)

core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (2/4)

core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/utils.hpp Outdated Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (3/4)

core/src/geom/utils.cpp Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
core/src/geom/utils.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (4/4)

core/src/geom/utils.cpp Outdated Show resolved Hide resolved
core/src/geom/intersections.cpp Outdated Show resolved Hide resolved
core/src/geom/intersections.cpp Outdated Show resolved Hide resolved
core/src/geom/intersections.cpp Outdated Show resolved Hide resolved
engine/src/collisions/narrow_phase/plugin.cpp Outdated Show resolved Hide resolved
engine/src/collisions/narrow_phase/plugin.cpp Outdated Show resolved Hide resolved
engine/src/collisions/narrow_phase/plugin.cpp Outdated Show resolved Hide resolved
engine/src/collisions/narrow_phase/plugin.cpp Outdated Show resolved Hide resolved
engine/src/collisions/narrow_phase/plugin.cpp Outdated Show resolved Hide resolved
engine/src/collisions/narrow_phase/plugin.cpp Outdated Show resolved Hide resolved
@fallenatlas fallenatlas force-pushed the 1243-add-ability-to-calculate-contact-point-for-collision-and-store-it-for-each-entity branch from 51a786a to c7fbe41 Compare August 22, 2024 00:14
Copy link

codecov bot commented Aug 22, 2024

Codecov Report

Attention: Patch coverage is 7.58017% with 317 lines in your changes missing coverage. Please review.

Project coverage is 35.94%. Comparing base (7a05b26) to head (03b9126).
Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
core/include/cubos/core/geom/box.hpp 0.00% 157 Missing ⚠️
core/src/geom/utils.cpp 0.00% 71 Missing ⚠️
core/src/geom/intersections.cpp 0.00% 42 Missing ⚠️
engine/src/collisions/narrow_phase/plugin.cpp 17.64% 42 Missing ⚠️
core/src/geom/plane.cpp 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1301      +/-   ##
==========================================
- Coverage   36.25%   35.94%   -0.31%     
==========================================
  Files         395      398       +3     
  Lines       31537    31876     +339     
==========================================
+ Hits        11433    11458      +25     
- Misses      20104    20418     +314     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fallenatlas fallenatlas marked this pull request as ready for review August 22, 2024 01:15
Copy link
Member

@RiscadoA RiscadoA left a comment

Choose a reason for hiding this comment

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

Incredible work, ty very much for working on this! Also ran the sample on my PC, very cool to see the results 👀
Performance wise I'm sure we can improve this a lot further but it's not worth it to waste time on that right now. The only thing I think you should change on that part is replacing std::list by std::vector - unless you've a reason for sticking to the former ofc.

Also sorry for the review delay once again!

core/include/cubos/core/geom/box.hpp Outdated Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Show resolved Hide resolved
core/include/cubos/core/geom/box.hpp Show resolved Hide resolved
core/include/cubos/core/geom/intersections.hpp Outdated Show resolved Hide resolved
@RiscadoA
Copy link
Member

RiscadoA commented Sep 4, 2024

Also it would be cool to link those resources in the code itself so that it doesn't get forgotten

Copy link
Contributor

@Dacops Dacops left a comment

Choose a reason for hiding this comment

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

post resolving the pending conversations; lgtm

@fallenatlas fallenatlas force-pushed the 1243-add-ability-to-calculate-contact-point-for-collision-and-store-it-for-each-entity branch from f59816b to d8c8ba3 Compare September 9, 2024 18:29
@fallenatlas fallenatlas force-pushed the 1243-add-ability-to-calculate-contact-point-for-collision-and-store-it-for-each-entity branch from f9876c4 to 2f6ccfe Compare September 9, 2024 19:58
@fallenatlas fallenatlas merged commit 4c2d6a4 into main Sep 10, 2024
11 checks passed
@fallenatlas fallenatlas deleted the 1243-add-ability-to-calculate-contact-point-for-collision-and-store-it-for-each-entity branch September 10, 2024 09:55
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.

Add ability to calculate contact point for collision and store it for each entity
4 participants