MeshcatVisualizer sets visibility directly without events #19578
Conversation
RussTedrake
added
status: single reviewer ok
There was a problem hiding this comment.
Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform)
a discussion (no related file):
See RussTedrake#50.
Changing the visibility should be congruent with sending the initial batch of geometry. There is no need to send it prior to that.
jwnimmer-tri
removed
the
status: single reviewer ok
There was a problem hiding this comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @RussTedrake)
a discussion (no related file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
See RussTedrake#50.
Changing the visibility should be congruent with sending the initial batch of geometry. There is no need to send it prior to that.
I reviewed and like that change. Thanks. How should we finish out the review process?
There was a problem hiding this comment.
+@rpoyner-tri for platform review per schedule, please.
Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @RussTedrake)
a discussion (no related file):
Previously, RussTedrake (Russ Tedrake) wrote…
I reviewed and like that change. Thanks. How should we finish out the review process?
I think we can treat it as co-author and co-feature-reviewer, so all we'll need is platform review as a final step.
jwnimmer-tri
changed the title
There was a problem hiding this comment.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @RussTedrake)
RussTedrake
force-pushed
the
meshcat_visualizer_invisible
branch
from
854ab7f to
7e7cd52
Compare
Previously, the visibility parameter was not respected (a) before the initialization event or (b) if delete_on_initialization_event was false. fixup! Update visibility exactly when we first transmit shapes to Meshcat. (Don't add to the scene tree panel too early.) On the same tack, don't update visibility during re-initialization. We don't want the dummy note in the scene tree panel then, either.
RussTedrake
force-pushed
the
meshcat_visualizer_invisible
branch
from
7e7cd52 to
56afc18
Compare
There was a problem hiding this comment.
Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: 1 unresolved discussion
-- commits line 8 at r3:
nit this line seems out of place -- should I manually delete it when merging the patch?
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status: 1 unresolved discussion
|
I'm going to label this +(release notes: breaking change) not because its necessarily breaking but because it's "important" and maybe "especially surprising", so we should probably say more about it in the release notes. (See #19702.) |
jwnimmer-tri
added
the
release notes: breaking change
…otion#19578) Previously, the visibility parameter was not respected (a) before the initialization event or (b) if delete_on_initialization_event was false. Now, we update visibility exactly when we first transmit shapes to Meshcat and we don't update visibility during (re-)initialization.
Previously, the visibility parameter was not respected (a) before the initialization event or (b) if delete_on_initialization_event was false.
+@jwnimmer-tri for both reviews, please.
This change is