-
Notifications
You must be signed in to change notification settings - Fork 3.6k
allow further texture uniforms #12558
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for the pull request, @bkuster! ✅ We can confirm we have a CLA on file for you. |
A friendly reminder to @jjhembd to take a pass on this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bkuster! The code change looks good to me. I think we only need to extend the docs update to a couple more places inside Texture
. Then this should be good to go!
@@ -483,7 +483,7 @@ function loadFramebufferSource(texture, source) { | |||
* Load texel data from an Image into a texture. | |||
* | |||
* @param {Texture} texture The texture to which texel values will be loaded. | |||
* @param {ImageData|HTMLImageElement|HTMLCanvasElement|HTMLVideoElement} source The source for texel values to be loaded into the texture. | |||
* @param {ImageData|HTMLImageElement|HTMLCanvasElement|HTMLVideoElement|OffscreenCanvas|ImageBitmap} source The source for texel values to be loaded into the texture. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This update to the type should also apply to the functions that call loadImageSource
.
- The doc for
Texture.copyFrom
should be a straightforward update. - The
Texture
constructor itself is messier, but I thinkTexture.ConstructorOptions.source
could have the same doc asoptions.source
fromTexture.copyFrom
.
Description
The number of image data types available to
Materials
should be upgraded with some more data types:In my usecase, I already have an ImageBitmap created in a worker and would like to avoid having to draw it onto a canvas to then pass it to the Texture, when I could just pass it directly.
Testing plan
Added tests to MetarialSpec.js for both use cases.
Author checklist
CONTRIBUTORS.md
CHANGES.md
with a short summary of my change