Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for 1120 #1132

Merged
merged 4 commits into from
Aug 4, 2017
Merged

Fix for 1120 #1132

merged 4 commits into from
Aug 4, 2017

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented Aug 3, 2017

Fixes #1120.

Basically #1084 caused a regression that caused markers which had a mesh text and color to render white until the color or alpha was updated, after which point it would render correctly.

I also improved the marker mesh test executable to test a few of these things. Here's a screen shot of the behavior with this pr (the correct behavior imo):

screen shot 2017-08-03 at 4 47 51 pm

@wjwwood wjwwood self-assigned this Aug 3, 2017
@wjwwood wjwwood requested a review from dhood August 3, 2017 23:57
@wjwwood
Copy link
Member Author

wjwwood commented Aug 3, 2017

I should note that the first row of items in the above matrix of test markers only has something in the first column. The other columns are empty, though I might update the tool again to fill the spots out, just for completeness.

even though the previously missing ones would be
invisible in the correct behavior
@wjwwood
Copy link
Member Author

wjwwood commented Aug 4, 2017

41bf5e2 adds the missing mesh markers to the matrix, but it doesn't affect the screen shot as they are invisible due to alpha of 0.

@wjwwood
Copy link
Member Author

wjwwood commented Aug 4, 2017

@ros-pull-request-builder retest this please

Copy link
Contributor

@dhood dhood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. A side effect of this is that some meshes that used to be visible are now correctly rendered as invisible. Desired behaviour FWIU, but wanted to bring it up in case it's worth an announcement

Without dc094c7:
screen shot 2017-08-04 at 4 47 51 pm

With dc094c7:

screen shot 2017-08-04 at 4 47 51 pm

@wjwwood wjwwood merged commit c3d955c into indigo-devel Aug 4, 2017
@wjwwood wjwwood deleted the fix_1120 branch August 4, 2017 20:34
wjwwood added a commit that referenced this pull request Aug 21, 2017
* improve mesh marker test tool

* fix first time color on mesh markers

fixes regression of #1084

* fill out matrix of mesh markers

even though the previously missing ones would be
invisible in the correct behavior

* remove travis in favor of build.ros.org pr testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants