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

r72 : .computeTangents() has been removed #7190

Closed
RemusMar opened this issue Sep 18, 2015 · 46 comments
Closed

r72 : .computeTangents() has been removed #7190

RemusMar opened this issue Sep 18, 2015 · 46 comments
Labels

Comments

@RemusMar
Copy link
Contributor

r71: perfect
http://necromanthus.com/Test/html5/SMC.html

r72: a mess
http://necromanthus.com/Test/html5/SMC_r72.html

No other comments.

@bhouston
Copy link
Contributor

What were you using Tangents for may I ask? It isn't clear to me with a quick look. It seems like whole objects are missing frmo your scene and also that some material brightnesses are completely out of whack.

@RemusMar
Copy link
Contributor Author

What were you using Tangents for may I ask? It isn't clear to me with a quick look.

I didn't use it.
.computeTangents() is needed by the SEA3D loader.
But the main question is why it has been removed ???

@sunag
Copy link
Collaborator

sunag commented Sep 19, 2015

Hi @RemusMar

Have you tried it with SEA3D 1.7v?
https://github.com/sunag/sea3d/releases/tag/v1.7-r1

Also have the SEA3D TJS version optimized for Three.JS.
http://community.poonya.com/160
example http://threejs.org/examples/#webgl_loader_sea3d_hierarchy

Cheers.

@sunag
Copy link
Collaborator

sunag commented Sep 19, 2015

SEA3DLoader for Three.JS r72, only TJS files
https://github.com/mrdoob/three.js/tree/master/examples/js/loaders/sea3d

@bhouston
Copy link
Contributor

@sunag btw I think that the Sea3DLoader would benefit a lot if it was to load into the new Unity3D-like AnimationMixer set of classes for its animations. This PR has the code and also I've converted over most examples to use it -- it replaces Animation/AnimationHandler/etc.:

#6934

@RemusMar
Copy link
Contributor Author

SEA3DLoader for Three.JS r72, only TJS files

With that you cannot even load the SEA file.
You'll get this error:

"SEA3D 1.7.0.0" SEA3D.js
"THREE.WebGLRenderer" "72" three.min_72.js
uncaught exception: Sign 'S3D' not supported! Use SEA3D Studio to export.
TypeError: this.container is undefined SEA3DLoader.js

Another issue: the latest minified SEA3D loader does not include DEFLATE ... !?!
cheers

@RemusMar
Copy link
Contributor Author

@sunag btw I think that the Sea3DLoader would benefit a lot if it was to load into the new Unity3D-like AnimationMixer set of classes for its animations.

Ben,
SEA3D is already the best THREE loader by far.
Talking about animations, it supports animation names and interpolation (animation blending).
http://necromanthus.com/Test/html5/testA.html
Just get closer to that dog and pay attention to the smooth transitions.
cheers

@bhouston
Copy link
Contributor

@RemusMar Animation names, blending, warping is supported by the AnimationMixer/AnimationClip system - including the ability to animate material properties, skinning, node properties, and morph targets, and blend shapes. I think the new animation system can easily be used by the Sea3D loader: #6934 So I am suggesting an improvement to Sea3D, not denigrating it in any way.

@RemusMar
Copy link
Contributor Author

@RemusMar Animation names, blending, warping is supported by the AnimationMixer/AnimationClip system - including the ability to animate material properties, skinning, node properties, and morph targets, and blend shapes.

Are they included into the main package?
Or are you talking about "examples" and external scripts?

model.play('walk', 0.5);
500 miliseconds interpolation between the current animation and the "walk" one.
This is possible with SEA3D only.
cheers

@bhouston
Copy link
Contributor

Are they included into the main package?
Or are you talking about "examples" and external scripts?

The PR is for inclusion of the new animation system within the core of ThreeJS: #6934

model.play('walk', 0.5);
500 miliseconds interpolation between the current animation and the "walk" one.
This is possible with SEA3D only.

The new animation system supports cross fade between animation clips as well as warps (where it slowly adjusts the length of the cycles to match whlie doing the cross fade between the animations):

THREE.AnimationMixer.crossfade( fromAnimName, toAnimName, duration );
THREE.AnimationMixer.warp( fromAnimName, toAnimName, duration );

It was because the functionality of the new animation system matches the needs of Sea3D, I was suggesting that it also adopt it, rather than maintaining a separate but similar animation system.

@RemusMar
Copy link
Contributor Author

THREE.AnimationMixer.crossfade( fromAnimName, toAnimName, duration );

A not very inspired syntax.
Why do you need "fromAnimName"?

  • if there is no animation, you don't need it.
  • if there is a current animation, you don't need it.
    So ...

p.s.
model.play('walk', 0.5); is the best approach by far.

@bhouston
Copy link
Contributor

A not very inspired syntax.

I have a play function as well called mixer.play takes a duration for the fade in time. I'll add an option to schedule everything else to fade out at the same time -- thus it will cover your use case exactly:

https://github.com/mrdoob/three.js/pull/6934/files#diff-c21f40104b3fb390702d00a32f62cdb6R133

model.play('walk', 0.5); is the best approach by far.

I'll modify the code now to have the optional fade out of everything else using this exactly syntax.

Why do you need "fromAnimName"?

THREE.AnimationMixer is a general mixer and can support mixing together any number of animations -- such as facial animations, multiple bone animations, etc. So you can have a shoot animation for the arms bones, while fading from walk to run on the leg bones -- because it supports any number of concurrent animations, you likely do not just want to fade out all of them in favor of a new one -- thus the mixer itself needs to know which to fadeOut, especially if you are doing a warp.

There is also fadeIn, fadeOut, play, warp etc functions -- so you can full control over what happens.

Also, Sea3D doesn't have to change the UI they are providing to you as a user of the Sea3D classes, rather they could use the new animation mixer behind the scenes because it does more than their current animation system and it will reduce the amount of JavaScript required to download to run Sea3D scenes.

@mrdoob
Copy link
Owner

mrdoob commented Sep 19, 2015

This is getting off-topic...

I think the main issue is that @remoe is not happy with the new SEA3D loader.

@sunag
Copy link
Collaborator

sunag commented Sep 19, 2015

Hi

@RemusMar it is necessary to export from SEA3D Studio in File -> Publish... and choose Three.JS as output. The file turns with a TJS tag.

About the changes, my idea is to simplify the game creation process for increased productivity, for advanced users they can create their own characteristics over SEA3D. The interface of SEA3D was based on the use in our company and it has worked for several games, if migrating from Flash, Director for THREE will also be easier.

We have examples of animation Basics and Advanced here:
http://sunag.github.io/sea3d/
http://threejs.org/examples/#webgl_loader_sea3d_keyframe
crossfade, fades, timeScale, warp as offset, all this already have in sea3d animation.

Cheers

@mrdoob
Copy link
Owner

mrdoob commented Sep 19, 2015

@sunag I think @remoe just wants to export from 3D Studio...?

@sunag
Copy link
Collaborator

sunag commented Sep 19, 2015

@mrdoob @remoe = @RemusMar ?

another user requested me a TJS from the 3ds max exporter too, at the moment just from sea3d studio :( this is because has some things still pending. If all goes well until next month.

@mrdoob
Copy link
Owner

mrdoob commented Sep 19, 2015

@mrdoob @remoe = @RemusMar ?

Oops! Silly Github auto-completion.

@RemusMar
Copy link
Contributor Author

I think the main issue is that @RemusMar is not happy with the new SEA3D loader.

.computeTangents() has been removed
Why?

@RemusMar it is necessary to export from SEA3D Studio in File -> Publish...

I don't want to REexport anything.
On the internet you can find thousands of SEA files.
Are you going to REexport them all?

A tip to all the software developers:
THE BACKWARD COMPATIBIL.ITY IS A MUST.
Otherwise less and less people will be interested in your work.
cheers

p.s.
This topic is not a question.

@mrdoob
Copy link
Owner

mrdoob commented Sep 20, 2015

@RemusMar can you try adding this to your code?

THREE.BufferGeometry.prototype.computeTangents = function () {

    var index = this.index;
    var attributes = this.attributes;

    // based on http://www.terathon.com/code/tangent.html
    // (per vertex tangents)

    if ( index === null ||
         attributes.position === undefined ||
         attributes.normal === undefined ||
         attributes.uv === undefined ) {

        console.warn( 'THREE.BufferGeometry: Missing required attributes (index, position, normal or uv) in BufferGeometry.computeTangents()' );
        return;

    }

    var indices = index.array;
    var positions = attributes.position.array;
    var normals = attributes.normal.array;
    var uvs = attributes.uv.array;

    var nVertices = positions.length / 3;

    if ( attributes.tangent === undefined ) {

        this.addAttribute( 'tangent', new THREE.BufferAttribute( new Float32Array( 4 * nVertices ), 4 ) );

    }

    var tangents = attributes.tangent.array;

    var tan1 = [], tan2 = [];

    for ( var k = 0; k < nVertices; k ++ ) {

        tan1[ k ] = new THREE.Vector3();
        tan2[ k ] = new THREE.Vector3();

    }

    var vA = new THREE.Vector3(),
        vB = new THREE.Vector3(),
        vC = new THREE.Vector3(),

        uvA = new THREE.Vector2(),
        uvB = new THREE.Vector2(),
        uvC = new THREE.Vector2(),

        sdir = new THREE.Vector3(),
        tdir = new THREE.Vector3();

    function handleTriangle( a, b, c ) {

        vA.fromArray( positions, a * 3 );
        vB.fromArray( positions, b * 3 );
        vC.fromArray( positions, c * 3 );

        uvA.fromArray( uvs, a * 2 );
        uvB.fromArray( uvs, b * 2 );
        uvC.fromArray( uvs, c * 2 );

        var x1 = vB.x - vA.x;
        var x2 = vC.x - vA.x;

        var y1 = vB.y - vA.y;
        var y2 = vC.y - vA.y;

        var z1 = vB.z - vA.z;
        var z2 = vC.z - vA.z;

        var s1 = uvB.x - uvA.x;
        var s2 = uvC.x - uvA.x;

        var t1 = uvB.y - uvA.y;
        var t2 = uvC.y - uvA.y;

        var r = 1.0 / ( s1 * t2 - s2 * t1 );

        sdir.set(
            ( t2 * x1 - t1 * x2 ) * r,
            ( t2 * y1 - t1 * y2 ) * r,
            ( t2 * z1 - t1 * z2 ) * r
        );

        tdir.set(
            ( s1 * x2 - s2 * x1 ) * r,
            ( s1 * y2 - s2 * y1 ) * r,
            ( s1 * z2 - s2 * z1 ) * r
        );

        tan1[ a ].add( sdir );
        tan1[ b ].add( sdir );
        tan1[ c ].add( sdir );

        tan2[ a ].add( tdir );
        tan2[ b ].add( tdir );
        tan2[ c ].add( tdir );

    }

    var groups = this.groups;

    if ( groups.length === 0 ) {

        groups = [ {
            start: 0,
            count: indices.length
        } ];

    }

    for ( var j = 0, jl = groups.length; j < jl; ++ j ) {

        var group = groups[ j ];

        var start = group.start;
        var count = group.count;

        for ( var i = start, il = start + count; i < il; i += 3 ) {

            handleTriangle(
                indices[ i + 0 ],
                indices[ i + 1 ],
                indices[ i + 2 ]
            );

        }

    }

    var tmp = new THREE.Vector3(), tmp2 = new THREE.Vector3();
    var n = new THREE.Vector3(), n2 = new THREE.Vector3();
    var w, t, test;

    function handleVertex( v ) {

        n.fromArray( normals, v * 3 );
        n2.copy( n );

        t = tan1[ v ];

        // Gram-Schmidt orthogonalize

        tmp.copy( t );
        tmp.sub( n.multiplyScalar( n.dot( t ) ) ).normalize();

        // Calculate handedness

        tmp2.crossVectors( n2, t );
        test = tmp2.dot( tan2[ v ] );
        w = ( test < 0.0 ) ? - 1.0 : 1.0;

        tangents[ v * 4 ] = tmp.x;
        tangents[ v * 4 + 1 ] = tmp.y;
        tangents[ v * 4 + 2 ] = tmp.z;
        tangents[ v * 4 + 3 ] = w;

    }

    for ( var j = 0, jl = groups.length; j < jl; ++ j ) {

        var group = groups[ j ];

        var start = group.start;
        var count = group.count;

        for ( var i = start, il = start + count; i < il; i += 3 ) {

            handleVertex( indices[ i + 0 ] );
            handleVertex( indices[ i + 1 ] );
            handleVertex( indices[ i + 2 ] );

        }

    }

};

@mrdoob
Copy link
Owner

mrdoob commented Sep 20, 2015

As per your question... The reason computeTangents() was removed was because the built-in materials/shaders no longer required tangents.

@DVLP
Copy link
Contributor

DVLP commented Sep 22, 2015

That's also a problem which I've encountered when migrating to r72 today. Temporarily adding

THREE.BufferGeometry.prototype.computeTangents = function () {

var index = this.index;
var attributes = this.attributes; .....

to my code too until sea3d exporter for 3ds max will be compatible with r72

also adding:

THREE.Geometry.prototype.computeTangents = function () {};

just to mute the warnings.

Will it make any difference if instead of including full body of
THREE.BufferGeometry.prototype.computeTangents function I'll just mute warnings too like below?

THREE.BufferGeometry.prototype.computeTangents = function () {};

@RemusMar
Copy link
Contributor Author

@RemusMar can you try adding this to your code?

It doesn't change anything.
Less warnings but the same messed up result.
You can try it by yourself.

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2015

You can try it by yourself.

How? Do you mind zipping the test so I can investigate locally?

@RemusMar
Copy link
Contributor Author

@sunag
Copy link
Collaborator

sunag commented Sep 23, 2015

@mrdoob @RemusMar I will be investigating this problem, It's probably something in sea3d loader.

@mrdoob
Copy link
Owner

mrdoob commented Sep 23, 2015

@sunag thanks!

@sunag
Copy link
Collaborator

sunag commented Sep 23, 2015

I found the two problems he was generating this.

Lightmap moved of multiply to additive, is necessary to use aoMap for the same purpose and BufferGeometry loader also had problems. I am sending a fix.

https://fhscript.com/12jP/sea3d.min.js

Still I will clean up for a update in root.

@RemusMar Your lightmap is inverted with diffuseMap. It will no longer work in r72.

diffuse-lightmap

Cheers.

@RemusMar
Copy link
Contributor Author

I am sending a fix.
https://fhscript.com/12jP/sea3d.min.js

I didn't find any fix.

Still I will clean up for a update in root.

The same problem with the minified version: DEFLATE is not included !?!

@RemusMar Your lightmap is inverted with diffuseMap. It will no longer work in r72.

In r71 if it's not inverted, it doesn't work !!

@sunag
Copy link
Collaborator

sunag commented Sep 27, 2015

The same problem with the minified version: DEFLATE is not included !?!

The lib I sent you have deflate.

In r71 if it's not inverted, it doesn't work !!

This was because the UV1 and UV2 and textures is swapped in your original file (3ds max I suppose).

I didn't find any fix.

Did you get to test the file? I managed to fix this in three steps.

  1. Download the fix and replaces the previous sea3d.min.js.
  2. Swap your geometry "sand" UV1 to UV2. I did it in SEA3D Studio.
    swapuv
  3. Swap your diffuse to lightmap.
    swaplightmap

Result
smc

I particularly recommend the TJS version, for that is optimized for Three.JS and you will find the latest updates. At the beginning of next month I will be posting the update SEA3DLegacyLoader.js In which case it would be this version of the loader. but legacy.js is not our priority at the time.

Cheers.

@RemusMar
Copy link
Contributor Author

I am sending a fix.
https://fhscript.com/12jP/sea3d.min.js
The lib I sent you have deflate.

Again, that file cannot be loaded (it does not exist).

At the beginning of next month I will be posting the update SEA3DLegacyLoader.js In which case it would be this version of the loader. but legacy.js is not our priority at the time.

As I said before, the BACKWARD COMPATIBILITY is a must in any serious project.
You can find a bunch of SEA files over the internet.
Are you going to re-export them all from 3DS Max ???

@mrdoob
Copy link
Owner

mrdoob commented Sep 28, 2015

@RemusMar

We are the first ones to hate breaking backwards compatibility. We go great lengths to avoid that but some times we have no option than correcting bad designs and decisions. On those cases we try to add code to warn the user and if possible to keep it working still.

It's hard for us to cover all the possible use cases, so it's imposible not to break some thing. For those cases, the last thing we want is to get attacked with a demanding tone like the one you give us.

It may be a language issue and it may not be your intention to come across like that. But, please, try to be respectful to us the same way we are to you.

@RemusMar
Copy link
Contributor Author

some times we have no option than correcting bad designs and decisions.

I totally agree with that.
But correcting the bad design and breaking the backward compatibility are two different things.
cheers

@sunag
Copy link
Collaborator

sunag commented Oct 8, 2015

Hi, I finished the loader update now hybrid, it works both for the TJS as to the Legacy.

@RemusMar @DVLP
https://github.com/sunag/sea3d/blob/gh-pages/Build/Three.JS/sea3d.min.js

Compiler and files included.
https://github.com/sunag/sea3d/blob/gh-pages/Compiler/build-three.js.bat

hello world threejs-r72 (legacy)
http://sunag.github.io/sea3d/Examples/Programmer/WebGL/Three.JS/Basics/hello-world.html

Cheers.

@RemusMar
Copy link
Contributor Author

RemusMar commented Oct 8, 2015

Hi, I finished the loader update now hybrid, it works both for the TJS as to the Legacy.
https://github.com/sunag/sea3d/blob/gh-pages/Build/Three.JS/sea3d.min.js

I see some improvements: all the models are loaded and I don't get errors.
But there are still serious issues:

  1. the textures with alpha channel (transparency) do not work
  2. I have to rotate all the models with 180 degrees on the X axis.
  3. the LightMap legacy (inverted Diffuse/LightMap) does not work

r71: perfect
http://necromanthus.com/Test/html5/SMC.html

r72: issues
http://necromanthus.com/Test/html5/SMC_r72.html

cheers

@sunag
Copy link
Collaborator

sunag commented Oct 9, 2015

@RemusMar thanks for tests, the 1 and 2 items apparently they have been fixed.

@mrdoob aoMap only works with Ambient Light?
https://github.com/mrdoob/three.js/blob/master/src/renderers/shaders/ShaderChunk/aomap_fragment.glsl

Cheers.

@WestLangley
Copy link
Collaborator

@sunag An ambient occlusion map occludes only ambient light sources. Hence its name.

Another term for ambient light is indirect light.

There are currently three models of indirect, diffuse light: AmbientLight, LightMap, and HemisphereLight.

The AO map occludes all three.

@sunag
Copy link
Collaborator

sunag commented Oct 9, 2015

Thaks @WestLangley! But if I want to give the same effect of a shadowmap it is possible? Also because in my design AO is a soft shadow, looking by physics

@RemusMar
Copy link
Contributor Author

RemusMar commented Oct 9, 2015

@RemusMar thanks for tests, the 1 and 2 items apparently they have been fixed.

That's right.
r71: http://necromanthus.com/Test/html5/SMC.html
r72: http://necromanthus.com/Test/html5/SMC_r72.html
But you should also fix the LightMap legacy (inverted Diffuse/LightMap).
Why?
Because there are hundreds or thousands of SEA files over the net.
And most of them won't be re-exported from 3DS Max.

Anyway, I found a new bug: animation related
r71: http://necromanthus.com/Test/html5/dog.html
r72: http://necromanthus.com/Test/html5/dog_r72.html
Please investigate and fix it.
cheers

@RemusMar
Copy link
Contributor Author

RemusMar commented Oct 9, 2015

sunag/sea3d@4d4eb0f

That bug was fixed, but there is another one: multiple animations related.
r71: http://necromanthus.com/Test/html5/dog.html
r72: http://necromanthus.com/Test/html5/dog_r72.html

Press key 3, after a few seconds press key 1, after a few seconds press key 2.
Everything runs perfect in r71
In r72 ALL the motions are mixed and that's wrong.
Please fix it.
cheers

@RemusMar
Copy link
Contributor Author

RemusMar commented Oct 9, 2015

Thaks @WestLangley! But if I want to give the same effect of a shadowmap it is possible?
Also because in my design AO is a soft shadow, looking by physics

See the "aoMapIntensity" value (1 by default)
For example with "loader.materials[0].aoMapIntensity = 3;" I was FINALLY able to get the same result with r71 and r72:
r71: http://necromanthus.com/Test/html5/SMC.html
r72: http://necromanthus.com/Test/html5/SMC_r72.html

cheers

@sunag
Copy link
Collaborator

sunag commented Oct 9, 2015

Hi @RemusMar

Because there are hundreds or thousands of SEA files over the net...

I'll be updating the Legacy to the next versions.

For example with "loader.materials[0].aoMapIntensity = 3;" I was FINALLY able to get the same result with r71 and r72:

The problem with this technique that just worked if there Indirect Light (in case Ambient Light). I made a change in native shader to AO be a Shadow Map for THREE.SEA3D.StandardMaterial.

That bug was fixed, but there is another one: multiple animations related.
That bug was fixed, but there is another one: multiple animations related.
r71: http://necromanthus.com/Test/html5/dog.html
r72: http://necromanthus.com/Test/html5/dog_r72.html

Try use the SEA3D AnimationHandler in updater:

var delta = clock.getDelta();
THREE.SEA3D.AnimationHandler.update( delta );
THREE.AnimationHandler.update( delta );

Cheers.

@RemusMar
Copy link
Contributor Author

RemusMar commented Oct 9, 2015

I'll be updating the Legacy to the next versions.

Don't forget about that.
As I said from the very beginning, the backward compatibility is a MUST in any serious project.

Try use the SEA3D AnimationHandler in updater:

var delta = clock.getDelta();
THREE.SEA3D.AnimationHandler.update( delta );
THREE.AnimationHandler.update( delta );

That did the trick
have a great weekend

@mrdoob
Copy link
Owner

mrdoob commented Oct 9, 2015

Many thanks guys!

@mrdoob mrdoob closed this as completed Oct 9, 2015
@pailhead
Copy link
Contributor

pailhead commented Dec 2, 2015

Hi,

I'm not sure if i should open up another issue, but i missed this one and there is a ton of stuff here about some specific loader.

Why was this removed? Is the idea to rely on derivatives and fragment shaders always? If so, i think this will more or less be useful for procedural surface like textures (orange skin, brick wall etc.) very not useful for any kind of a bake (maya, max, xnormal etc.).

@mrdoob
Copy link
Owner

mrdoob commented Dec 2, 2015

If so, i think this will more or less be useful for procedural surface like textures (orange skin, brick wall etc.) very not useful for any kind of a bake (maya, max, xnormal etc.).

How so?

@WestLangley
Copy link
Collaborator

@pailhead Please see #7094. Perhaps you can continue the discussion there, instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants