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

fix issue #124: Texture and MipMap types #227

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

capnmidnight
Copy link
Contributor

Why

Issue #124 discusses a desire to have concrete types for the image and mipmaps fields of THREE.Texture.

This is made difficult by the fact that Three.js appears to occasionally use the images field not as an actual source of image data, but as a metadata field to store width/height information about composite texture types like THREE.CubeTexture.

For the mipmaps field, all of the loaders in the examples code end up returning an ImageData structure, but in some cases like THREE.VideoTexture, the mipmaps field could contain an HTMLVideoElement. In general, Three.js does some checking on the type of the THREE.Texture before choosing whether to use the image field or one of the mipmaps array elements for a call to various texture loading functions in WebGL, some of which are capable of loading any TexImageSource. Indeed, it appears it's kept as such to also support manually generating mipmaps, e.g. by resizing images via rendering to a Canvas.

What

For all of the Loader types in the examples folder, I've updated their returned mipmaps fields to be ImageData[] instead of object[].

For the base THREE.Texture type, I started by defining a number of utility types that describe all the extant ways that the image field appears to be used in Three.js. I then used these to create optional type parameters to THREE.Texture. Subclasses of THREE.Texture can then specify values for those type parameters, e.g. THREE.VideoTexture specifies that the image type is HTMLVideoElement.

One curious thing is that there actually aren't a lot of restrictions on the types of images you put in THREE.Textures. For example, THREE.VideoTexture could contain an HTMLCanvasElement. All THREE.VideoTexture does is run an animation loop to set needsUpdate without the user having to do so in their own code. You could conceivably use THREE.VideoTexture with a canvas object that you intend to draw to continuously, saving you from having to write an animation loop to update the texture. Similarly, THREE.CanvasTexture doesn't really care if you give it a canvas or not, all it does is set needsUpdate to true.

But in those cases, I opted to be explicit in the typing of the textures. If someone were to want to do some shenanigans like that, they can always just cast to any.

Finally, I also updated the type signatures for THREE.WebGLRenderTarget and its subclasses, so that their texture fields could also be typed more explicitly.

I'm submitting this as a draft PR because I'm not really sure of the ergonomics of this. The tests pass, I even managed to write a few that prove out the general concept in type checking. But I'm not really sure how useful the generic THREE.Texture type with the default type parameters will be when dealing with textures loaded from model loaders. Well, at the very least, it gives you more guidance on how to go about type filtering on those values, but I'm curious to see what others think.

Checklist

  • Checked the target branch (current goes master, next goes dev)
  • Added myself to contributors table
  • Ready to be merged

@capnmidnight capnmidnight force-pushed the texture-image-and-mipmaps-types branch from a723409 to a5b3c30 Compare June 14, 2022 16:38
@capnmidnight capnmidnight force-pushed the texture-image-and-mipmaps-types branch from a5b3c30 to 49cffe4 Compare June 14, 2022 16:44
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.

1 participant