WebGLPrograms: Add missing ParameterNames #17651

gero3 · 2019-10-03T06:54:29Z

This is continuation of #17552. This adds all missing parameters to the parameterNames Array.

Mugen87 · 2019-10-03T13:42:55Z

This is continuation of #17552.

So it's a replacement for #17552, correct?

In essence, the entries in parameterNames should match the object properties of parameters, right (except shaderID)?

three.js/src/renderers/webgl/WebGLPrograms.js

Lines 143 to 235 in a5c9edd

    
           var parameters = { 
        
           	isWebGL2: isWebGL2, 
        
           	shaderID: shaderID, 
        
           	precision: precision, 
        
           	instancing: object.isInstancedMesh === true, 
        
           	supportsVertexTextures: vertexTextures, 
        
           	outputEncoding: getTextureEncodingFromMap( ( ! currentRenderTarget ) ? null : currentRenderTarget.texture, renderer.gammaOutput ), 
        
           	map: !! material.map, 
        
           	mapEncoding: getTextureEncodingFromMap( material.map, renderer.gammaInput ), 
        
           	matcap: !! material.matcap, 
        
           	matcapEncoding: getTextureEncodingFromMap( material.matcap, renderer.gammaInput ), 
        
           	envMap: !! material.envMap, 
        
           	envMapMode: material.envMap && material.envMap.mapping, 
        
           	envMapEncoding: getTextureEncodingFromMap( material.envMap, renderer.gammaInput ), 
        
           	envMapCubeUV: ( !! material.envMap ) && ( ( material.envMap.mapping === CubeUVReflectionMapping ) || ( material.envMap.mapping === CubeUVRefractionMapping ) ), 
        
           	lightMap: !! material.lightMap, 
        
           	aoMap: !! material.aoMap, 
        
           	emissiveMap: !! material.emissiveMap, 
        
           	emissiveMapEncoding: getTextureEncodingFromMap( material.emissiveMap, renderer.gammaInput ), 
        
           	bumpMap: !! material.bumpMap, 
        
           	normalMap: !! material.normalMap, 
        
           	objectSpaceNormalMap: material.normalMapType === ObjectSpaceNormalMap, 
        
           	tangentSpaceNormalMap: material.normalMapType === TangentSpaceNormalMap, 
        
           	clearcoatNormalMap: !! material.clearcoatNormalMap, 
        
           	displacementMap: !! material.displacementMap, 
        
           	roughnessMap: !! material.roughnessMap, 
        
           	metalnessMap: !! material.metalnessMap, 
        
           	specularMap: !! material.specularMap, 
        
           	alphaMap: !! material.alphaMap, 
        
           	gradientMap: !! material.gradientMap, 
        
           	sheen: !! material.sheen, 
        
           	combine: material.combine, 
        
           	vertexTangents: ( material.normalMap && material.vertexTangents ), 
        
           	vertexColors: material.vertexColors, 
        
           	vertexUvs: !! material.map || !! material.bumpMap || !! material.normalMap || !! material.specularMap || !! material.alphaMap || !! material.emissiveMap || !! material.roughnessMap || !! material.metalnessMap || !! material.clearcoatNormalMap, 
        
           	fog: !! fog, 
        
           	useFog: material.fog, 
        
           	fogExp2: ( fog && fog.isFogExp2 ), 
        
           	flatShading: material.flatShading, 
        
           	sizeAttenuation: material.sizeAttenuation, 
        
           	logarithmicDepthBuffer: logarithmicDepthBuffer, 
        
           	skinning: material.skinning && maxBones > 0, 
        
           	maxBones: maxBones, 
        
           	useVertexTexture: floatVertexTextures, 
        
           	morphTargets: material.morphTargets, 
        
           	morphNormals: material.morphNormals, 
        
           	maxMorphTargets: renderer.maxMorphTargets, 
        
           	maxMorphNormals: renderer.maxMorphNormals, 
        
           	numDirLights: lights.directional.length, 
        
           	numPointLights: lights.point.length, 
        
           	numSpotLights: lights.spot.length, 
        
           	numRectAreaLights: lights.rectArea.length, 
        
           	numHemiLights: lights.hemi.length, 
        
           	numDirLightShadows: lights.directionalShadowMap.length, 
        
           	numPointLightShadows: lights.pointShadowMap.length, 
        
           	numSpotLightShadows: lights.spotShadowMap.length, 
        
           	numClippingPlanes: nClipPlanes, 
        
           	numClipIntersection: nClipIntersection, 
        
           	dithering: material.dithering, 
        
           	shadowMapEnabled: renderer.shadowMap.enabled && shadows.length > 0, 
        
           	shadowMapType: renderer.shadowMap.type, 
        
           	toneMapping: material.toneMapped ? renderer.toneMapping : NoToneMapping, 
        
           	physicallyCorrectLights: renderer.physicallyCorrectLights, 
        
           	premultipliedAlpha: material.premultipliedAlpha, 
        
           	alphaTest: material.alphaTest, 
        
           	doubleSided: material.side === DoubleSide, 
        
           	flipSided: material.side === BackSide, 
        
           	depthPacking: ( material.depthPacking !== undefined ) ? material.depthPacking : false 
        
           };

gero3 · 2019-10-03T14:12:35Z

This is continuation of #17552.

So it's a replacement for #17552, correct?

I would prefer #17552 over this, but this indeed fixes the same problem.

In essence, the entries in parameterNames should match the object properties of parameters, right (except shaderID)?

three.js/src/renderers/webgl/WebGLPrograms.js

Lines 143 to 235 in a5c9edd

var parameters = {

isWebGL2: isWebGL2,

shaderID: shaderID,

precision: precision,

instancing: object.isInstancedMesh === true,

supportsVertexTextures: vertexTextures,

outputEncoding: getTextureEncodingFromMap( ( ! currentRenderTarget ) ? null : currentRenderTarget.texture, renderer.gammaOutput ),

map: !! material.map,

mapEncoding: getTextureEncodingFromMap( material.map, renderer.gammaInput ),

matcap: !! material.matcap,

matcapEncoding: getTextureEncodingFromMap( material.matcap, renderer.gammaInput ),

envMap: !! material.envMap,

envMapMode: material.envMap && material.envMap.mapping,

envMapEncoding: getTextureEncodingFromMap( material.envMap, renderer.gammaInput ),

envMapCubeUV: ( !! material.envMap ) && ( ( material.envMap.mapping === CubeUVReflectionMapping ) || ( material.envMap.mapping === CubeUVRefractionMapping ) ),

lightMap: !! material.lightMap,

aoMap: !! material.aoMap,

emissiveMap: !! material.emissiveMap,

emissiveMapEncoding: getTextureEncodingFromMap( material.emissiveMap, renderer.gammaInput ),

bumpMap: !! material.bumpMap,

normalMap: !! material.normalMap,

objectSpaceNormalMap: material.normalMapType === ObjectSpaceNormalMap,

tangentSpaceNormalMap: material.normalMapType === TangentSpaceNormalMap,

clearcoatNormalMap: !! material.clearcoatNormalMap,

displacementMap: !! material.displacementMap,

roughnessMap: !! material.roughnessMap,

metalnessMap: !! material.metalnessMap,

specularMap: !! material.specularMap,

alphaMap: !! material.alphaMap,

gradientMap: !! material.gradientMap,

sheen: !! material.sheen,

combine: material.combine,

vertexTangents: ( material.normalMap && material.vertexTangents ),

vertexColors: material.vertexColors,

vertexUvs: !! material.map || !! material.bumpMap || !! material.normalMap || !! material.specularMap || !! material.alphaMap || !! material.emissiveMap || !! material.roughnessMap || !! material.metalnessMap || !! material.clearcoatNormalMap,

fog: !! fog,

useFog: material.fog,

fogExp2: ( fog && fog.isFogExp2 ),

flatShading: material.flatShading,

sizeAttenuation: material.sizeAttenuation,

logarithmicDepthBuffer: logarithmicDepthBuffer,

skinning: material.skinning && maxBones > 0,

maxBones: maxBones,

useVertexTexture: floatVertexTextures,

morphTargets: material.morphTargets,

morphNormals: material.morphNormals,

maxMorphTargets: renderer.maxMorphTargets,

maxMorphNormals: renderer.maxMorphNormals,

numDirLights: lights.directional.length,

numPointLights: lights.point.length,

numSpotLights: lights.spot.length,

numRectAreaLights: lights.rectArea.length,

numHemiLights: lights.hemi.length,

numDirLightShadows: lights.directionalShadowMap.length,

numPointLightShadows: lights.pointShadowMap.length,

numSpotLightShadows: lights.spotShadowMap.length,

numClippingPlanes: nClipPlanes,

numClipIntersection: nClipIntersection,

dithering: material.dithering,

shadowMapEnabled: renderer.shadowMap.enabled && shadows.length > 0,

shadowMapType: renderer.shadowMap.type,

toneMapping: material.toneMapped ? renderer.toneMapping : NoToneMapping,

physicallyCorrectLights: renderer.physicallyCorrectLights,

premultipliedAlpha: material.premultipliedAlpha,

alphaTest: material.alphaTest,

doubleSided: material.side === DoubleSide,

flipSided: material.side === BackSide,

depthPacking: ( material.depthPacking !== undefined ) ? material.depthPacking : false

};

