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 concrete types to the image and mipmaps properties of Texture #124

Open
mewore opened this issue Sep 3, 2021 · 2 comments
Open

Add concrete types to the image and mipmaps properties of Texture #124

mewore opened this issue Sep 3, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@mewore
Copy link

mewore commented Sep 3, 2021

Describe the feature you'd like:

image: any; // HTMLImageElement or ImageData or { width: number, height: number } in some children;

mipmaps: any[]; // ImageData[] for 2D textures and CubeTexture[] for cube textures;

If it's certain that these are the only types they can have, then they can be added to the type declaration itself.

However, note that this is a breaking change for the transpilation of TS code which directly uses any sub-properties of these properties (except .image.width and .image.height, which are always present) or casts one of them to an unrelated type.

Suggested implementation:

    /**
     * @default THREE.Texture.DEFAULT_IMAGE
     */
    image: HTMLImageElement | ImageData | {width: number, height: number};

    /**
     * The array of the texture mipmaps ({@link ImageData} for 2D textures and {@link CubeTexture} for cube textures).
     * 
     * @default []
     */
    mipmaps: ImageData[] | CubeTexture[];

This is based on my understanding that if it's a 2D texture, then all of its mipmaps are of type ImageData, and if it's a cube texture, then all of its mipmaps are of type CubeTexture. If I'm wrong, then it should be mipmaps: (ImageData | CubeTexture)[] instead.

@mewore mewore added the enhancement New feature or request label Sep 3, 2021
@capnmidnight
Copy link
Contributor

The image field for a Texture can actually be a lot of different things. Just the raw types that WebGL can accept as a texture source alone are:

  • HTMImageElement
  • HTMLVideoElement
  • HTMLCanvasElement
  • OffscreenCanvas
  • ImageData
  • ImageBitmap

There is a type in lib.d.ts called TexImageSource that unions all of these types together.

But Three.js also has a few hacks in place where they use the image field to track the dimensions of the texture without actually storing a TexImageSource. As you've noted in your suggestion, a width/height object is one case (CompressedTexture, DepthTexture, and FramebufferTexture). But also, CubeTexture allows for an array of images, DataTexture is similar to CompressedTexture with an additional data field, Data3DTexture and DataArrayTexture are similar to DataTexture with an additional depth field,

It seems, in all cases except CubeTexture, the images field will always have at least { width: number; height: number; }. But that on its own wouldn't be too helpful and it's not clear what the most accurate way to type all of this would be.

Perhaps it is necessary to break from trying to 1-to-1 mirror the exported Texture types of Three.js and introduce a new interface, some kind of BaseTexture, that has the maximal union of all types that image can take, and then change the concrete class definitions to extend BaseTexture with more restrained types for their respective parameters.

capnmidnight added a commit to capnmidnight/three-ts-types that referenced this issue May 19, 2022
@capnmidnight
Copy link
Contributor

capnmidnight commented May 19, 2022

IDK, maybe something like this? I added some generic type parameters to Texture, that should be able to accept any of those hack cases, plus with a default setting of TexImageSource. The other subclasses of Texture then define a more meaningful value for their case.

capnmidnight@7990050

I'm not too sure about the mipmaps field, so I haven't submitted this as a PR. Maybe someone else has some insight here?

capnmidnight added a commit to capnmidnight/three-ts-types that referenced this issue May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants