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

Denoise diffuse indirect lighting #750

Merged
merged 7 commits into from
Dec 19, 2024
Merged

Denoise diffuse indirect lighting #750

merged 7 commits into from
Dec 19, 2024

Conversation

LifeKILLED
Copy link

For enabling new denoising method use cvars:

rt_denoise_gi_by_sh 1
Enable denoising global illumination by Quake 2 algorythm (spherical harmonics)

rt_only_diffuse_gi 1
Blend diffuse and specular indirect lighting for cleaner image (reflections denoising will be in future with other algorythm)

@LifeKILLED
Copy link
Author

New denoising (my):
image

Old denoising (in "vulkan" branch now):
image

Without global illumination:
image

Copy link
Owner

@w23 w23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кодировка слетела в двух местах.
А ещё мне не очень понятно, как именно вкл/выкл флагами делается -- в denoiser.comp изменения не под флагами.

@@ -250,7 +252,7 @@ Components blurATrous(const ivec2 res, const ivec2 pix, vec3 pos, vec3 shading_n
const vec3 d0 = indirect_diffuse_c1 - indirect_diffuse_c0;
const vec3 d1 = indirect_diffuse_c2 - indirect_diffuse_c1;

// TODO(or not todo): The Á-Trous paper mentions that it should be c2 + d1 + d0, but
// TODO(or not todo): The -Trous paper mentions that it should be c2 + d1 + d0, but
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кодировонька слетела :(

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я пробовал заменять символ обратно, но видимо IDE само сменило кодировку под Виндой, не знаю, что делать :( Из-за этого пишу все на английском всегда

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Починил

@@ -553,7 +556,7 @@ void main() {
colour = LINEARtoSRGB(colour);

// See issue https://github.com/w23/xash3d-fwgs/issues/668, map test_blendmode_additive_alpha.
// Adding emissive_blend to the final color in the *incorrect* sRGB-γ space. It makes
// Adding emissive_blend to the final color in the *incorrect* sRGB-? space. It makes
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

то же самое

(editorconfig там форсит latin1, я не вмержил исправление в эту ветку ещё)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не знаю, что делать с кодировкой :( Она ломается в любом случае. Надо как-то настраивать IDE, чтобы такого не было, но я пока не знаю, как

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это .editorconfig файл в корне репы шалит. Поменяй там latin1 на utf-8. Это изменение есть у меня в рабочей ветке, пока не принёс в вулкан.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Починил

@@ -306,7 +308,6 @@ Components blurSamples(const ivec2 res, const ivec2 pix) {
direct_diffuse_total += direct_diffuse_scale;

c.direct_diffuse += imageLoad(light_point_diffuse, p).rgb * direct_diffuse_scale;
c.direct_diffuse += imageLoad(light_poly_diffuse, p).rgb * direct_diffuse_scale;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это нормально, что это изменение не под флагом?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это ошибка, я удалил лишнюю строку, исправлю

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправил

vec3 diffuse = c.direct_diffuse + c.indirect_diffuse;
vec3 indirect_diffuse_sh = imageLoad(indirect_diffuse_denoised_by_sh, pix).rgb;

vec3 diffuse = c.direct_diffuse + c.indirect_diffuse + indirect_diffuse_sh;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тоже не под флагом, как оно работает?

Copy link
Author

@LifeKILLED LifeKILLED Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я добавляю все возможные виды пассов в rt.json, они все всегда запускаются и работают (за неимением какой-то системы пришлось так сделать). В своих пассах я проверяю флаги и в зависимости от этого сохраняю результаты в буфферы. В зависимости от флагов в этих буфферах либо нули, либо значения. Например, я проверяю флаги в diffuse_gi_sh_denoise_save.comp и забиваю нулями результаты работы других шейдеров, которые хочу отключить. Перенести ли мне эту логику в denoiser.comp? Наверное, это было бы более правильным и снизило бы число выборок в блюрах.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил в denoiser.comp проверки на флаги где только можно, изменения в этом коммите: f19214e

Также пропускаю флагом ненужный пасс indirect_diffuse_atrous1 при использовании своего метода

@@ -99,6 +100,7 @@ Components dontBlurSamples(const ivec2 res, const ivec2 pix) {
c.direct_diffuse += imageLoad(light_point_diffuse, p).rgb;
c.direct_diffuse += imageLoad(light_poly_diffuse, p).rgb;
c.indirect_diffuse += imageLoad(indirect_diffuse, p_indirect).rgb;
c.indirect_diffuse += imageLoad(indirect_diffuse_denoised_by_sh, p).rgb;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не под флагом

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тогда добавлю сюда ифчик, чтобы читать не все подряд буферы, а только тот, который влияет на картинку

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил флаги

@w23 w23 merged commit e7ef753 into w23:vulkan Dec 19, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants