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

Require accessor min/max for POSITION? #593

Closed
pjcozzi opened this issue May 18, 2016 · 11 comments
Closed

Require accessor min/max for POSITION? #593

pjcozzi opened this issue May 18, 2016 · 11 comments

Comments

@pjcozzi
Copy link
Member

pjcozzi commented May 18, 2016

See CesiumGS/cesium#3925

@pjcozzi
Copy link
Member Author

pjcozzi commented Jun 1, 2016

Labeling as resolved. See fdf4337 in the 1.0.1 branch.

@lexaknyazev
Copy link
Member

Just to be clear, why are min|max always required in fdf4337, not just for POSITION?
Are there any common use cases for NORMAL or TEXCOORD?

@lexaknyazev
Copy link
Member

One more possibly worth mentioning constraint: min|max length must be one of [1, 2, 3, 4, 9, 16] and match accessor's type.

@pjcozzi
Copy link
Member Author

pjcozzi commented Jun 3, 2016

One more possibly worth mentioning constraint: min|max length must be one of [1, 2, 3, 4, 9, 16] and match accessor's type.

See #617.

Just to be clear, why are min|max always required in fdf4337, not just for POSITION?
Are there any common use cases for NORMAL or TEXCOORD?

This is for simplicity and consistency. Early on, we received some glTF feedback that all properties should be required. We didn't agree as it can make things verbose; however, experience has shown that having some optional and some required properties has caused some implementation confusion for clients - which goes against one of our main goals: making glTF easy to implement consistency. So I think requiring min/max only for POSITION starts to make the required/optional rules too complex and could lead to a less robust ecosystem.

@lexaknyazev
Copy link
Member

So I think requiring min/max only for POSITION starts to make the required/optional rules too complex and could lead to a less robust ecosystem.

I was curious because of opposite approach in #616 (diff).

@pjcozzi
Copy link
Member Author

pjcozzi commented Jun 3, 2016

Ah, yes, I think we should make #616 (diff) apply to all attributes, not just POSITION.

@mlimper @lasalvavida what do you think?

Just to be clear, why are min|max always required in fdf4337, not just for POSITION?
Are there any common use cases for NORMAL or TEXCOORD?

This is for simplicity and consistency. Early on, we received some glTF feedback that all properties should be required. We didn't agree as it can make things verbose; however, experience has shown that having some optional and some required properties has caused some implementation confusion for clients - which goes against one of our main goals: making glTF easy to implement consistency. So I think requiring min/max only for POSITION starts to make the required/optional rules too complex and could lead to a less robust ecosystem.

@mlimper
Copy link
Contributor

mlimper commented Jun 5, 2016

It doesn't seem like a huge amount of work for implementers of glTF writers if we require them to also write min/max for other attributes, given that they would already provide this information for POSITION.
On the client side, requiring min/max will facilitate implementation... although I must admit that I currently don't have any important real-world use case in mind where one would need min/max for normals / texcoords. Maybe you could figure out that you are looking at an object where everything is viewed from the back (using normal min/max) and then not render it, if backface culling would be enabled?^^
To conclude, I would tend towards making min/max required for all attributes, for simplicity.

@pjcozzi
Copy link
Member Author

pjcozzi commented Jun 6, 2016

Thanks for the input, @mlimper!

@lexaknyazev are you good with this? The WEB3D_quantized_attributes spec will also go this direction.

@lexaknyazev
Copy link
Member

@lexaknyazev are you good with this? The WEB3D_quantized_attributes spec will also go this direction.

Consistency matters.

On the client side, requiring min/max will facilitate implementation

Validation will be simplier too.

@pjcozzi
Copy link
Member Author

pjcozzi commented Jun 6, 2016

Thanks for the prompt response! 😄

@pjcozzi
Copy link
Member Author

pjcozzi commented Jun 15, 2017

Updated in #826

@pjcozzi pjcozzi closed this as completed Jun 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants