Allow to pass varying from fragment to light shader function

[Chaosus]

Allows passing varying from fragment to light:

It's exactly what @clayjohn was requested. I've implemented this variant cuz it's the easiest to implement of all variants. If it's not what everybody wants, I could create new variants... or @lyuma could try to fix it himself.
Closes godotengine/godot-proposals#2022

[lyuma]

See my comment here: godotengine/godot-proposals#2022 (comment)
In short, I'd strongly prefer the shader language not overload the use of the varying keyword.

I made a suggestion and other remarks in the linked comment above.

[clayjohn]

@Chaosus on review, and based on lyuma's comments. I think adding an optional hint hint_fragment_to_light makes sense. Varyings tagged with the hint can only be accessed in the fragment and light functions and don't waste a GLSL varying. This will be necessary because low end devices have pretty low limits for varyings.

[Chaosus]

I think adding an optional hint hint_fragment_to_light makes sense.

Ok, will modify it at the nearest time.

[Chaosus]

@clayjohn Added fragment_to_light and vertex_to_fragment hints, working properly:

[clayjohn]

I am happy with this implementation. Normally at this point I would approve, but we haven't reached consensus in godotengine/godot-proposals#2022

[BastiaanOlij]

Looks pretty sound to me, I'm assuming hint_vertex_to_fragment is the assumed default if no hint is given?

[Chaosus]

I'm assuming hint_vertex_to_fragment is the assumed default if no hint is given?

Yes

[reduz]

I suggest the following

• varying should ONLY work vertex2fragemt, vertex2light, or fragment2light. It also should be be always read-only in the destination stage. If used in more than two stages, you tell the user this is invalid and user must use a different one and pass the value. I understand in a very rare use case you may want to do this, given its rare its easier than the user does the workaround.

• When compiling, the compiler should detect where it was used and tag it accordingly for the codegen, which will use the proper approach for this.

[reduz]

example:

//valid
varying float something;

vertex() {
    something=1.0;
}

light() {
    LIGHT.r = something;
}

//valid
varying float something;

vertex() {
    something=1.0;
}

fragment() {
    COLOR.r = something;
}

//valid
varying float something;

fragment() {
    something=1.0;
}

light() {
    LIGHT.r = something;
}

//invalid: error: Keyword 'varying' must only be used in two different stages, which can be 'vertex' 'fragment' and 'light'
varying float something;

vertex() {
    something=1.0;
}

fragment() {
    something +=1.0;
}


light() {
    LIGHT.r = something;
}

//invalid: error: Keyword 'varying' must only be used in two different stages, which can be 'vertex' 'fragment' and 'light'
varying float something;

vertex() {
    something=1.0;
}

[reduz]

Also ShaderLanguage::Varying can have the following info for the codegen:

struct Varying {
    enum Stages {
        STAGES_VERTEX_TO_FRAGMENT,
        STAGES_VERTEX_TO_LIGHT,
        STAGES_FRAGMENT_TO_LIGHT,
    };
    
    Stages stages = STAGES_VERTEX_TO_FRAGMENT;
    DataType type = TYPE_VOID;
    DataInterpolation interpolation = INTERPOLATION_FLAT;
    DataPrecision precision = PRECISION_DEFAULT;
    int array_size = 0;

    Varying() {}
};

[lyuma]

I have a question: if something is vertex2light, it makes sense to also allow reading it in both fragment() and light(): the write/read/read case.

For example, say I wanted to take some special value calculated in vertex() and pass it into both the fragment() and light(). It seems awkward to require using two interpolators and two different variable names just so that I can use it in both places.

While I'll ultimately accept whatever reduz decides, I continue to find the above proposed use of the keyword "varying" to be inconsistent with anything you'll find in any real-world shading environments, and, therefore, confusing both for seasoned developers and for novices who look up documentation outside of the Godot bubble. I really want Godot to adopt idioms in widespread industry use, rather than painting a make-believe world (such as one in which fragment() and light() are separate programs that magically send data to one another)

[Chaosus]

Done, all as @reduz requested.

[akien-mga]

Thanks!

[SIsilicon]

Excellent!
I apologize if I'm asking too much of you @Chaosus, but is it possible for this to be backported to 3.2?
It doesn't have to be by 3.2.4. It could be in the next release (3.2.5?).

[Chaosus]

@SIsilicon It's possible, but as I'm a non-paid contributor it depends on my current motivation and mood to do so.

[SIsilicon]

I can relate to that. You're probably even busy doing something else right now.

[lyuma]

I will be making an attempt to backport this change to 3.2, as it reduces fragmentation to have as much shader language compatibility as possible.

I also maintain ports of other shader features introduced in master to 3.2, including structs ( https://github.com/lyuma/godot/tree/shader_struct_backport ). I'd be happy to submit these as PR's to the 3.2 branch after 3.2.4 is released.

[lyuma]

@SIsilicon This feature was just merged into the 3.x branch! This means it will be in 3.4 when it gets released. (It is not available in 3.3 or earlier.)