--[PBR Update 1 of 3] Map materials' extension/layer data to Drawables cache #2112
Conversation
use >= for comparisons when comparator enums consist of more than 1 bit.
facebook-github-bot
added
the
CLA Signed
| .addSource(flags_ & Flag::ObjectId ? "#define OBJECT_ID\n" : "") | ||
| .addSource(flags_ & Flag::ClearCoatLayer ? "#define CLEAR_COAT\n" : "") |
There was a problem hiding this comment.
Oops one flag got past me as I cherry picked this. If this antagonizes anyone 😁 I'll get rid of it, but it's going to be added in the next PR anyway.
| Mn::Shaders::PhongGL::Flag::InstancedObjectId) || | ||
| ((flags_ & Mn::Shaders::PhongGL::Flag::ObjectIdTexture)) == | ||
| Mn::Shaders::PhongGL::Flag::ObjectIdTexture)) | ||
| : (((flags_ >= Mn::Shaders::PhongGL::Flag::InstancedObjectId) || |
There was a problem hiding this comment.
Why do we compare flags with >=? Isn't flags basically a bitfield?
There was a problem hiding this comment.
that's how magnum compares multi-bit flags
There was a problem hiding this comment.
Docs for reference, >= is chosen because it looks close to a ⊇, a symbol for a superset.
There was a problem hiding this comment.
Ok so, what the heck do these lines mean? :) Is it testing whether flags_ includes InstanceObjectId?
There was a problem hiding this comment.
Is it testing whether
flags_includesInstanceObjectId?
Yes. It's a shorthand for (a & other) == other, as the docs say. It does exactly what it did before -- checking if one set of bits is a superset of the other set -- just in a more concise way.
Motivation and Context
This PR is the first of 3 (or possibly 4) PRs to implement Habitat's new PBR materials extension support, drawn from the original WIP found here.
In this PR the gltf extension data present in the Magnum material are loaded into each drawable's existing cache structure, to be more easily accessed each draw. No changes to the shader are present in this PR.
How Has This Been Tested
Existing C++ and python tests pass locally.
Types of changes
Checklist