Fix vertex colored flat shading

[ldcWV]

Motivation and Context

Currently, it is impossible to view a mesh that is flat shaded because this case is being handled incorrectly. Flat shaded meshes show up completely black. This PR fixes that issue.

Before:

After:

In addition, the property vertexColored is now removed from PhongMaterialData, as this is not technically a property of a material because it depends on the geometry of the mesh. It has now been changed to a flag for Drawable.

How Has This Been Tested

A flat shaded mesh was loaded into the viewer successfully. If more testing is necessary, please give suggestions.

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have completed my CLA (see CONTRIBUTING)
• I have added tests to cover my changes.
• All new and existing tests passed.

[bigbike]

Have you tried to load any vertex colored mesh?
Post the result image in the comments, will you? :)

[ldcWV]

Screenshot requested by @bigbike of a vertex colored mesh. Better before/after pictures will be uploaded later.

[bigbike]

Interesting. This mesh is vertex colored mesh for the Replica dataset. We can display it before even without your change.

Can you try the "master" branch on this mesh? I suspect you can display it already.

[ldcWV]

Just tried it on the master branch, and it's all black:

[eundersander]

Interesting. This mesh is vertex colored mesh for the Replica dataset. We can display it before even without your change.

Context on this test model: I provided it to Lawrence. It is a GLB that I created (not the Replica source PLY). I agree that we'd like to see another vertex-colored test model here.

[erikwijmans]

You'll need to add the HasVertexColor flag here

habitat-sim/src/esp/assets/ResourceManager.cpp

Line 1171 in 8ec2ef5

gfx::Drawable::Flags meshAttributeFlags{};

[ldcWV]

You'll need to add the HasVertexColor flag here

habitat-sim/src/esp/assets/ResourceManager.cpp

Line 1171 in 8ec2ef5

gfx::Drawable::Flags meshAttributeFlags{};

I pushed a change that does this, although I'm not sure if this is actually a good idea. It will apply to all instance meshes because currently, there's no way to determine whether it's actually supposed to be vertex colored or not (see the warning comment under the changed line). This is a bigger issue than this PR is trying to address.

[erikwijmans]

Yep, not the job of this PR to address that issue, but currently we default to instance meshes have vertex coloring, so we shouldn't change that unintentionally.

[bigbike]

Good job, @ldcWV !

I approved this PR but left a question for you to consider.

Also please wait the CI tests passes.

[Skylion007]

Pull from master to fix the Habitat-Lab issue.

[bigbike]

Wait. Why I cannot see any comments from @eundersander ? I received a lot of comments in email notification. But cannot see any here?

To answer the "invisible" question:
Yes, that model is purple. I confirm it. Sorry I happened to give him a model that looks a bit strange.

[ldcWV]

Wait. Why I cannot see any comments from @eundersander ? I received a lot of comments in email notification. But cannot see any here?

To answer the "invisible" question:
Yes, that model is purple. I confirm it. Sorry I happened to give him a model that looks a bit strange.

He replied to an already resolved conversation. The thread is the second one in your approval comments.

[eundersander]

Nice work! LGTM!