marcofugaro / three-projected-material

📽 Three.js Material which lets you do Texture Projection on a 3d Model
https://marcofugaro.github.io/three-projected-material/
MIT License
674 stars 58 forks source link

"The mesh material must be a ProjectedMaterial" - why all of them? #47

Closed molnarm closed 1 year ago

molnarm commented 2 years ago

Hi,

Thanks for the awesome library!

The only thing I don't understand is why do you validate that when a mesh has multiple materials, all of them must be ProjectedMaterial? From a quick glance at the source, it looks unnecessary to me.

I'd like to have images projected on a model with existing textures. I tried commenting out that check and it still works perfectly, so I'm wondering why does it matter what other materials are there?

sapoluri commented 1 year ago

Have the same question. Why do they all need to be ProjectedMaterial?

sapoluri commented 1 year ago

@molnarm Could you please share a sample of your code to see how you projected onto a mesh with existing texture.

trusktr commented 1 year ago

Yeah, I think it should project only on the materials that are projected materials. Happy to take a PR here: https://github.com/lume/three-projected-material

(forked, converted to TypeScript, and more changes)

molnarm commented 1 year ago

@sapoluri If I had to do it again, I would use patch-package to remove the check (and if I had more time, I'd make a PR here).

But as it was just a quick POC, here's the terrible hack I did: existingMaterial.isProjectedMaterial = true; This "lies" about the existing material so the check in this library passes. It doesn't break the existing material because no other code is checking this. Another thing was to make sure that it doesn't overwrite the existing material where the photo is not projected: material.uniforms.backgroundOpacity.value = 0; I don't remember the details anymore, this is something the library already does somewhere.

marcofugaro commented 1 year ago

@molnarm good catch, it will be fixed in next release.

The check now validates that at least one of the material is a ProjectedMaterial, does that work for your usecase?

marcofugaro commented 1 year ago

Fixed in v2.2.0