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

Add vertex color support to mesh_loader #1144

Open
wants to merge 1 commit into
base: kinetic-devel
Choose a base branch
from
Open

Add vertex color support to mesh_loader #1144

wants to merge 1 commit into from

Conversation

emersondispatch
Copy link

@emersondispatch emersondispatch commented Aug 23, 2017

This allows you to view colors on RobotModels (etc.) that are defined via Collada (etc.) and do not have textures but instead have per-vertex color.

@wjwwood
Copy link
Member

wjwwood commented Aug 30, 2017

Thanks for the patch. We'll have a look asap.

@wjwwood wjwwood added this to the untargeted milestone Aug 30, 2017
@dhood
Copy link
Contributor

dhood commented Dec 11, 2017

@emersondispatch are you able to provide an example model for us to test with?

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.

I found a mesh to test with at https://forums.sketchup.com/t/import-of-vertex-colored-collada-dae-models/27706 and while this change fixes the display for that mesh, it has an impact on other meshes (see below)

@@ -447,6 +465,7 @@ void loadMaterials(const std::string& resource_path,

Ogre::Technique* tech = mat->getTechnique(0);
Ogre::Pass* pass = tech->getPass(0);
pass->setVertexColourTracking(Ogre::TVC_DIFFUSE);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is impacting other meshes, e.g. rosrun rviz mesh_marker_test and you'll see some of the meshes flicker because of this change. I don't have a suggestion yet for a fix

@dhood
Copy link
Contributor

dhood commented Apr 26, 2018

I've put the "changes requested" label on this because of #1144 (comment) which we don't have time to investigate ourselves. I've put "help wanted" in case @emersondispatch or someone else has time to look at it.

@wjwwood wjwwood removed this from the untargeted milestone May 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants