[render] Deprecate CameraProperties and DepthCameraProperties #14376
Conversation
There was a problem hiding this comment.
Ignore revision 1 - that's a snapshot of #14375. R2 is the real PR. I just wanted to get a jump on CI, making sure I haven't missed anything.
+@EricCousineau-TRI for the finishing touch, please. After #14375 merges, I'll rebase this back to a single commit.
Reviewable status: LGTM missing from assignee EricCousineau-TRI(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @EricCousineau-TRI)
There was a problem hiding this comment.
Reviewed 12 of 12 files at r2.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @EricCousineau-TRI and @SeanCurtis-TRI)
a discussion (no related file):
Blocking pending merge of prerequisite PR
geometry/render/test/render_camera_test.cc, line 278 at r2 (raw file):
#pragma GCC diagnostic ignored "-Wdeprecated-declarations" /* Test the converstion constructor. This will be removed when CameraProperties
BTW spelling "converstion"
Though meh; test code, and deprecated test code at that.
SeanCurtis-TRI
force-pushed
the
PR_vtk_camera_intrinsics
branch
2 times, most recently
from
bb1ea55 to
423595b
Compare
There was a problem hiding this comment.
+(status: do not merge) (pending merge of prereq PR)
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @EricCousineau-TRI and @SeanCurtis-TRI)
geometry/render/test/render_camera_test.cc, line 278 at r2 (raw file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
BTW spelling "converstion"
Though meh; test code, and deprecated test code at that.
Done; A typo so good, it's worth having three times.
SeanCurtis-TRI
force-pushed
the
PR_vtk_camera_intrinsics
branch
4 times, most recently
from
da4b572 to
af19844
Compare
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @EricCousineau-TRI and @SeanCurtis-TRI)
a discussion (no related file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
Blocking pending merge of prerequisite PR
Given that prereq PR #14375 merged, is this almost ready to review?
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers, labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @EricCousineau-TRI and @SeanCurtis-TRI)
a discussion (no related file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
Given that prereq PR #14375 merged, is this almost ready to review?
Correction: "ready to have review be finished"?
This deprecates the old camera properties classes themselves. As part of
that endeavor:
- bindings deprecate the constructors.
- A slew of deprecation warning suppressions everywhere.
- In some cases, test cases were moved around to facilitate deprecation
suppression en masse (see render_camera_test.cc)
- Updated python tests to expect deprecation warnings.
SeanCurtis-TRI
force-pushed
the
PR_vtk_camera_intrinsics
branch
from
af19844 to
46bc33d
Compare
There was a problem hiding this comment.
@EricCousineau-TRI Ready for finishing the review.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers, labeled "do not merge" (waiting on @EricCousineau-TRI and @SeanCurtis-TRI)
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers, labeled "do not merge" (waiting on @EricCousineau-TRI)
a discussion (no related file):
Previously, EricCousineau-TRI (Eric Cousineau) wrote…
Correction: "ready to have review be finished"?
We can close this; we're officially rebased to a single commit.
There was a problem hiding this comment.
-(status: do not merge)
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers, labeled "do not merge" (waiting on @EricCousineau-TRI)
There was a problem hiding this comment.
+@sherm1 for simultaneous platform review, maybe?
Reviewable status: 1 unresolved discussion, LGTM missing from assignee sherm1(platform) (waiting on @EricCousineau-TRI and @sherm1)
There was a problem hiding this comment.
Reviewing now ...
Reviewable status: 1 unresolved discussion, LGTM missing from assignee sherm1(platform) (waiting on @EricCousineau-TRI and @sherm1)
There was a problem hiding this comment.
Platform 
Will there be some Anzu suffering to go with this?
Reviewed 11 of 12 files at r2, 8 of 10 files at r3.
Dismissed @EricCousineau-TRI from a discussion.
Reviewable status:complete! all discussions resolved, LGTM from assignees EricCousineau-TRI(platform),sherm1(platform)
|
I believe there will be some; I hope @EricCousineau-TRI will be willing to help suppress the deprecation warnings. |
|
Yup, can do! FTR I like more active requests than passive requests that make my ears burn ;) EDIT: See Anzu issue 6027 and PR 6026 |
This deprecates the old camera properties classes themselves. As part of that endeavor:
suppression en masse (see render_camera_test.cc)
Towards resolving checklist in #11880.
Closes #11880
This change is