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

NodeMaterial doesn't always recompile #18347

Closed
3 tasks done
njarraud opened this issue Jan 9, 2020 · 6 comments
Closed
3 tasks done

NodeMaterial doesn't always recompile #18347

njarraud opened this issue Jan 9, 2020 · 6 comments

Comments

@njarraud
Copy link
Contributor

njarraud commented Jan 9, 2020

Description of the problem

NodeMaterial is only recompiled the first time the needsUpdate flag is set to true if the WebGL program cache key doesn't change. I believe that this is due to the fact that now material recompiling is based on version and not only on this needsUpdate flag.

To fix this issue, onBeforeCompile.toString method of NodeMaterial shall return the material version instead of the needsCompile flag. It will force programCacheKey to be different and therefore the material will recompile as requested by the needsUpdate flag.

this.onBeforeCompile.toString = function () {

    return self.version

};
Three.js version
  • Dev
Browser
  • All of them
OS
  • All of them

\ping @sunag

@sunag
Copy link
Collaborator

sunag commented Jan 9, 2020

It will force programCacheKey to be different and therefore the material will recompile as requested by the needsUpdate flag.

It can be a problem if the proposal is to share the same shader program with other materials.

Do you already try to add needsCompile = false after the build?

this.onBeforeCompile = function ( shader, renderer ) {

		var materialProperties = renderer.properties.get( this );

		if ( this.version !== materialProperties.__version ) {

			this.build( { renderer: renderer } );

			shader.uniforms = this.uniforms;
			shader.vertexShader = this.vertexShader;
			shader.fragmentShader = this.fragmentShader;

			this.needsCompile = false;

		}

	};

@njarraud
Copy link
Contributor Author

njarraud commented Jan 9, 2020

No, that doesn't work.

@rclankhorst
Copy link

rclankhorst commented Feb 28, 2020

this.onBeforeCompile = function ( shader, renderer ) {

		var materialProperties = renderer.properties.get( this );

		if ( this.version !== materialProperties.__version ) {

			this.build( { renderer: renderer } );

			shader.uniforms = this.uniforms;
			shader.vertexShader = this.vertexShader;
			shader.fragmentShader = this.fragmentShader;

			this.needsCompile = false;

		}

	};

This change worked for r111 and r112, but is no longer sufficient for r113.
The line changed by @njarraud does produce a fix.

@rclankhorst
Copy link

/ping @sunag Any ideas? I don't feel like making a PR for something potentially troublesome.

@sunag
Copy link
Collaborator

sunag commented Jul 14, 2020

@njarraud @rclankhorst You can test now with the current dev state of the threejs?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 17, 2020

Should be fixed via #19833.

I think it's okay to close this one. It can be reopened if there are still recompilation issues with r119.

@Mugen87 Mugen87 closed this as completed Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants