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

Move WebGLConstants from Renderer to Core #4731

Merged
merged 12 commits into from
Dec 13, 2016

Conversation

lasalvavida
Copy link
Contributor

Justification:

Currently we use WebGLConstants in Core (ComponentDataType, IndexDataType, PixelFormat, PrimitiveType, ...), but WebGLConstants lives in Renderer. Moving WebGLConstants to Core makes Core completely self contained with no dependencies on the rest of Cesium.

This PR also makes WebGLConstants public and updates the reference doc for it. We use WebGLConstants as part of the Cesium API in gltf-pipeline, so this seemed appropriate.

There were also a few members missing @memberof declarations in other modules which were being added to global in the doc, so this corrects those as well.

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 8, 2016

@lasalvavida this is perfect, just one request: for maintenance and conciseness, it is perfectly OK to not document each individual enum vale in this case; instead, the reference doc for WebGLConstants can say that these match the WebGL spec constants and provide a link to the WebGL 1 and WebGL 2 specs.

@lasalvavida
Copy link
Contributor Author

lasalvavida commented Dec 8, 2016

@lasalvavida this is perfect, just one request: for maintenance and conciseness, it is perfectly OK to not document each individual enum vale in this case; instead, the reference doc for WebGLConstants can say that these match the WebGL spec constants and provide a link to the WebGL 1 and WebGL 2 specs.

Okay. I need to see if I can make jsdoc hide the description column here for this. I added the comments because having a completely empty column looked a bit odd.

@lasalvavida
Copy link
Contributor Author

This is updated removing the per entry comments and I've added links to the WebGL 1.0 and 2.0 specifications. I can't find a good way to hide the description column, so it is still there currently. Let me know if there are any other changes you would like to see, otherwise this is good to go.

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 13, 2016

Thanks @lasalvavida!

@pjcozzi pjcozzi merged commit 8328880 into CesiumGS:master Dec 13, 2016
@pjcozzi pjcozzi deleted the core-webglconstants branch December 13, 2016 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants