RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.17k stars 1.24k forks source link

Generalize RenderEngineGl's geometries and instances #21522

Closed SeanCurtis-TRI closed 3 weeks ago

SeanCurtis-TRI commented 4 weeks ago

Previously, RenderEngineGl would assume a great deal about the geometries and how they get posed in the world. This changes those assumptions:

Relates #20185 and #21520.


This change is Reviewable

SeanCurtis-TRI commented 4 weeks ago

+(release notes: none)

To see how this fits into the larger picture, see the overview PR in which RenderEngineGl consumes glTF files (#21520).

ggould-tri commented 3 weeks ago

geometry/render_gl/internal_opengl_geometry.h line 243 at r2 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
BTW The [GSG policy](https://drake.mit.edu/styleguide/cppguide.html#Variable_and_Array_Initialization) only requires member initialization for built-in types, not an Eigen type. The defect here would need to be something more specific like "too brittle of a footgun to live" or "confused about when this actually gets populated" or something like that.

Another of the many reasons to object would be "the default-initialized Matrix4f is not a homogeneous transform matrix as documented above".

ggould-tri commented 3 weeks ago

geometry/render_gl/internal_opengl_geometry.h line 87 at r2 (raw file):

Previously, ggould-tri wrote…
minor: Per the discussion below that these are 4x4, shouldn't this have four elements on the diagonal?

(whoops, that should be "1" instead of "0")