mrdoob / three.js

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

Need inverse(modelMatrix) uniform in shaders #11162

Closed jcowles closed 7 years ago

jcowles commented 7 years ago
Description of the problem

Currently, there is no way to expose the inverse of the modelMatrix to a shader, which means you must write an inverse function in the shader, which is slow on older devices.

It would seem there is no way to expose this without hacking the Three.js source.

For reference, Unity provides this matrix as a built-in value: https://docs.unity3d.com/Manual/SL-UnityShaderVariables.html

Three.js version
Browser
OS
Hardware Requirements (graphics card, VR Device, ...)

N/A

pailhead commented 7 years ago

Gotta advertise this :)

https://github.com/mrdoob/three.js/pull/10791

You can check out my branch, and basically do this:

myMaterial.shaderUniforms = { uModelMatrixInverse: { value: new THREE.Matrix4() } }
myMaterial.shaderIncludes = { chunkWhereYouNeedYourLogic: GLSL_code }

edit

Currently, there is no way to expose the inverse of the modelMatrix to a shader

This is not true, you can provide whatever you want to ShaderMaterial.

WestLangley commented 7 years ago

which means you must write an inverse function in the shader

Not necessarily. See the section Transforming Points with the Inverse Matrix in this link.

mi7flat5 commented 7 years ago

GLSL has a built in inverse function. https://www.khronos.org/registry/OpenGL-Refpages/gl4/html/inverse.xhtml

pailhead commented 7 years ago

Glsl for OpenGL 4?

jcowles commented 7 years ago

@pailhead - When you say "not so, you can provide whatever you want", how would you do this efficiently per object? Note that I'm asking for inverse(modelViewMatrix), that means it potentially changes per mesh or am I missing something?

@WestLangley - From the article you linked: "In most cases, the best solution is to define another uniform variable for the inverse matrix and set the inverse matrix in the main application. The shader can then apply the inverse matrix like any other matrix. This is by far more efficient than computing the inverse in the shader."

@mi7flat5 - No, there is no inverse() function in glsl for WebGL 1.0 and you wouldn't want to encourage people to use it if there were.

pailhead commented 7 years ago

If you don't want to compute the inverse in the shader where do you intend to compute it?

myShaderMaterial.uniforms.uMyMatrixInverse.value.ANY_MAT_METHOD()

Or am I missing something?

mi7flat5 commented 7 years ago

@jcowles on the first point I stand corrected, but depending on needs of application inverse may be preferred in the shader, though probably only in the vertex shader.

jcowles commented 7 years ago

@pailhead - I want to compute it on the CPU and send it to the shader as a uniform

@mi7flat5 - fair, but I would say the common case is to not want to compute it

pailhead commented 7 years ago

Shouldn't be more than this.

let myMatrix = new THREE.Matrix4()

let myShader = new THREE.ShaderMaterial({
  uniforms:{ 
    uMyMatrix: { value: myMatrix } 
  },
  ...
})

let myOtherMatrix = new THREE.Matrix4()

function someFunc(){
  doStuffWithMatrix( myOtherMatrix )
  myMatrix.getInverse( myOtherMatrix ) //automagically update the uniform in shader
}

uniform mat4 uMyMatrix;

void main(){
...
}
jcowles commented 7 years ago

@pailhead I'm having a hard time understanding how what you propose maps on to my request. Are you proposing a custom material for every mesh?

jcowles commented 7 years ago

... and iterating over every mesh on every frame to set the uniform?

pailhead commented 7 years ago

Well, i'm having a hard time understanding this issue. If you are already writing your own shaders, i don't understand what's the problem. You pass a uniform just like any other uniform. If it's a matrix4 then you can do whatever you want with that matrix ( take a matrix, transpose it, inverse it, multiply it etc. etc.).

If you are not writing your own shaders (not sure what you refer to as "custom material") then i'm super confused. There is no way to inject glsl into the existing (non-custom??) materials. I've made a pull request that should allow this though.

... and iterating over every mesh on every frame to set the uniform?

Dont know of any other way to set uniforms.

jcowles commented 7 years ago

I want the inverse model matrix -- that's a unique matrix for every mesh, even if they have the same shader.

Imagine I have two meshes, both are using the same diffuse shader. Can one material have a single uniform ("inverseModelMatrix") with two different values for that matrix?

pailhead commented 7 years ago

No, it will have one value. Can you share some code, I'm really curious where this is coming from. This may be a stack overflow issue

jcowles commented 7 years ago

By "where this is coming from," do you mean, "why would I want an inverse model matrix?" -- I need to do some calculations in view space and take the result back to untransformed object space. So I take some value, multiply by modelViewMatrix, compute something, and then take the result back to object space by multiplying by inverse(modelViewMatrix).

Sometimes you can work around this changing the order of how the shader works, but in some cases that's not a viable option.

jcowles commented 7 years ago

Why do you think Unity provides this as a built-in value?

Because it's inefficient to compute in the shader and the renderer is the most efficient point at which it can be computed (and cached) and assigned as a uniform on the material and doesn't require creating a separate material instance for every object in the scene, just to get an inverse model matrix.

WestLangley commented 7 years ago

@jcowles

@WestLangley - From the article you linked: "In most cases, the best solution is to define another uniform variable for the inverse matrix and set the inverse matrix in the main application. The shader can then apply the inverse matrix like any other matrix. This is by far more efficient than computing the inverse in the shader."

  1. It is not more efficient if that inverse matrix is not needed -- which it hasn't been, so far.

  2. You have said you want inverse( modelMatrix ), and then you said you want inverse( modelViewMatrix ) for your ShaderMaterial. Which one is it?

  3. Can you explain why the technique I linked to is not satisfactory for your use case?

  4. Can you try using this pattern?

    
    var myFunction = function( renderer, scene, camera, geometry, material, group ) {
    
    // update your material uniform on a per-instance basis here

}

mesh.onBeforeRender = myFunction;


5. If that is not acceptable, can you provide a live demo of your use case?
pailhead commented 7 years ago

By "where this is coming from," do you mean, "why would I want an inverse model matrix?" -- I need to do some calculations in view space and take the result back to untransformed object space.

So it's safe to assume that you are working with ShaderMaterial, and you are referring to the built in variables (attributes, uniforms)? Not all shaders need this inverse, but most shaders do need things like projectionMatrix, modelViewMatrix.

and doesn't require creating a separate material instance for every object in the scene, just to get an inverse model matrix.

You might be mixing up shaders and materials. Materials are just a wrapper, an abstraction that helps you define abstract concepts such as "color". Three uses materials to manage the drawcalls. You can avoid keeping multiple material instances by using #4 from what @WestLangley suggested. This approach is much closer to how WebGL actually works. You don't have to compute the inverse every frame, but you should have some check to decide when to update. If you disable sorting, the objects will render in the order they were added. Changing the uniform should incur less overhead if the same shader is sequenced.

If this doesn't make sense please provide some code :)

arose commented 7 years ago

See #9870 for an example on how to use onBeforeRender to update uniforms.

jcowles commented 7 years ago

I argue that encouraging users to implement this on their own via onBeforeRender is the wrong approach for the following reasons:

1) it adds performance overhead in an inner loop of the renderer.

2) it forfeits the ability to do something smarter, such as caching the matrices instead of recomputing the inverse for every object on every frame.

3) the renderer owns the modelView matrix and I have demonstrated in #11240 there is a simple way to expose this functionally per-material -- forcing the user to implement the inverse is both inconsistent and inconvenient.

What are the counter arguments?

pailhead commented 7 years ago
  1. I'm not sure how your solution is supposed to be more performant....

  2. Care to elaborate? What's preventing you from caching your own matrices, or generally be smarter? Where is the caching happening in your pull request? Why is there a Matrix4 object being created seemingly every frame (or am i misinterpreting this), if im not mistaken this is an explicit anti-pattern with three.js, if anything should be cached it should be that.

  3. Are you worried about this? You want to do an inverse of something that happens automagically under the hood? So you want to avoid computing the matrix one more time in order to get the inverse? This could be an issue, but i think a more three like approach would be to set a X_needsUpdate or autoUpdate so that you're in control of how many times it gets computed. I might be completely missing the point.

  4. Tone, style of writing, something is off here. I see that people are having trouble understanding what you want to do, it's ambiguous. I used a crystal ball to guess that you are working with ShaderMaterials but am still not 100% on that. The first question that pops to mind is "why arent you working with RawShaderMaterial and just take care of everything yourself? I think you should familiarize yourself more with how things are done with three.js and whats the reasoning behind those decisions. Why do you think that all the materials should include this flag, or this check, when only one type actually allows you to write GLSL? Why should a MeshLambertMaterial ever care about this when nothing is being done with these uniforms? It's pretty much relevant only on the ShaderMaterial.

One more interesting thing to note, out of 116 contributors to WebGLRender i see 5 authors listed in the file, similar with Material, out of 26, only two mentioned in the file... Good to see you added your name up there ;)

