webots
webots copied to clipboard
Add `Pose` node (like `Transform` node but without `scale`)
Description
Scaling of robots is problematic due to the presence of sensors and physics. The Pose
node, from which Transform
, Solids
and Robots
will inherit, is much like a Transform
node but without the scale parameter. This latter will be exposed by the "new" Transform
node, in order to maintain backwards compatibility.
Related Issues Fixes https://github.com/cyberbotics/webots/issues/4034
Tasks Add the list of tasks of this PR.
- [x] add
Pose
node- [x] rename
Transform
toPose
- [x] rename
AbstractTransform
toAbstractPose
- [x] rename node constants
- [x] rename utilities like
findUpperTransform
,findUppermostTransform
, ... - [x] rename variables, like
WbPose *transform
toWbPose *pose
- [x] rename
- [x] create
WbTransform
class that inherits fromWbPose
- [x] make
WbMatter
inherit fromWbPose
- [x] make
WbSkin
inherit fromWbTransform
instead ofWbAbstractTransform
- [x] remove
scale
fromPose
(and all references to it inSolid
,Robot
, ... ) - [ ] check
WbTrack
functionality - [ ] only allow
Shape
children inTransform
nodes - [ ]
boundingObjects
should allow only Pose - [ ] x3d exporting of transform/pose
- [ ] update docs
- [ ] update node chart
- [ ] script to automatically detect/change
Transform { Solid {}}
structures intoPose { Solid {}}
(should be rare)
If it were possible, my inclination would be to instead move in the direction of letting scale
work meaningfully with more things, rather than reducing the number of things that have a scale
field. E.g., it'd be nice to be able to make oblate spheroid bounding objects. Or is it prohibitively difficult to get the physics to work right with those?
I did run into a lot of perplexment once when I had scaled a Camera
node to make its visual appear the way I wanted, and discovered that this scale
also squashed the camera's image (or at least the location of recognition objects on the image)! That surprised me and took awhile to debug, but in the end I decided that was probably sort of working as designed. But this "fix" probably would have kept me from stumbling into such perplexment, by preventing me from scaling the camera at all.
Also I have doubts about the claim that it "should be rare" that people would have Solid
descendants inside a Transform
node. I think I have a fair number of those, since this is an intuitive way to position a group of items, including some uses in places that your script wouldn't reach (content to be imported into the world via supervisor). The backwards incompatibility issues here would work out to be a headache for me, though perhaps I'm unusual. Again, that would make me lean in the direction of a different solution, e.g., making scale
work as you would intuitively expect it to on solids if that's feasible, or just making scale
fields have no effect on solids/robots, but not actually removing those fields or forcing people to convert old node-types to new ones.
make WbSkin inherit from WbTransform instead of WbAbstractTransform
Note that the Skin
node should not have the children
field, so if Transform
does it then Skin
cannot inherit from Transform
and we still need an intermediate node like WbAbstractTransform
.
~~mmh, that might be a bit of a problem. I assume the skin node does require the scale, correct?~~ A better approach might be to consider the Skin an exception and be its own thing, otherwise it gets messy
mmh, that might be a bit of a problem. I assume the skin node does require the scale, correct?
Yes.
The WbAbstractTrasform
class has been introduced to avoid duplicating all the translation/rotation/scale-related code in WbTransform
and WbSkin
.
Changing WbSkin
so that inherits from the new WbAbstractPose
would at least avoid duplication of code related to translation and rotation. It should be evaluated the size of the scale-related code.
Changing WbSkin so that inherits from the new WbAbstractPose would at least avoid duplication of code related to translation and rotation. It should be evaluated the size of the scale-related code.
I see, I'm not entirely sure WbAbstractPose would even need to exist after the rework to be honest. Since the scale-related methods would be moved down to WbTransform, the only difference that remains between WbPose & WbAbstractPose is the children field. After discussing with Olivier, it might be cleaner to just have a Pose node that inherits from Group (hence avoid multiple inheritance too) and consider the Skin its own thing. It would entail some duplication that's true, but frankly given that the scale
now drop down at the Transform level, I'm not convinced the benefit of code-reutilization justifies adding an intermediate Frankenstein just for the Skin in order to have both a scale
but not the children
field, making the two coexist might turn out worse than duplicating translation/rotation. There's a lot of moving pieces though, so I'll investigate further
As it stands, it would look something like this, but like I mentioned there might be roadblocks:
WbGroup (children)
|
|
|__ WbPose (add translation + rotation)
|
|__ WbSolid (add solid stuff) __ WbRobot
|
|__ WbTransform (add scale)
WbSkin
If WbSkin
doesn't have a common base class without the children field with WbPose
you have to duplicate all the code to handle the translation and rotation in the WbSkin
.
This will mean that almost all the code that now it is in WbAbstractTransform
should be added both in WbSkin
AND WbPose
.
It is quite a lot of code and duplicating (even ignoring the scale part) will pose many maintenance issues on the long term.
How do you suggest Skin to have scale but not children in this context (i.e. with the addition of the Pose node that doesn't have scale)?
I would keep at least the WbAbstractPose class including all the translation/rotation code and use it as base class for the WbPose and WbSkin classes.
I'm fine with that, but that would still leave duplication for the scale, as I don't see another way of adding scale support without also getting children in the mix (or at least, not without making a mess with the classes)
continued here: https://github.com/cyberbotics/webots/pull/5978