mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
101.87k stars 35.32k forks source link

UV offset / repeat should be part of materials rather than textures #5876

Closed QuaziKb closed 3 years ago

QuaziKb commented 9 years ago

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.

sunag commented 8 years ago

Example:

var uniqueTextureOffset = map.createInstance();
var material = new THREE.SpriteMaterial( { map: uniqueTextureOffset } );

Maintain the same uuid and version will share the texture on GPU.

https://github.com/mrdoob/three.js/blob/master/src/renderers/WebGLRenderer.js#L2832 https://github.com/mrdoob/three.js/blob/master/src/renderers/webgl/WebGLProperties.js#L11

sunag commented 8 years ago

Although for me it's a provisional solution. I believe in NodeMaterial for the future.

titansoftime commented 8 years ago

Can we please move uv transforms to material?

bhouston commented 8 years ago

How about best of both worlds. Add a flag overrideOffsetRepeat on material with a new uvOffset and uvRepeat on the material. This way if the flag is false, it is backwards compatible, and that is the default. And if it is true, it uses the material offset/repeat. I would support this as it seems the need is intense and widespread. @WestLangley? @mrdoob?

(Although I really do support using NodeMaterial for everything going forward, but it is a pain to switch to it.)

mrdoob commented 8 years ago

I still think the solution for this is to create THREE.Image. https://github.com/mrdoob/three.js/issues/5876#issuecomment-81189892

rhys-vdw commented 8 years ago

THREE.Image

@mrdoob, would that mean that each assigned Texture would be cloned along with a Material?

bhouston commented 8 years ago

@mrdoob wrote:

I still think the solution for this is to create THREE.Image

Yes that would work, it would be a little bit less backwards compatible though (if we now require everyone to create Images before creating Texture), but a cleaner overall design. Maybe it is possible to have an Image created automatically per Texture if one didn't specify it separately, then that would achieve backwards compatibility? And you would only need to manipulate the Image directly if you wanted to do something tricky.

I guess this solution would require twice the new objects for a set of sprites - one Texture for each sprite and one Material? Where is the other approach would just require a new Material per each sprite?

I know that Unity supports repeat/offset per Material, but high end VFX tools like 3DS Max, Maya, Softimage do not -- they just have a Bitmap node and a Texture node (which contains both the Bitmap node as well as a UV Mapping nod), which is similar to @mrdoob's design.

UE4 is also similar to what @mrdoob is proposing, they have a "Texture" node which is a bitmap loader, and then they have a series of UV Mapping nodes to do various types of UV mappings.

The advanced tools do split up Image/Bitmap from the Texture and they also split out a separate UV mapping node, in this form:

Texture
 -> Bitmap
 -> UVMapping

We do not allow for much UVMapping options in the main StandardMaterial right now. But various types of UVMappings used in UE4, Maya, Softimage, 3DS Max, would be which channel to use, a transform to apply to it, to use World Coordinates as the source, or if one should do a Spherical, Cube, Planar projection based on the required parameters for those projections.

bhouston commented 8 years ago

UE4 has a sprite texture that allows for repeat/offset within the material:

https://docs.unrealengine.com/latest/INT/Engine/Paper2D/Sprites/index.html

bhouston commented 8 years ago

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.

mrdoob commented 8 years ago

Maybe it is possible to have an Image created automatically per Texture if one didn't specify it separately, then that would achieve backwards compatibility? And you would only need to manipulate the Image directly if you wanted to do something tricky.

Exactly 😊

MasterJames commented 8 years ago

Probably a bit out of place to mention but I would like to recommend PTEX be incorporated as soon as possible. http://ptex.us/PtexFile.html If there's a way to make typical projections etc cast/convert to a NodeTexture (?) Or option that is a new more powerful and comprehensive base level texture mapping system? ...well then maybe that's something to consider ahead of time. [Further to the same point:] The concept with ptex is its not really a 2d image but a uv relationship so you can paint/stamp/project around a complex surface without the concept/challange of how to translate 2d to 3d, which is mathimaticaly a hack at best in comparison (always technically battling with destortion). I just point out that ptex should have made more sense and been a priority 20+ years ago and should not to be considered an extension or second class performer, but really for me it's the other way around. It should be the old original way of declaritively saying how a 2d image is projected/stamped into the one true always functioning base level system of ptex. Anyway just enforcing the idea it should be integrated if not best to take a more central roll. Thanks for the opportunity to make lofty suggestions. I was so happy that the ptex was made to a spec. I thought of it myself 10+ year prior but as a child had no power to define new specs etc. In hindsight I should have seen that maybe I could have tried to make a difference instead of the roll of observer I still maintain. So here is an attempt to undo a long standing wrong.

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. Again thanks for the opportunity to be heard.

mrdoob commented 8 years ago

@MasterJames kind of off-topic... create a new thread please?

QuaziKb commented 8 years ago

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.

mrdoob commented 8 years ago

@QuaziKb Yep. That's what NodeMaterial tackles.

evrimoztamur commented 8 years ago

Isn't it the case that Texture uses the same OpenGL texture instance for each .clone(), or am I missing something. Is it actually reuploading it for each clone? If that's the case, then this is a very serious issue.

rhys-vdw commented 8 years ago

@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.

karimbeyrouti commented 8 years ago

@rhys-vdw: For a current project I ended up with a bastardized version of MeshBasicMaterial: https://gist.github.com/karimbeyrouti/790d2e1a8c0137b16bae

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)

bhouston commented 8 years ago

I've just submitted a PR that should address this issue, PR #8278

EliasHasle commented 7 years ago

@WestLangley wrote:

I believe it is reasonable to assume the following maps have the same offset/repeat values:

map specularMap normalMap bumpMap alphaMap aoMap (future) glossMap (future) displacementMap (future)

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.

sunag commented 7 years ago

I think this solves the problem of instances with different UV in Sprite. It is possible modify the vertex and pixel shader preserving the same texture.

https://threejs.org/examples/#webgl_sprites_nodes

It use SpriteNodeMaterial and Mesh with a shared PlaneBufferGeometry. The interface is not appropriate to Sprite but work. Maybe it can evolve to SpriteMesh to make an interface more friendly

makc commented 7 years ago

this thread is TL;DR for me. but I have just now discovered this code in r68 (old project, yep):


        // uv repeat and offset setting priorities
        //  1. color map
        //  2. specular map
        //  3. normal map
        //  4. bump map
        //  5. alpha map

        var uvScaleMap;

        if ( material.map ) {

            uvScaleMap = material.map;

        } else if ( material.specularMap ) {

            uvScaleMap = material.specularMap;

        } else if ( material.normalMap ) {

            uvScaleMap = material.normalMap;

        } else if ( material.bumpMap ) {

            uvScaleMap = material.bumpMap;

        } else if ( material.alphaMap ) {

            uvScaleMap = material.alphaMap;

        }

        if ( uvScaleMap !== undefined ) {

            var offset = uvScaleMap.offset;
            var repeat = uvScaleMap.repeat;

            uniforms.offsetRepeat.value.set( offset.x, offset.y, repeat.x, repeat.y );

        }

so, yeah...

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

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?

gigablox commented 6 years ago

@sunag For PR: https://github.com/mrdoob/three.js/pull/11531

One issue I ran into: https://jsfiddle.net/f0j2v3s8/

It seems like SpriteNodeMaterial transparency is lost, there is no combination of blending I can use on to get this example to work.

Any ideas?

jimtang commented 6 years ago

@karimbeyrouti wrote:

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 believe this is obsolete since uniforms.offsetRepeat is changed to uniforms.uvTransform (r88).

Regarding the 'reuse a texture atlas with multiple Object3D instances' use case, I suggest a simple walk around:

  1. store the UVs data (offset, repeat) in an atlas json object;
  2. hook the onBeforeRender\onAfterRender function pair of Object3D;
  3. in the before render callback, load the UVs data from atlas json object and set to material.map;
  4. in the after render callback, reset it back;

It will result in:

  1. only one Texture & one Material shared by multiple objects, no Clone is needed and the info.memory.textures counter will not increase;
  2. but still all the other maps(normal, ao, displacement...) must comply with the same UV translation;
Mugen87 commented 3 years ago

The topics in this issue are a bit mixed and already separately discussed in other issues. Merging this issue into two others.

The leading issue for uv channels/transform per texture #9457.

The leading issue for THREE.Image is #17766.

QuickMick commented 3 years ago

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?

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?

if(material.customUVTransform){
  uvScaleMap = material.customUVTransform;
}else ...

this is my current work around using the onBeforeCompile

onBeforeCompile(shader){
  shader.uniforms.uvScale = { type: "v2", value: new THREE.Vector2(x,y) };

    shader.vertexShader = shader.vertexShader.replace(
    "#include <clipping_planes_pars_vertex>",
    `#include <clipping_planes_pars_vertex>
     uniform vec2 uvScale;
    `
    );

    shader.vertexShader = shader.vertexShader.replace(
    "#include <uv_vertex>",
    `#include <uv_vertex>
      #ifdef USE_UV
        vUv = ( uvTransform * vec3( uv, 1 ) ).xy * uvScale;
      #endif`
    );
}