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

[3.x] Built-in toon diffuse mode too bright #63297

Open
atirut-w opened this issue Jul 21, 2022 · 7 comments
Open

[3.x] Built-in toon diffuse mode too bright #63297

atirut-w opened this issue Jul 21, 2022 · 7 comments

Comments

@atirut-w
Copy link
Contributor

atirut-w commented Jul 21, 2022

Godot version

v3.4.4.stable.fedora [419e713]

System information

Fedora Linux 36 (Workstation Edition), GLES 3

Issue description

When you set a material's diffuse mode to toon and have a light source(only tested with directional light), the lit part of the material becomes significantly brighter than normal materials of the same albedo with Burly(ddefault) diffuse mode.

This is more noticeable with the Glow effect enabled(screen mode) and shadow disabled(a separate issue of toon shader being dependent on shadowmap instead of dot product?).

Testing condition:

  • Directional light energy at 1
  • default skybox

With shadows

Without glow

image

With glow

image

Without shadows

Without glow

image

With glow

image

Steps to reproduce

  1. New scene
  2. Add directional light
  3. Add mesh with toon material
  4. Burn your retinas

Minimal reproduction project

No response

@atirut-w atirut-w changed the title Built-in toon diffuse mode too bright with shadows disabled Built-in toon diffuse mode too bright Jul 21, 2022
@fire
Copy link
Member

fire commented Jul 21, 2022

@lyuma mentioned to me about dividing by a pi constant. Do you remember?

@Calinou
Copy link
Member

Calinou commented Jul 21, 2022

Note that changing this in 3.x will break compatibility with existing projects, unless it's made opt-in via a project setting (which could be automatically enabled in projects created with 3.6).

@atirut-w
Copy link
Contributor Author

atirut-w commented Jul 31, 2022

@Calinou can a fix for this get into 4.0? It seems to kind of also be happening in alpha 13 and I'd like this fixed before the freeze.

@Calinou
Copy link
Member

Calinou commented Jul 31, 2022

@Calinou can a fix for this get into 4.0? It seems to kind of also be happening in alpha 13 and I'd like this fixed before the freeze.

Bug fixes are not affected by the feature freeze – they can be contributed at any time 🙂

I'd recommend looking into opening a pull request yourself.

@Calinou
Copy link
Member

Calinou commented Nov 13, 2023

@atirut-w Can you test this in 4.2.beta 6? This may be fixed as evidenced by #65544 (comment).

This wasn't backported to 3.x yet though (see my above comment about compatibility).

@atirut-w
Copy link
Contributor Author

I'll try it out this afternoon-evening, thanks. 👍

@atirut-w
Copy link
Contributor Author

@Calinou it is indeed fixed, in the latest master branch. The "receding shadows" issue is still present, but I think this is easily fixable by changing the order of lighting calculations. It's probably a similar issue to the ones I've encountered when making custom lightings.

@lyuma lyuma changed the title Built-in toon diffuse mode too bright [3.x] Built-in toon diffuse mode too bright Jan 14, 2024
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

3 participants