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

Adding feature IDs to custom shaders #9894

Closed
sanjeetsuhag opened this issue Oct 26, 2021 · 4 comments · Fixed by #10018
Closed

Adding feature IDs to custom shaders #9894

sanjeetsuhag opened this issue Oct 26, 2021 · 4 comments · Fixed by #10018

Comments

@sanjeetsuhag
Copy link
Contributor

It may be useful to have access to the feature ID value in the custom shaders. Based on offline discussion with @ptrgags, this is the design we came up with:

struct FeatureIds {
  float attributes[3];
  float textures[2];
}

This example shows the struct that would be generated for a primitive with 3 feature ID vertex attributes and 2 feature ID textures. Inside the custom shader, the values could be accessed by something like fsInput.featureIds.attributes[0] or fsInput.featureIds.textures[1].

A couple of open questions:

  1. Is there a better name for textures? Currently, this seems to denote that you're accessing a texture, instead of a value extracted from a texture.
  2. Does there need to be another member of the struct that denotes the "active" feature ID i.e. the one that is used for picking/styling?

CC: @lilleyse

@lilleyse
Copy link
Contributor

lilleyse commented Oct 26, 2021

Is there a need to separate attributes and textures? In the latest EXT_mesh_features they are all one featureIds array and I think the custom shaders "API" should mirror that.

The JS application should know the active feature id so I don't think it necessarily needs to be part of the struct.

Also the type should be int.

@ptrgags
Copy link
Contributor

ptrgags commented Oct 26, 2021

@lilleyse in regards to keeping textures separate:

  1. for EXT_feature_metadata backwards compatibility how do we number the feature textures? how does the user know which index to use?
  2. What about differences between vertex and fragment shaders? you still want feature ID attributes in the VS (but no feature ID textures), but you don't want them to be re-ordered in the FS, that would get confusing.
  3. even in EXT_mesh_features, in the glTF you put FEATURE_ID_0 for the attribute, but if you declared:
featureIds:
   { offset: 1, repeat: 3}
   { attribute: 0} // this is now fsInput.featureIds[1]... a bit confusing but I guess you just need to document it well

@lilleyse
Copy link
Contributor

Good points...

  1. For EXT_feature_metadata backwards compatibility maybe we just need some convention that attributes are first, then textures.
  2. I would keep the same order but maybe assign a sentinel value to the feature ids that come from a feature id texture
  3. I think this would be a problem with the design above too. Documentation should clear it up. Also, FEATURE_ID_0, FEATURE_ID_1, etc are still accessible through the attributes struct right? That's one avenue if users want to go straight to the vertex attribute.

@ptrgags
Copy link
Contributor

ptrgags commented Oct 26, 2021

@lilleyse

  1. Sounds good
  2. hm, if we're using type int, we could make -1 the sentinel. Or for consistency, use the maximum index + 1
  3. yes, fsInput.attributes.featureId0 would always be the first feature ID with an explicit accessor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants