godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.16k stars 97 forks source link

Non-physical shading: access to lighting within the `fragment()` function #1620

Open lyuma opened 4 years ago

lyuma commented 4 years ago

Describe the project you are working on:

I am building a VRM importer for Godot. VRM is a glTF extension based around distribution of stylized non-realistic characters such as models available from VRoid studio, Nico 3D or booth.pm - there is a rapidly growing market of model creators building content in this format. The MToon shader is a main feature of this model format, and it has already been ported to multiple engines, including three.js: https://www.w3.org/2019/09/Meetup/YutakaObuchi_VRM.pdf

Describe the problem or limitation you are having in your project:

We wish to provide means of true NPR (non-physically rendering) shading approaches within Godot. Among the premise of this issue is that we need a solution which avoids any of the following as they create friction:

Currently, it is not possible to build a toon shader in Godot without hitting one of these limitations.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:

For the purpose of this discussion, I picked the well-licensed shader, MToon, which also happens to be part of the VRM model interchange specification. The official MToon implementation is a Unity shader, so I made an identical testing environment in both Unity 2018.4 and Godot 4.0 and Godot 3.2 in which to test lighting. Here is the end result, with an implementation of this proposal applied:

For comparison and as a control, I made my best attempt to port MToon using the tools available in the current version of Godot, and I was unable to attain the desired shading. Here is the attempt:

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:

First, I will link to reference code and projects exhibiting my reference implementation.

The Godot project for the above can be found at https://github.com/lyuma/Godot-MToon-Shader The Unity project available within the VRMUnityTestProject folder of https://github.com/lyuma/Godot-MToon-Shader Finally, the pull request with Godot engine modifications to support this PR is available at godotengine/godot#42599

We have shown now that changes to the Godot Engine are necessary, and this capability cannot be provided on top of Godot or in the Asset Library. So, with that out of the way, why modify fragment() rather than extending the light() function?

I found three approaches that could offer functionality to provide stylized shading.

The mechanics of passing data into and out of light() were quite complex given the lack of static globals in GLSL. The post-light approach is not sufficient to implement all toon-shading techniques, including the one used by MToon. I opted for the last option which offered the most flexibility.

This leaves us with the need to expose lighting information to user-written custom shaders, within the fragment() function. This also matches closely approaches used in other environments. See a Unity URP based approach: https://github.com/ColinLeung-NiloCat/UnityURPToonLitShaderExample as a demonstration of this process of iterating through lights.

The approach I used for my reference implementation was to reorganize the scene shader to expose more things into functions, such as the calculations for directional shadow attenuation, decals and more. I migrated functions for calculating stuff like Lighting, directional lighting, ambient, baked etc. to return 0 if the approriate #define is not set, rather than being undefined. To keep the API organized and as a convention, I used UPPERCASE_FUNCTION_NAMES for functions which call into these parts of the scene shader.

Functions / lighting paths which are no longer implemented can be easily deprecated into a function which returns false. Some lighting paths may be possible to expose as const bool expressions, but many which rely on instances.data[instance_index].flags must be functions.

Allow for disabling builtin lighting passes, if overridden. One approach is to do this if the appropriate expression is used. For now, I disable most processing if the shader writes to AMBIENT_LIGHT, but I am open to other proposals. Decal processing is separate from lighting, and the builtin decal processing is currently disabled if DECAL_PROCESS() is ever called.

Future work: Writing all shading in a single function is a little bit awkward. I would like to change the injection point for user-written functions to be immediately above void main(). This would allow user-written functions to call some of these builtin functions as well as access some globals.

Giving more access to global variables can only be a good thing, I think... I don't see a reason to disallow this. Also, variables not assigned in the vertex stage can still be disallowed at the shading language level. (We might need to do some dependency checking to avoid including functions which are called only from fragment in the vertex stage.)

Finally, about compatibility with low-end rendering environments, such as GLES2 should it be implemented: this can be done in two ways:

If this enhancement will not be used often, can it be worked around with a few lines of script?:

I predict that this enhancement, or assets based on it, will be heavily used. NPR shading is an extremely desired feature by some communities. In fact, there were 3 questions posed in the chat at the last Q&A asking for NPR. Genshin Impact is a high profile game and was based around an extremely stylized non-physical renderer (deferred, but no reason it can't be done as clustered-forward).

Is there a reason why this should be core and not an add-on in the asset library?:

See above: This proposal serves to add functionality to the engine not directly related to NPR, but which will enable toon shaders to be written and (hopefully) published to the asset library.

Master-J commented 4 years ago

I'm currently writing a toon shader that needs to react to multiple light sources and GI. While i managed to write a very good shader, i had to do it in 2 passes :

Theoraticaly, my shader could be done in a single shader, but as of Godot 3.2.x, wirting a light shader is painfull because of how it's designed, here's a handfull of things that really bugged me :

Those 3 points discouraged me from writing anything but a simple rim light and specular highlight in my shader (without those features, the shader could be only an unshaded fragment() shader).

A thing that would make the light() function easier to deal with in both PBR and NPR stuff is instead of making it run per light, make it run a single time while passing it arrays containing the lighting data (light directions, light colors/energy/attenuation, ambient light....).

I could see a use for most of the new built-ins this PR adds to the engine and i'd really want to have them for godot 4.0 but i'm not sure pushing them all in the fragment() function is the most sensible way of doing things (maybe that's the most convienient way of implementing them, i'm not familiar enougth with the engine's inner workings). I think the best way to do this this giving the light function a big upgrade to include them as built-ins like "LIGHTMAP_CONTRIBUTION" "LIGHT_ENERGY" 'LIGHT_COLOR" (without energy) and so on.

Overall, great proposal, it brings a lot of nice things that could make NPR really easier to do than in 3.x and with a bit of polish and edits here and there, it could also benefit any kind of rendering.

HungryProton commented 4 years ago

I like this proposal and I think there's demand among users for something like this.

I don't know much about the engine internals but I'm in the same boat as @Master-J, currently writing my own toon shader and facing similar limitations, mostly:

So yes I think this is the way to go and I like the idea of exposing as much information as possible without making specific assumptions that only benefits a PBR approach (like mixing three different information in a single constant like LIGHT_COLOR for example).

lyuma commented 4 years ago

@reduz, I have fully implemented two toon shaders, and have them working in a modified 3.2.x, as well as modified 4.x.

My studies have mostly revolved around Unity shaders, as many shaders are published on Github and the MIT License is widely used. I am also more familiar with Unity, and further, I have already shown that its lighting model is compatible with Godot.

Common features: Most Unity shaders use directional lighting, support additional omni/spot lights, and of these, most support extracting direction from spherical harmonics for baked lighting environments. In addition, many shaders offer some optional PBR features, including specular, reflections, normals and detail normals, used on clothing for example.

Together, these ports have shown that what is proposed here is sufficient to implement a variety of shading and lighting models. It leaves open questions of whether there are features exposed which do not need to be, and if there are better ways to organize these shader designs (such as using fragment() + light() with data passing).

Regarding the usage of omni/spot light array in clustered

I have been thinking heavily about this particular issue over the past month. I've largely come to the conclusion that there is no sane operation to be done for additional omni and spot lights, other than either summing them, or baking them to probes and sampling the probes, which would then take all information into account.

@reduz, I believe you made a comment that clustered forward could create trouble with the use of the omni and spot light array. Indeed, in an attempt I made to calculate average light direction, I did not take the attenuation of the lights, and I was able to create clustering artifacts:

multilight_clustered

If I used the attenuation of the lights (even raised to a fractional power), I was able to avoid this artifact, but this drastically limited the technique, and often led to sharp contour lines along the edge of the lights' influence. This does appear to me like a dead-end for techniques such as calculating an "average light source".

I had originally proposed this, because Unity’s Universal Render Pipeline (URP) offers a similar API to what I proposed. However, what I did not realize at the time, based on extensive searching on Github is that almost every shader written for URP merely does a loop over lights, and sums the light contributions (with a single exception: an commercial toon shader in "Lux URP Essentials" on the Unity Asset Store -- I am not able to verify this claim for myself).

All together, this is leading me to the conclusion that there is no use case for working outside the light() framework in Godot, provided that additional functionality is added.

Extremely helpful features that I found useful or necessary to get Silent's shader running:

A note to @HungryProton, @Master-J and those asking for a separation between "Light Intensity" and LIGHT_COLOR, I believe this distinction to be poorly defined and inconsistent. My suggestion is take the maximum over R,G,B (absolute value?) as the intensity, and divide by this value (assuming not 0 / black) to derive the original color. Is there a reason why this is unacceptable? (What intensity and color values are you unable to detect with this method)

Conclusions and next steps I hope I have demonstrated why a large portion of the features in this proposal are necessary. I am willing to concede that access to the omni and spot light array is not a requirement, rather merely a convenience in avoiding the need to extend or modify light().

To move forward, we might need to discuss what changes ought to be made to this proposal.... ideally keeping shader creation in line with Godot's principles while still providing enough tools to develop Silent's shader and others.

For example, I could come up with a different proposal/PR which includes extensions to the light() function, along with a means to pass data to it. Or, we could clean up the API to return a struct, instead of a series of GET_... functions. This would require compiler changes but could be a nicer experience.

-Lyuma

HungryProton commented 4 years ago

@lyuma

My suggestion is take the maximum over R,G,B (absolute value?) as the intensity, and divide by this value (assuming not 0 / black) to derive the original color.

That's what I'm doing to get the color but this method proved to be unreliable to get the exact light energy. It also makes it impossible to differentiate a gray light source at 1.0 energy from a white light source at 0.5 energy, which may be fine in PBR rendering but not when trying to do something stylized.

I'm currently using this method to derive the data I need:

vec3 light_tint = LIGHT_COLOR / max(LIGHT_COLOR.r, max(LIGHT_COLOR.g, LIGHT_COLOR.b));
float energy = 0.73 * length(LIGHT_COLOR) / pi;

But it's not reliable and requires some magic numbers that comes from trial and error, (plus it's wasting some processing power that could be used elsewhere) which is why I would love to have these two as separate variables. I believe @Master-J got around this issue by making his shader in two passes but that's extra complexity I'd like to avoid.

reduz commented 4 years ago

@lyuma I am honestly not super interested about what you can do with SH. I understand Unity kinda lacks support for other types of global illumination and this is the only thing it you can use, but as there are other (more modern) ways to create GI nowadays I think its kind of pointless to go this route. Additionally, sampling SH in multiple directions is quite expensive.

From Godot 4.0 onwards you have global and per instance uniforms, so you will most likely be able to do a custom system with much greater ease.

I am more interested about the logic you run per light, and how do you add lights together.

reduz commented 4 years ago

Well, I suppose if required we can give access to SH, it wont hurt much, but users will be probably quite confused it only works with lightmaps.

reduz commented 4 years ago

Still, let me put this another way. As what you are doing is basically creating your own forward shader, and given Godot default shaders are not meant to provide rendering technique specific implementation and work with all of them, I am not interested in allowing this to happen. I think its a lot of work, it will never be enough, and it will break when users use this in different back ends. This is also super limiting as it only works with one specific type of rendering and shading.

Instead, I just find simpler you provide a new .glsl file (that replaces the entire Godot one) and do this on your own (only if you want to implement such a specific rendering technique that only works in a specific context), instead of using Godot shaders.

If you are worried that core Godot shaders might change, this is something we can work on by moving further stuff to the _inc file (that the engine will provide for you), to ensure you are not affected by those changes as much, in a vein a bit more similar to how Unity works. But then again, I see no point in making the built-in shaders (that are meant to be easy to use) more complex for the sake of this.

fire commented 4 years ago

Summarizing the discussion:

At the end you will be creating something that only works on a given driver and context (in this case, this toon shader only works with lightmaps and with vulkan), imo its kinda pointless to expose all this

I don't understand where it says vulkan is required

Because you need the result of the light additions in GLES3, light is done multipass, so you cant obtain this result, it wont work there. and when we make the filmic renderer sometime in the future, it will use deferred rendering, so you wont be able to do this either

Make a new .glsl shader, it's not much more complicated (in fact, it should be more straightforward), and it has the extra advantage you can use as a meta-shader, then you create materials using that meta shaders.

Have the ability to replace the base GLSL shader but have most of the standard functions in some engine-defined include files

Call this feature "meta-shaders," and they would be back-end specific

Some fears were described:

  1. It should not require a engine change
  2. It needs to be packageable as an asset library archive
  3. Distributable on stable Godot Engine releases

None of these apply to the meta-shader approach.

reduz commented 4 years ago

@fire as it is now its probably quite complicated to make a new glsl shader, but we can move a lot of stuff to functions and to the common file to make this process simpler.

lyuma commented 4 years ago

reduz: I'd respectfully like to rebut some of the arguments you have made. But I understand a lot of the comments you make, and I also recognize that I asked for way more than I actually need. I think we can actually find some common ground here that doesn't require moving to GLSL.

I am more interested about the logic you run per light, and how do you add lights together.

I loop over all lights once, and sum the results, same way almost all shaders work. This means that I can modify the proposal to work within the light() framework, with some modifications.

As what you are doing is basically creating your own forward shader, and given Godot default shaders are not meant to provide rendering technique specific implementation and work with all of them

The same could be said about any shader which uses SCREEN_TEXTURE, any shader which is written as unshaded, even any shader which relies on a transparency. These could behave differently or unexpectedly in different rendering techniques, yet we still allow them, because they are useful.

Not all content will be future-proof in every way against any possible renderer. Sometimes this requires trust in the community that assets will be maintained to work in future Godot versions. ( For my immediate projects, I can make a point that this only works in forward renderers and designed for Godot 4.0, and not guaranteed to work in future implementations. )

Additionally, sampling SH in multiple directions is quite expensive.

I'm assuming you are referring to the fact that I call AMBIENT_PROCESS or REFLECTION_PROCESS multiple times, in order to calculate an "average" or "flat shading". This is certainly not ideal.

I'm thinking what the best thing is to do here. Your mention of global uniforms has given me some ideas.....

Well, I suppose if required we can give access to SH, it wont hurt much, but users will be probably quite confused it only works with lightmaps.

SH (with lightmap bake) is ideal for toon shading: given the coefficients, they trivially provide a dominant light direction in L1, the gradient along that direction, as well as the constant L0 term for flat shading. (Requiring only a few math operations)

Hence, I imagine that some games would bake the entirety of their levels. This alone could allow for nice cel shading in games which opt to go this route, and is cheap enough to work even on mobile.

So while you are right to point out that it only works in some environments, I'd like to keep the discussion for SH coefficients open even if it is solved separately from the "general case" I mention above.

Given the fact that this has specific scope and is not general purpose, shall I migrate this to its own proposal?

Instead, I just find simpler you provide a new .glsl file (that replaces the entire Godot one)

I disagree, at least without seeing the framework you have in mind (and when will it be done?) For example, if an error is made, the engine spams log messages that are hard to find and diagnose. You can't edit them in-engine. (The in-engine editor has been quite helpful in porting and playing with the two shaders above)

this is something we can work on by moving further stuff to the _inc file (that the engine will provide for you)

This seems like it's adding maintenance burden on both ends now. This proposal was intended to reduce maintenance burden by providing a clean interface, not add to it.

lyuma commented 4 years ago

A small update: While it may appear that this proposal has stagnated, this functionality is not dead; however, its implementation will come in a different form, and will be available in Godot 4.0.

I'll keep you all posted once that has progressed.

Read my full comment on the matter here: https://github.com/godotengine/godot-proposals/issues/1778#issuecomment-723515820

lyuma commented 3 years ago

Just wanted to draw your attention to a new video that happens to be highly relevant to this proposal: https://www.youtube.com/watch?v=GGTTHOpUQDE

Unity just released this excellent 10-minute tutorial on building toon shading using their Shader Graph for Universal Render Pipeline, including support for shadowing and multiple lights. (This is part of their Open Project) It shows a great illustration of multiple concepts referenced within this proposal. I think this flexibility is something for us to aspire to achieve.

In terms of Godot, the approach we will take for now we will be based on the GLSL master shaders concept set forth by Reduz, hopefully such that VisualShader or the in-engine shading language can be used on top. This could bring out the best of VisualShader while also giving the power to build custom lighting models.

Even if not suitable for building master shaders, this does validate the effectiveness of a good integration with VisualShader (as well as the in-engine shading language). For example, given a toon shading master node, VisualShader could be suited to provide effects it is good at, for example a "wind effect using vertex displacement" as mentioned in the Unity video.

ghost commented 3 years ago

I would also like to add that from what I have seen most indie devs tend to go for unique look for their games rather than photorealism due to practical reasons. Is not possible for majority of smaller dev teams to compete against big studios which push realism to the max and have unlimited resources and money to research the next best movie-like technology. Making games with less resources means to make them stand out visually or mechanically. Supporting NPR should would actually help indie devs create a much better visual look for their games. Godot seems to attract a lot more indie devs and I feel like many would actually benefit from more control over lighting pass in shader for example. I hope a way can be found where this can happen and core devs would also be okay with the implementation. Thank you for your work.

tlitookilakin commented 3 years ago

Access to accumulated lighting data either in fragment() or a post-light pass would be incredibly useful, and not only for stylization. I've been working on a project using light masking and wanted to manipulate the accumulated alpha values to create a kind of threshold effect, but unfortunately this is not possible in 3.2,.

lyuma commented 3 years ago

Access to accumulated lighting data either in fragment() or a post-light pass would be incredibly useful, and not only for stylization. I've been working on a project using light masking and wanted to manipulate the accumulated alpha values to create a kind of threshold effect, but unfortunately this is not possible in 3.2,.

Unless I misunderstood, the specific thing you ask for is already possible in Godot (for 3D). Caveat: multiple directional lights are not supported in Godot 3.2 and will fall back to multiple draw calls for every directional light, which may look wrong.

On Godot 3.2, DIFFUSE_LIGHT, SPECULAR_LIGHT and ALPHA are both read and write (on GLES3). Here's something I just made which should work on vanilla Godot 3.2... is this what you wanted, @tlitookilakin ? Example shader showing an object invisible except where hit by light

This means, it actually is possible to accumulate light into these values and update ALPHA at the end of each light() function. Here's the simple example shader which you can try for yourself: https://gist.github.com/lyuma/ff48f672a9c4abeb24fc131892e06a5e

Note: For brevity, I didn't copy code for specular from the default BRDF, so this only implements diffuse. Adding specular would be trivial by copying from example code.

To avoid making this proposal into a help channel, if you have more questions about this, you're free to ping me on the Godot discord with the same username, by email, DM on RocketChat or elsewhere.

tlitookilakin commented 3 years ago

I've actually been working primarily in 2D, but I'll take a look at your technique and see if I can adapt it. EDIT: ALPHA does not seem to be accessible in 2d light shaders, but thanks for the suggestion. The only things modifiable in 2d light shaders are the shadow and light colors, which are only accessible from within light()