Preventing redeclaration of variables when using ShaderChunks

[WestLangley]

clipping_planes_pars_vertex.glsl uses this coding pattern

#if NUM_CLIPPING_PLANES > 0 && ! defined( PHYSICAL ) && ! defined( PHONG )

    varying vec3 vViewPosition;

#endif

In other words, declare viewViewPosition -- but not if it has been predeclared.

These material-dependent chunks become problematic when creating new materials, because it can lead to variable redeclaration conflicts if it is not known if the variable has been previously declared. For example:

#if SOME_CONDITION

    varying vec3 vViewPosition;

#endif

...

//from clipping_planes_pars_vertex
#if NUM_CLIPPING_PLANES > 0 && ! defined( PHYSICAL ) && ! defined( PHONG )

    varying vec3 vViewPosition;

#endif

As the number of materials increases, figuring out how to modify the chunks to avoid a potential redeclaration is too tedious.

Would the following coding pattern be acceptable? To be used only when needed.

varying vec3 vViewPosition;
#define VVIEWPOSITION

...

//rewrite of clipping_planes_pars_vertex
#if NUM_CLIPPING_PLANES > 0 && ! defined( VVIEWPOSITION ) 

    varying vec3 vViewPosition;

#endif

Or, is there a better solution to decouple the material types from the Chunks?

[bhouston]

The way we have been doing it until now is to list all of the conditions that require that variable at the point of its declaration. If we were to follow that pattern here, it would result in simpler code.

[mrdoob]

Sounds good to me, yeah.

[bhouston]

@WestLangley, could you just add something like a

#require varying vec3 vViewPosition

And once would parse the shader looking for the above structure replace these with your defines automatically. I guess I want to keep the glsl simple, even if it means adding some additional preprocessor commands.

[WestLangley]

@bhouston I am open to that. But if you want to go that route, someone other than myself should implement it -- at least at first.

[mrdoob]

I think we can do this on two steps. @WestLangley suggestion sounds like a (much needed) clean up.

[bhouston]

No worries, I was just brainstorming. To be more specific, I was just thinking of doing a search and replace of patterns like this:

#require varying vec3 vViewPosition

// or here is another form:
#defineonce( varying vec3 vViewPosition )

With something like this:

#ifndef defineonce_varying_vec3_vViewPosition
   varying vec3 vViewPosition
   #define defineonce_varying_vec3_vViewPosition
#endif

Then the code example you gave about would look like:

#defineonce( varying vec3 vViewPosition; )

...
//rewrite of clipping_planes_pars_vertex
#if NUM_CLIPPING_PLANES > 0

    #defineonce( varying vec3 vViewPosition; )

#endif

Which would be transformed into this:

#ifndef defineonce_varying_vec3_vViewPosition
   varying vec3 vViewPosition;
   #define defineonce_varying_vec3_vViewPosition
#endif
...
//rewrite of clipping_planes_pars_vertex
#if NUM_CLIPPING_PLANES > 0

   #ifndef defineonce_varying_vec3_vViewPosition
      varying vec3 vViewPosition;
      #define defineonce_varying_vec3_vViewPosition
   #endif

#endif

EDIT: It may make more sense to use a hash (MD5 or shorter) of the "defineonce" contents rather than trying to turn it into a valid preprocessor variable.