jcowles commented 7 years ago

@pailhead ah, sorry about adding the author, I was really just pattern matching, I'm glad you have a friendly way of onboarding new contributors that doesn't involve sarcasm :)

And yes, great note that this should be on RawShaderMaterial, will fix both.

As for the rest of the discussion, if you think this doesn't belong in the lib, that's cool.

pailhead commented 7 years ago

:)

Doesn't belong like this, but i think discussion is important. Am I at least right on understanding #3? I still think you might be missing something, out of the two GLSL apis that three exposes, this seems like it would be more relevant on the ShaderMaterial rather than RawShaderMaterial. Raw exposes nothing or very little, you need to set everything up, shader is the one that gives you stuff for convenience.

pailhead commented 7 years ago

if #3 is the problem then i think a more three.js like solution is to have something like this

jcowles commented 7 years ago

Ah yeah, ShaderMaterial.

And yes, (2) and (3) are very related. If the engine controls the transform hierarchy, it has an opportunity to do something smart. What I mean by "forfeit" is that the engine is giving up control to the user -- but maybe that's the three.js way? Renderers I've written have explicitly internalized this, since it's an opportunity for a big performance win and users are unlikely to do it right.

So should I fix (move to ShaderMaterial, fix comments, etc) or just kill this PR?

WestLangley commented 7 years ago

@jcowles I agree with you that there is something to be said for following Unity's lead. Thing is, there has yet to be a demand for pushing these particular inverse matrices to the GPU.

Even so, we are trying to propose a flexible solution that accommodates your use case.

I think onBeforeRender is that solution, and I think your use case is a perfect example of how the method can be beneficial.

pailhead commented 7 years ago

I would keep stabbing at this. Code definitely helps us half autistic dorks communicate :D I think it was kinda hard to understand your use case until you made the pull request. I also find it hard to navigate three's core. I've been poking through these classes recently but I don't know off the top of my head if your thing can be achieved the way it's been proposed here.

My understanding is that three favors some magic for ease of use (first thing that pops to mind are gpu uploads that happen the first time render() encounters a new object) but also flexibility (exposing the necessary hooks for you to extend three). It's not the most optimized library, but this is a very fine balance. I'm totally in favor of borrowing ideas from other engines. Look up onBeforeRender's history to see how tricky it may be to push something like this :) . The concept is totally borrowed from Unity's onWillRenderObject and such events. I wanted to do an onWillUpdate but i found it tricky the way three handles the updates as is.

With all of that being said, something that alters the core like this should probably be discussed well.

What i think i learned from this conversation is that the modelViewMatrix is automagically computed under the hood, and that it isn't accessible for further computation, when needed. There is a valid use case, (computing some other matrix from this matrix), and while the current system gives you a convenient hook to write your logic and pass it to the shader, it requires a heavy matrix operation to be done twice (you'd do it once explicitly, but you can't prevent the renderer from doing it again).

What you may have learned is that there is already a pattern in place for checking if a matrix is dirty or if the renderer should update it at all (autoUpdate and needsUpdate i think). So your approach may be adjusted. Also probably a bit about the materials that completely abstract any GLSL, and the two different GLSL apis that three has.

My suggestion would be to still rely on the onBeforeRender hook ( is this how you would handle it in unity, by using Update()?) but with the modelViewMatrix that's updated only when needed?

jcowles commented 7 years ago

@WestLangley yes, "that's a niche use case," is a totally valid argument -- if that were the initial response, I would have actually dropped this right at the start.

But I am curious, why are you opposed to making it an opt-in core feature?

jcowles commented 7 years ago

@pailhead I'm glad we're starting to understand each other :)

I'll hack up the onBeforeRender approach and see how it works, though I'm pretty strapped for time currently. I'm still more in favor of what I'm proposing, but you and @WestLangley are right, I should definitely give it a try, look at the resulting code, and run some profiles.

arose commented 7 years ago

@jcowles Here is what I did to get a couple of matrices via onBeforeRender https://github.com/arose/ngl/blob/master/src/viewer/viewer.js#L48-L129

When there would be a callback after setProgram is called in the WebGLRenderer then you could avoid the call to gl.useProgram( p.program ); and the calculation of the object.modelViewMatrix.

pailhead commented 7 years ago

The way uniforms have to be updated right now is super cumbersome :( it shouldn't require low level gl calls.

jcowles commented 7 years ago

So this launched today: https://tiltbrush.com/sketches/

The camera-facing snow particles are my motivating use case for this issue.