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

Loading multiple textures with useTexture #176

Closed
Code47X opened this issue Nov 11, 2020 · 10 comments
Closed

Loading multiple textures with useTexture #176

Code47X opened this issue Nov 11, 2020 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@Code47X
Copy link
Contributor

Code47X commented Nov 11, 2020

I'm using the useTexture hook to load in a collection of textures. Currently I've been loading each texture individually like so, which seems to work fine:

const MetalicCube = () => {
  const colorMap = useTexture('/Textures/Metal007_2K_Color.jpg');
  const normalMap = useTexture('/Textures/Metal007_2K_Normal.jpg');
  const metalnessMap = useTexture('/Textures/Metal007_2K_Metalness.jpg');
  const roughnessMap = useTexture('Textures/Metal007_2K_Roughness.jpg');
  const displacementMap = useTexture('Textures/Metal007_2K_Displacement.jpg');

  return (<div />)
}

I've checked out the useTexture examples, and TypeScript shows it can return an array of textures, but I'm not quite following the syntax. It seems like useTexture will only accept a single string as an argument. TS won't accept an array of strings as an argument. I've tried pointing it to the folder containing the textures, but no luck.

From Example:
const [texture1, texture2] = useTexture([texture1, texture2])

Just trying to figure out if there is a better way to load all the textures, or if using useTexture individually for each is the correct way. Thanks.

@drcmda
Copy link
Member

drcmda commented Nov 11, 2020

yes it should work like that. useLoader does for sure. could you look into it and fix it? maybe the types are off.

@Code47X
Copy link
Contributor Author

Code47X commented Nov 11, 2020

Checked it out, and I believe the typing is slightly off. It looks like the conditional type would always evaluate to string. I can throw up a quick PR. This fix worked fine for me.

Current:
useTexture(url: string extends any[] ? string[] : string): Texture | Texture[];

Fixed for me:
useTexture<T extends string | string[]>(url: T): Texture | Texture[];

@joshuaellis
Copy link
Member

@drcmda when using useLoader I get the following error:

Type 'Texture' must have a '[Symbol.iterator]()' method that returns an iterator.

For this code:

 const [matcap1, matcap2] = useLoader(TextureLoader, ['matcap-1.png', 'matcap-2.png'])

Using @Code47X's TS example in useTexture get's the same error.

@Code47X
Copy link
Contributor Author

Code47X commented Jan 11, 2021

@joshuaellis Not sure if this helps you at all, but when I was messing with source code for useTexture I had to pass in a generic useLoader<Texture[]>(TextureLoader, [t1.png, t2.png]) in order to make it work with multiple textures.

@joshuaellis
Copy link
Member

Kind of helps me, I was under the impression the Types were implied through the generics, hopefully @drcmda can offer some insight.

@Code47X
Copy link
Contributor Author

Code47X commented Jan 11, 2021

I was also under the assumption that the types were implied initially. Checking out the source, it seems the return type and the input type check if the generic extends an array. It looks like when useLoader is called without a generic it defaults to a non-array input and LoaderResult. Is it designed this way on purpose, or is this a bug?

export function useLoader<T>(
  Proto: new () => LoaderResult<T>,
  input: T extends any[] ? string[] : string,
  extensions?: Extensions,
  onProgress?: (event: ProgressEvent<EventTarget>) => void
): T
type LoaderResult<T> = T extends any[] ? Loader<T[number]> : Loader<T>

@joshuaellis
Copy link
Member

The solution will be:

export function useTexture<T extends string | string[]>(url: T): T extends any[] ? Texture[] : Texture {
  return useLoader(TextureLoader, url)
}

assuming pmndrs/react-three-fiber#917 is approved. Could change depending on that PR.

@joshuaellis joshuaellis self-assigned this Jan 12, 2021
@joshuaellis joshuaellis added the bug Something isn't working label Jan 12, 2021
@Code47X
Copy link
Contributor Author

Code47X commented Jan 14, 2021

@joshuaellis I saw your pr (pmndrs/react-three-fiber#930) to fix useLoader was merged in. Nice work, I learned a lot about conditional return types and function signatures just reading through it.

Are you planning on fixing the typing for the useTexture hook yourself, or would you like me to make a pr?

@joshuaellis
Copy link
Member

@Code47X thank you, but I must honest, it was @stephencorwin who really came up with the plan with the conditionals & your suggestion too for handling the string correctly, I just put the two together.

I hope to finish #223 this evening, so keep an eye out for the issue being closed, I think the PR is nearly there tbh.

@joshuaellis
Copy link
Member

fixed in #231

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants