[WIP] Refactor Bullet plugin implementation #286
Conversation
joxoby
force-pushed
the
bullet-refactor
branch
3 times, most recently
from
9766391 to
9f182cf
Compare
| public: inline Identity AddWorld(std::unique_ptr<World> &&_world) { | ||
| auto id = this->GetNextEntity(); | ||
| this->world = std::move(_world); | ||
| auto identity = this->GenerateIdentity(id, nullptr); |
There was a problem hiding this comment.
Note how I'm using nullptr for the generated Identity. The function GenerateIdentity only takes a shared_ptr which means that it will also manage the lifetime of the related object. Since we don't want this, we could potentially add a version of GenerateIdentity that takes a raw pointer instead?
joxoby
force-pushed
the
bullet-refactor
branch
from
9f182cf to
fe3eeca
Compare
joxoby
force-pushed
the
bullet-refactor
branch
5 times, most recently
from
eb8f64c to
b57b5dd
Compare
joxoby
changed the title
bullet/src/SDFFeatures.cc
Outdated
| const std::size_t child = this->FindOrConstructLink( | ||
| modelIdentity, _sdfModel, sdfJoint->ChildLinkName()); | ||
| // Construct all the model's links | ||
| for (std::size_t i = 0; i < _sdfModel.LinkCount(); ++i) { |
There was a problem hiding this comment.
For some reason, this loop never happens since _sdfModel.LinkCount() always gives 0. All the links seem to be constructed after all the models. ConstructSdfLink seems to be called through another path. This seems to be a problem: since I can't know a priori the number of links that a model has, I can't create an instance of btMultiBody...
There was a problem hiding this comment.
That's one of the reasons we did not use btMultiBody. One solution we thought about at some point when trying to use it, was create some sort of nullptr, and "recreate" the multibody once you have enough information. Also, that would make you "recreate" the multibody any time something changes
There was a problem hiding this comment.
That could work. The problem is programmatically defining the "enough information" trigger.
There was a problem hiding this comment.
AFAIK, once you reach joint creation, you have already received all links (That only applies if you are not going to have detachable joints at some point). Edit* Even though is true, I meant on Collision creation, that's the direct next step after link creation.
Also
ConstructSdfLinkseems to be called through another path.
Ign seems to consume the different Construct functions you make available. Probably @azeey can correct me if I'm wrong.
There was a problem hiding this comment.
Ign seems to consume the different Construct functions you make available. Probably @azeey can correct me if I'm wrong.
Yes, it looks that it's called here: https://github.com/ignitionrobotics/ign-gazebo/blob/26832764749291b6ea22b525262136ecbb63fbb9/src/systems/physics/Physics.cc#L821-L820
AFAIK, once you reach joint creation, you have already received all links
We would basically create the btMultiBody instance inside ConstructSdfJoint. But, what if there are no joints?
(That only applies if you are not going to have detachable joints at some point).
Detachable Joints could be implemented separately using btFixedconstraints or something, WDYT?
There was a problem hiding this comment.
I'm not sure if detached joints are considered part of the same model in IGN.
You would normally use detached joints to connect links across different models.
There was a problem hiding this comment.
I mean, if you have a model, and you detach a link, is that detached joint still considered part of that model in the IGN's structure, or is a new model created with that link inside? I'm just thinking about the btMultibody structure, and if the latter one is correct, you would need to create 2 new ones on detaching.
There was a problem hiding this comment.
In the IGN structure, I don't think that detachable joints are part of any model. They just connect two different models together. When you detach a joint, you're detaching different models, not a link from a model.
There was a problem hiding this comment.
Then btFixedconstraints should make the trick
There was a problem hiding this comment.
Sorry, I missed one step. Don't create it on joint creation, create it on Collision creation.
The problem here is that we probably want to create btRigidBody for models without joints and btMultiBody for models with joints. This information is not yet available during Collision creation.
Just FYI, a related comment on this: #44 (comment) . |
Signed-off-by: Juan Oxoby <[email protected]>
joxoby
force-pushed
the
bullet-refactor
branch
from
b57b5dd to
c284da5
Compare
Refactor
Summary
Refactors Bullet plugin implementation to use a hierarchical memory ownership model. See Base.hh for detailed explanation. This representation greatly simplifies the entity management implementation.
EDIT: I'm currently looking into switching to
btMultiBodyso that Bullet uses reduced coordinates instead.CC: @Blast545 @Lobotuerk