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

Add support for WebGL2 #838

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Add support for WebGL2 #838

wants to merge 8 commits into from

Conversation

tsnee
Copy link

@tsnee tsnee commented Mar 10, 2024

This PR is an attempt to add complete support for WebGL 2 (as defined at MDN, not the official WebGL2 spec). In the course of preparing this PR I have uncovered a few inconsistencies between the two and alerted MDN.

I was advised to use Int for everything that would fit in 32 bits and Double for everything that would not, and I did my best to follow that advice, but I don't know if there is a way to verify that I did it perfectly.

Any and all comments and criticism are welcome.

Change types of GLintptr and GLsizeiptr values from Int to Double.
Add tables to WebGLCompressedTextureASTC ScalaDoc.
Reformat.
@zetashift
Copy link
Contributor

Thank you very much!!! This is amazing work. The reviewing process might be a bit slow, so please bear with us!

@@ -0,0 +1,17 @@
/*
* Copyright (C) 2014 Matt Seddon. This source is donated in full to the scala-js-dom project.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, who is Matt Seddon? What is the origin and license of this code?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created this file and and several others by copying WebGLBuffer.scala and changing only the class name and comment. I didn't feel good about copying the code without the copyright statement. Did I err?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. It looks like these contributions originated in #14. Thanks for bringing this to my attention.

To be clear, the issue is not the (C) 2014 Matt Seddon. The issue is that "donated in full" is not a well-known license. I don't know what that means. For example, does "donated in full" allow you to copy and modify the code like you just did in this PR?

@mseddon would you be able to clarify the license that this code is under?

Copy link
Contributor

@mseddon mseddon Jun 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, that is me, it appears to have crept in from a template I had years ago- please remove this and any comment like it, it is unnecessary. All code committed here is property of the scala-js-dom project. You have complete ownership of it, and I relinquish any perceived claims of ownership. I'm happy to merge. Apologies delaying this PR, and thank you @tsnee for contributing this much needed feature!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@armanbilge Confirming via ping since there hasn't been much movement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mseddon Thanks for the ping, I had missed this (although haven't had much time recently for sjs-dom maintenance). I appreciate the clarification! In retrospect, it should have been obvious that mseddon is Matt Seddon 😅

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.

4 participants