The parameters are used to create different shaders and different shaders are detected by parameterNames.

Mugen87 · 2019-10-03T15:28:47Z

Um, why not changing:

for ( var i = 0; i < parameterNames.length; i ++ ) {

   array.push( parameters[ parameterNames[ i ] ] );

}

to

Array.prototype.push.apply( array, Object.values( parameters ) );

In this way, parameterNames is not needed. Would it be a problem if array contains shaderID?

gero3 · 2019-10-03T15:48:03Z

It doesn't matter if shaderID is added. ShaderID will just be added twice in that array then. (Line 245)

It can be in a critical path, so we should check performance. Also compability could be a problem if we expect internet explorer to still work. Object.values is not supported by internet explorer but we could polyfill it.

Mugen87 · 2019-10-03T15:58:37Z

It doesn't matter if shaderID is added. ShaderID will just be added twice in that array then.

We could refactor the mentioned lines to the following in order to solve this problem:

if ( parameters.shaderID === undefined )

    array.push( material.fragmentShader );
    array.push( material.vertexShader );

}

It can be in a critical path, so we should check performance. Also compability could be a problem if we expect internet explorer to still work.

True. Unfortunately, IE does not support Object.values() 😞

Mugen87 · 2019-10-03T15:59:41Z

With ES6 we could even write^^:

array.push(  ...Object.values( parameters ) );

Mugen87 · 2019-10-03T16:01:39Z

Anyway, I don't care which PR (this one or #17552) is going to be merged. Both are fine. My suggested refactoring can be done later, too.

Mugen87 · 2019-10-05T09:08:46Z

@gero3 Can you please resolve the merge conflicts? I'd like to approve this change.

gero3 · 2019-10-05T13:53:51Z

@Mugen87 The conflicts are resolved.

Mugen87 · 2019-10-07T09:08:00Z

Merging this one in favor of #17552.

Mugen87 · 2019-10-07T09:10:13Z

Updated builds: fcb84f5

Add missing ParameterNames

a5c9edd

Mugen87 added this to the r110 milestone Oct 5, 2019

gero3 added 2 commits October 5, 2019 15:50

fix merge conflict

5f080a8

cleanup

00d814b

Mugen87 approved these changes Oct 5, 2019

View reviewed changes

Mugen87 merged commit b1e25cf into mrdoob:dev Oct 7, 2019

brianpeiris mentioned this pull request Jan 9, 2020

Rendering to a renderTarget with outputEncoding causes shader compilation on every frame #18354

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

WebGLPrograms: Add missing ParameterNames #17651

WebGLPrograms: Add missing ParameterNames #17651

Uh oh!

gero3 commented Oct 3, 2019

Uh oh!

Mugen87 commented Oct 3, 2019 •

edited

Loading

Uh oh!

gero3 commented Oct 3, 2019 •

edited

Loading

Uh oh!

Mugen87 commented Oct 3, 2019 •

edited

Loading

Uh oh!

gero3 commented Oct 3, 2019 •

edited

Loading

Uh oh!

Mugen87 commented Oct 3, 2019

Uh oh!

Mugen87 commented Oct 3, 2019

Uh oh!

Mugen87 commented Oct 3, 2019 •

edited

Loading

Uh oh!

Mugen87 commented Oct 5, 2019

Uh oh!

gero3 commented Oct 5, 2019

Uh oh!

Mugen87 commented Oct 7, 2019

Uh oh!

Mugen87 commented Oct 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

WebGLPrograms: Add missing ParameterNames #17651

WebGLPrograms: Add missing ParameterNames #17651

Uh oh!

Conversation

gero3 commented Oct 3, 2019

Uh oh!

Mugen87 commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gero3 commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mugen87 commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gero3 commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mugen87 commented Oct 3, 2019

Uh oh!

Mugen87 commented Oct 3, 2019

Uh oh!

Mugen87 commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mugen87 commented Oct 5, 2019

Uh oh!

gero3 commented Oct 5, 2019

Uh oh!

Mugen87 commented Oct 7, 2019

Uh oh!

Mugen87 commented Oct 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mugen87 commented Oct 3, 2019 •

edited

Loading

gero3 commented Oct 3, 2019 •

edited

Loading

Mugen87 commented Oct 3, 2019 •

edited

Loading

gero3 commented Oct 3, 2019 •

edited

Loading

Mugen87 commented Oct 3, 2019 •

edited

Loading