-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
UV offset / repeat should be part of materials rather than textures #5876
Comments
Related post: #3549
In what sense are tons of memory wasted?
What do you mean by that? If we keep the current approach, one thing that needs to be changed, is when a texture is cloned, the shared imaged should only be passed to the GPU once. |
if you have an object with a large number of frames/sheets, there's many useless objects that are created, and as you stated, the Image is copied multiple times onto the GPU which is extremely wasteful. Even more wasteful, if you have multiple objects which need to be at different points in different animations, you'll have to create a unique set of textures per unique material/object, so it quickly becomes annoying to handle, where a material uniform could much more easily be manipulated without having these extra texture offset/repeat data-structures built around a badly thought out API just to get it working. I have major slowdown in a certain use case of my application where i'm having to create these unique texture groups everywhere, and feel like the only solution would be to replace all the stock materials i use with shadermaterials to work around this single issue, or else fork my THREE.js version and make the necessary modifications. In regards to the second statement: I meant that the current paradigm only facilitates work in cases where you want to apply the same texture with the same UV offset/repeat, but thats generally a rare case, since it seems much more likely that you'd want to define this like other uniforms, per material, and share a material rather than just the texture. The main caveat with the current sys. is that the offset/repeat only really exists as a single uniform which affects all textures in the shader, yet the fact it's attached to THREE.Texture means it can't be used properly as a uniform and fools people into thinking that offsets/repeats can be chosen differently for the various textures which can be applied on stock materials (i.e. you can define a different diffuse offset and normal map offset, but this isn't actually possible even though they can be set uniquely on the different textures). I can see it might have been remnant from the canvas renderer but it just makes no sense with the webGLrender / GLSL material system, which has largely usurped it. mrdoob stated that it would need to be of the following form as a reason against having it per material, material.map but again, the offset / repeat don't work per-map-type even now, so this isn't really a valid argument. It would waste too many uniforms anyway to have it per-map-type, (not to mention you usually only use one set of UV's so you wouldn't really have multiple offsets for a normalmap / bumpmap / diffusemap / specularmap). I think if you weigh all the options reasonable, it really should be a material property rather than texture property. I really don't feel there are any advantages to the current system. I feel animating materials through the UV offset is a major reason to even have the property at all, and can't even properly used for that. Not to mention if you didn't need those uniforms they could be omitted from the shader compilation, whereas now they're there as long as a map is. |
@QuaziKb With all due respect, if you are going to propose a fundamental change to the library, such as the one you have proposed here, you have to provide a clear and compelling argument for that change. Arguing from the frame of reference of your particular application is not going to cut it. That being said, I too, would like to see offset/repeat moved from I believe it is reasonable to assume the following maps have the same offset/repeat values:
The single exception is
This is the way it is implemented now; even though each map has its own offset/repeat values, they are not honored. So, we could add to
We would remove In this way, implementing a sprite sheet is straight-forward. Each sprite has a material, and those materials share a single texture. The texture has an image. The texture no longer has to be cloned. On the GPU side, the materials would share a single shader program. IMHO, this is a more-natural API. I am trying to recall why the API was implemented the way it was. I expect there was a good reason. |
Sorry didn't mean to suggest it was particular to my example, simply that the current api leads to significant bloat when used for animating sprite sheets / offsets per unique object/material, with no real solution. Naturally the main purpose of having this as a uniform rather than simply baking the offset/repeat into the vertex UVs of the model is to animate the offset, and i was suggesting this can't be done without going around the API, making the uniform much less useful attached to textures than if it were to the material. |
FWIW, this behaviour (the current implementation) is causing significant frustration for me at this very moment. I am attempting to animate the reveal of a mesh by tweening the mesh material's alphaMap offset independently from it's diffuse map (which stays fixed). After some frustration I discovered via #4987 and this bug that this is not possible. |
@jcarpenter What is the "bug" you are referring to? What is your suggestion for improvement? |
Correction: by "bug" I meant this issue. A mixup due to excessive time in a Bugzilla culture. :p I do understand that this is not a bug, but rather the intended behavior. WRT to an improvement, based on my experience with traditional 3D content creation apps like Cinema 4D, I imagine the user being able to both:
Alternatively for the use case I'm working on ("attempting to animate the reveal of a mesh"), it would be fantastic to have an option for wrapS and wrapT that does not wrap at all. Per the attached GIF from Cinema4D, which has an option to disable tiling entirely for UVW mapping. Based on fairly extensive testing with the existing wrapS and wrapT methods, nothing like this is possible. Meaning my options for animate the reveal of items in three.js seems limited to tweening position and opacity of the entire mesh... Unless I'm missing something. |
Agreed. The plan is to simplificate all this (using a single matrix3 in the shader) and having a offset/repeat (or translate/scale) per texture. Anyone that wants to help with this would be much appreciated! |
sadly the only way to do this right now is with multiple meshes w/ different materials, or a custom shadermaterial. Both of which are kind of involved for a user who's just jumping into three.js workflow. There's a constant tradeoff between usability, performance and extensibility. My suggestion would be to rewrite the way textures and materials work at present in the api, so that textures are strictly parameters you plug in, and the values that are actually uniforms in the shader like offset/repeat/wrap mode are linked specifically to the material. In some cases you don't want a uniform wasted on controlling UVs for textures that never need them to change (it'd be a huge waste to have 4*5 extra uniforms in a phong material if you only need it for diffuse), so it'd be cool if there was some magic behind the scenes that detected if specific UV's are needed and the texture is rebuilt to meet those demands, or if some parameter could optionally be passed in to specify the number of required adjustable UV's & what maps they offset & how, but it's a difficult issue to resolve. |
|
This would be a much appreciated change. Having to clone textures just to put them at unique repeat values feels awfully cumbersome. Is this how other libraries do it? |
I think we should have a
Rotation yes. Center or not... I'm not sure, but as far as I understand, all this can be encoded in a single |
Here is a prototype showing how a If Comments welcome. EDIT: demo updated |
THIS IS GREAT! 👍 👍 I think I would go with |
What, exactly, should the new material properties be? There are a lot of material maps. What should be removed from the texture properties? I am assuming that |
I think I think the new properties should be... |
Referring to the title of this post, and to the arguments in #5876 (comment), I thought the point was to move these properties to the material. |
/ping @bhouston for comments. |
My thoughts are that the texture offset/repeat should be baked into the UVs as much as possible. It is easier. This is how UE4/Unity 5 do it for the most part. The exception is that Unity 5 does have the ability to specify one global offset/repeat per material that is shared across all textures though -- but it doesn't affect what are considered to be baked maps such as the lightMap or ambientOcclusion maps (those do not make sense to be adjusted.) The reason why I do not advocate for a lot of flexibility here is that professionally created models have proper UVs that make this generally not necessary -- content creation tools have ways of baking this. The other issue is that WebGL has a ridiculously low lower limit for Fragment Uniforms -- 16 Vec4. If you have to have a repeat/offset per map, and we will get a lot of maps soon, we are wasting Fragment Uniforms for very little value. I actually added a repeat/offset and then brightness/gain controls per texture in Clara.io recently and I will be undoing these changes because it is leading to overflowing the Fragment Uniforms on low end devices -- such as every Apple iOS device. Although having repeat/offset and brightness/gain works great on desktops with NVIDIA 980s, but we have to design for everyone, not the high end machines. ;) We have to treat Fragment Uniforms with respect and only use them when necessary. This I believe isn't one of those cases. |
This is basically what three.js is doing now -- although it gets the offset/repeat from the diffuse map, and all other material textures use the setting from that map. The proposal is to remove the offset/repeat from the texture, and add it to the material instead -- with perhaps a name change. Again, all the material textures would share the same settings (even though some users would not be happy about it). This would keep uniform usage low. Unfortunately, we are not getting a lot of consensus on this. |
Doing what Unity 5 is doing is a good idea. I would put it on the material instead of the texture as well. You have my support. |
Per map isn't really ideal, but it could be solved by having it specified with flags/keys somehow when the materials built, in case it is needed. |
Generally the only reason to modify UVs in the shader is because you want them animated. If you just want to transform the UVs statically, we shouldn't be modifying the shader to add this functionality. |
The only proposed change to the shader is to replace
with
|
Ok. How about this... We add vUV = uv; If the user sets vUv = ( uvTransform * vec3( uv, 1 ) ).xy; Of course, if That way we get the best of both worlds? |
|
I disagree with this thread. I don't think we should pollute the materials with
We kind of need something like that. Say that one reuses the same texture in different maps. We don't want to be computing the |
Would a texture still need to be cloned to have different repeat properties? In small scenes this is probably no big deal, but in large ones where there already are 40+ textures this a memory nightmare. |
UE4 has a sprite texture that allows for repeat/offset within the material: https://docs.unrealengine.com/latest/INT/Engine/Paper2D/Sprites/index.html |
Maya, Softimage, 3DS Max, and UE4 have the separation of Bitmap/Image from Texture (and from the UV Generation) as @mrdoob is suggesting, but they also all use shader graphs to achieve this. Unity3D, which does not have shader graphs is the tool that incorporates the offset/repeat into the material itself, probably because it can not properly separate it into a shader graph node. |
Exactly 😊 |
Probably a bit out of place to mention but I would like to recommend PTEX be incorporated as soon as possible. Maybe someone can start a new thread if anyone with a deeper understanding of the current methods potentially in Flux can make a more applicable proposal of how that would work in THREEjs. |
@MasterJames kind of off-topic... create a new thread please? |
Even if we had image so that "texture" could still be used, all the materials would need to be rewritten, since they dont actually support more than one uv offset/repeat. Obviously this could change, but would probably end up adding complexity since then redundant uniforms would be needed (how often do you want more than one set of offsets/repeats so that e.g. A normal map is offset from a diffuse) i think in the end for web where performance is premium the most common use case is one where there is one global offset/repeat which affects every map, and it just make sense for this to be on the material since it ends up being part of the materials architecture. Custom shader materials can handle edge cases just fine. |
@QuaziKb Yep. That's what |
Isn't it the case that |
@evrimoztamur, I believe it copies the texture each time. You can check out what's going on using WebGL Inspector. I tried tried every approach I could think of, including @sunag's workaround, but nothing worked. Based on my experimentation not yielding results, and the discussion above, I had a look around for how other other libraries handle sprite animation. I found Babylon.js's Sprite and SpriteManager API to be a solution that caters to my particular needs. It handles sprite sheets, offset and repeat, and animations. Maybe this is a higher level of abstraction than THREE.js aims to provide, but might be worth a look as a reference. |
@rhys-vdw: For a current project I ended up with a bastardized version of MeshBasicMaterial: When you set the Map, this automatically assigns the offset / repeat uniforms (which are in tucked away material ). You can easily set them separately - that will stop you needing to clone textures for now. ( working with r73) |
I've just submitted a PR that should address this issue, PR #8278 |
@WestLangley wrote:
Not always. I am currently using different repeat values for a map and a bumpmap on the same material (asphalt) to conceal the fact that both are tiled with rather small tiles. That way, I don't need to generate/have a big texture. It is very convenient. :-) EDIT: Well, that's at least what I thought I had done. The trick was probably ignored. And I can achieve similar results by adding noise in the shader or something. The WestLangley's Matrix3 demo is cool. |
I think this solves the problem of instances with different UV in Sprite. It is possible modify the https://threejs.org/examples/#webgl_sprites_nodes It use |
this thread is TL;DR for me. but I have just now discovered this code in r68 (old project, yep):
so, yeah...
I was trying to set different repeat on diffuse and normal maps and pulling my hair out because it did not work. since API places this setting in texture, I thought I could do that. so yes, how about saving my hair for compelling argument? this ticket is still open, 3js still has this setting on textures, and it is only respected for one of the textures = this is, in effect, already IS per-mateterial setting. If you are not going to move this where it belongs, at least say it has no effect in the docs? |
One issue I ran into: It seems like Any ideas? |
@karimbeyrouti wrote:
I believe this is obsolete since Regarding the 'reuse a texture atlas with multiple Object3D instances' use case, I suggest a simple walk around:
It will result in:
|
exactly the same happened to me today :D am i blind, or is this still missing? it would just require e.g. following lines in front of the if/else chain, isn't it?
this is my current work around using the onBeforeCompile
|
UV offset and repeat (and probably some of the other texture properties) are uniforms which are strangely passed from the texture into the material, which leads to issues where you have to clone textures per material (wasting a ton of memory) if you want per-material offsets/repeats, or roll your own custom shader. It's very inefficient to force people to do this, the best case being if someone wants to tile a texture across a surface based on the size of the object it's applied to, or use a texture as a spritesheet for animations. I can only see the current system as a hindrance with no real advantages, since the likelihood of needing shared UV offsets/Repeats on a "shared" texture is low, and would normally be better served by sharing a material. In anycase updating the uniforms off the texture is weird, since it really has no business being part of the texture, and ends up being confusing to the end user. For example, changing the offset/repeat when more than one map is applied to the material, example diffuse+normalmap, only uses the offset/repeat of the diffuse map, so the value of the normalmaps offset/repeat is completely useless in that context, when it really suggests it should affect the normal map offset/repeat. It's all around confusing.
I'm pretty sure the change is required in the "refreshUniformsCommon" function, but there's probably more to it. There's some changes required in the sprite plugin aswell. This would probably break a lot of peoples projects but it's a pretty big inconsistency in the texture/material API. It might be a better idea to make it an override that's normally null for materials, and when set ignores the values in the textures, just so it doesn't break everyones stuff.
The text was updated successfully, but these errors were encountered: