Potential bugs
Tilmann
Hi,
while going over the code I found a few thing that may (or may not) need fixing. I'll list them here but I could also provide PRs if you think this is more convenient, please let me know. (Personally I prefer "hints" over PRs for small fixes on my own projects).
I am referring to the master branch:
- demo_jointPU.cpp:350 : I believe there is a "break;" missing
- collision_libccd.cpp:768 : I believer this is currently returning the "index of the triangle index" instead of the triangle index itself. I think it should be "tempContact.side2 = indices[i];"
- collision_cylinder_trimesh.cpp:1112 : Not entriely sure, other GIMPACT colliders (e.g. dCollideBTL() for boxes or dCollideTriumeshPlane() for planes) recomute the AABBs before colliding. Only
the cylinder collider does not do it. Potentially missing:
o1 -> recomputeAABB();
o2 -> recomputeAABB();
- collision_libccd:800 : The code removes a contact and fills the empty position with the last contact in the list. I am not entirely sure but I thought contacts should be ordered by depth?
I think this is violated here. If this is wrong: I think it may be okay to just shift all contacts forward by 1. The contact list should be small and removal of contacts should be rare anyway?
- objects.h:2362 : I couldn't find the implementation of "dJointAddPUTorque", I don't think there is one? If there is, could you point me to it?
Best,
Til
Oleh Derevenko
Thank you Til. I will have a look.
Oleh Derevenko
-- Skype with underscore
From: ode-...@googlegroups.com <ode-...@googlegroups.com>
On Behalf Of Tilmann
Sent: Monday, June 5, 2023 1:25 PM
To: ode-users <ode-...@googlegroups.com>
Subject: [ode-users] Potential bugs
|
Ви нечасто отримуєте електронні листи від tilma...@gmail.com. Дізнайтеся, чому це важливо |
--
You received this message because you are subscribed to the Google Groups "ode-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to
ode-users+...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/ode-users/f540c220-a360-4e35-a34a-ad29389c9af3n%40googlegroups.com.
This e-mail may contain privileged and confidential information. If you are not the intended recipient, be aware that any use, disclosure, copying or distribution of this e-mail or any attachments is prohibited. If you have received this e-mail in error, please notify us immediately by returning it to the sender and delete this copy from your system. Thank you.
Oleh Derevenko
Hi Til
From: ode-...@googlegroups.com <ode-...@googlegroups.com>
On Behalf Of Tilmann
Sent: Monday, June 5, 2023 1:25 PM
To: ode-users <ode-...@googlegroups.com>
Subject: [ode-users] Potential bugs
while going over the code I found a few thing that may (or may not) need fixing. I'll list them here but I could also provide PRs if you think this is more convenient, please let me know. (Personally I prefer "hints" over PRs for small fixes on my own projects).
I am referring to the master branch:
- demo_jointPU.cpp:350 : I believe there is a "break;" missing
Fixed
- collision_libccd.cpp:768 : I believer this is currently returning the "index of the triangle index" instead of the triangle index itself. I think it should be "tempContact.side2 = indices[i];"
Fixed
- collision_cylinder_trimesh.cpp:1112 : Not entriely sure, other GIMPACT colliders (e.g. dCollideBTL() for boxes or dCollideTriumeshPlane() for planes) recomute the AABBs before colliding. Only
the cylinder collider does not do it. Potentially missing:
o1 -> recomputeAABB();
o2 -> recomputeAABB();
Fixed
- collision_libccd:800 : The code removes a contact and fills the empty position with the last contact in the list. I am not entirely sure but I thought contacts should be ordered by depth?
I think this is violated here. If this is wrong: I think it may be okay to just shift all contacts forward by 1. The contact list should be small and removal of contacts should be rare anyway?
The code does not seem to order by depth. It rather makes sure that if requested contact count is less than the available contact count the contacts with largest depths are returned. But these contacts are not ordered by depth and, therefore, it makes no sense to be shifting the array elements.
- objects.h:2362 : I couldn't find the implementation of "dJointAddPUTorque", I don't think there is one? If there is, could you point me to it?
The implementation was missing from the initial commit adding that joint. Since there are two degrees of rotational freedom the function may have no sense in its declared form. I personally never used the PU joint so
I can’t really judge here without getting into it.
Is there anyone who could suggest an implementation? Or should we remove the function?
Oleh Derevenko
-- Skype with underscore
Tilmann Zäschke
Hi Oleh,
thanks for fixing the issues!
> - collision_libccd:800 : The code removes a contact and
fills the empty position with the last contact in the list. I am
not entirely sure but I thought contacts should be ordered by
depth?
I think this is violated here. If this is wrong: I think it may
be okay to just shift all contacts forward by 1. The contact list
should be small and removal of contacts should be rare anyway?
> The code does not
seem to order by depth. It rather makes sure that if requested
contact count is less than the available contact count the
contacts with largest depths are returned. But these contacts
are not ordered by depth and, therefore, it makes no sense to be
shifting the array elements.
If was thinking about this issue:
https://bitbucket.org/odedevs/ode/issues/36/fix-gimpact-contacts-handling
The relevant function at collision_libccd:800 is
dCollideConvexTrimeshTrianglesCCD(...).
I may be mistaken, but my understanding was that Trimesh/Gimpact
collision contacts need to be sorted?
Best,
Til
--
You received this message because you are subscribed to the Google Groups "ode-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ode-users+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ode-users/AS4PR07MB889916900510928A06ABD696DD20A%40AS4PR07MB8899.eurprd07.prod.outlook.com.
Oleh Derevenko
Hi Tilmann,
Reading the Issue #36, the statement “ODE assumes that the contacts returned by Gimpact are ordered based on their importance” does not have any support provided and seems to be just the author’s assumption. I don’t know the initial plan but thinking logically and considering that contacts add equations to the equation system the order of equations should not matter. In the issue, the reporter solved the contact selection problem by sorting them before taking first few as a result. Here, we do not sort the contacts but, if the result contact count is bigger than the number of available slots, we make sure that the contact with minimum depth is replaced. This way we end with the same subset of the contacts with biggest depths — just, that the subset is not ordered.
Oleh Derevenko
-- Skype with underscore
From: ode-...@googlegroups.com <ode-...@googlegroups.com>
On Behalf Of Tilmann Zaschke
Sent: Sunday, June 25, 2023 1:19 PM
To: ode-...@googlegroups.com
Subject: Re: [ode-users] Potential bugs
|
Ви нечасто отримуєте електронні листи від zaes...@gmx.de. Дізнайтеся, чому це важливо |
Hi Oleh,
thanks for fixing the issues!
> - collision_libccd:800 : The code removes a contact and fills the empty position with the last contact in the list. I am not entirely sure but I thought contacts should be ordered by depth?
I think this is violated here. If this is wrong: I think it may be okay to just shift all contacts forward by 1. The contact list should be small and removal of contacts should be rare anyway?
> The code does not seem to order by depth. It rather makes sure that if requested contact count is less than the available contact count the contacts with largest depths are returned. But these contacts are not ordered
by depth and, therefore, it makes no sense to be shifting the array elements.
If was thinking about this issue: https://bitbucket.org/odedevs/ode/issues/36/fix-gimpact-contacts-handling
The relevant function at collision_libccd:800 is dCollideConvexTrimeshTrianglesCCD(...).
I may be mistaken, but my understanding was that Trimesh/Gimpact collision contacts need to be sorted?
Best,
Til
Tilmann Zäschke
Hi Oleh,
ah okay, I understand. Thanks for the clarification. Indeed,
ordering makes less sense if we already have the best contacts.
Thanks,
Til
--
You received this message because you are subscribed to the Google Groups "ode-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to ode-users+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/ode-users/AS4PR07MB88993A816A228562D5F13E79DD21A%40AS4PR07MB8899.eurprd07.prod.outlook.com.
Oleh Derevenko
Hi,
Since the PU joint internally inherits Universal joint, I’ve added dJointAddPUTorques() with implementation mimicking that for the latter.
Oleh Derevenko
-- Skype with underscore
From: 'Oleh Derevenko' via ode-users <ode-...@googlegroups.com>
Sent: Saturday, June 24, 2023 7:17 PM
To: ode-...@googlegroups.com
Subject: RE: [ode-users] Potential bugs
- objects.h:2362 : I couldn't find the implementation of "dJointAddPUTorque", I don't think there is one? If there is, could you point me to it?
The implementation was missing from the initial commit adding that joint. Since there are two degrees of rotational freedom the function may have no sense in its declared form. I personally never used the PU joint so
I can’t really judge here without getting into it.
Is there anyone who could suggest an implementation? Or should we remove the function?