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.

Sign up for GitHub

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

Fix voxel GI issues #76437

Merged
merged 2 commits into from
Apr 27, 2023
Merged

Fix voxel GI issues #76437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2beed0d
Fix breakages of volumetric fog on voxel GI changes
RandomShaper Apr 26, 2023
09aa1bb
Fix unsupported sampler filter used for voxel GI
RandomShaper Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/classes/RenderingDevice.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,14 @@
<description>
</description>
</method>
<method name="sampler_is_format_supported_for_filter" qualifiers="const">
<return type="bool" />
<param index="0" name="format" type="int" enum="RenderingDevice.DataFormat" />
<param index="1" name="sampler_filter" type="int" enum="RenderingDevice.SamplerFilter" />
<description>
Returns [code]true[/code] if implementation supports using a texture of [param format] with the given [param sampler_filter].
</description>
</method>
<method name="screen_get_framebuffer_format" qualifiers="const">
<return type="int" />
<description>
Expand Down
12 changes: 12 additions & 0 deletions drivers/vulkan/rendering_device_vulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4305,6 +4305,18 @@ RID RenderingDeviceVulkan::sampler_create(const SamplerState &p_state) {
return id;
}

bool RenderingDeviceVulkan::sampler_is_format_supported_for_filter(DataFormat p_format, SamplerFilter p_sampler_filter) const {
ERR_FAIL_INDEX_V(p_format, DATA_FORMAT_MAX, false);

_THREAD_SAFE_METHOD_

// Validate that this image is supported for the intended filtering.
VkFormatProperties properties;
vkGetPhysicalDeviceFormatProperties(context->get_physical_device(), vulkan_formats[p_format], &properties);

return p_sampler_filter == RD::SAMPLER_FILTER_NEAREST || (p_sampler_filter == RD::SAMPLER_FILTER_LINEAR && (properties.optimalTilingFeatures & VK_FORMAT_FEATURE_SAMPLED_IMAGE_FILTER_LINEAR_BIT));
}

/**********************/
/**** VERTEX ARRAY ****/
/**********************/
Expand Down
1 change: 1 addition & 0 deletions drivers/vulkan/rendering_device_vulkan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,7 @@ class RenderingDeviceVulkan : public RenderingDevice {
/*****************/

virtual RID sampler_create(const SamplerState &p_state);
virtual bool sampler_is_format_supported_for_filter(DataFormat p_format, SamplerFilter p_sampler_filter) const;

/**********************/
/**** VERTEX ARRAY ****/
Expand Down
45 changes: 27 additions & 18 deletions servers/rendering/renderer_rd/environment/fog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,15 +463,19 @@ Fog::VolumetricFog::~VolumetricFog() {
if (fog_uniform_set.is_valid() && RD::get_singleton()->uniform_set_is_valid(fog_uniform_set)) {
RD::get_singleton()->free(fog_uniform_set);
}
if (process_uniform_set_density.is_valid() && RD::get_singleton()->uniform_set_is_valid(process_uniform_set_density)) {
RD::get_singleton()->free(process_uniform_set_density);
}
if (process_uniform_set.is_valid() && RD::get_singleton()->uniform_set_is_valid(process_uniform_set)) {
RD::get_singleton()->free(process_uniform_set);
}
if (process_uniform_set2.is_valid() && RD::get_singleton()->uniform_set_is_valid(process_uniform_set2)) {
RD::get_singleton()->free(process_uniform_set2);

// At this point, due to cascade deletions, the sets may no longer be valid, but still they must work as a group.
gi_dependent_sets.valid = RD::get_singleton()->uniform_set_is_valid(gi_dependent_sets.process_uniform_set_density);
#ifdef DEV_ENABLED
gi_dependent_sets.assert_actual_validity();
#endif
if (gi_dependent_sets.valid) {
RD::get_singleton()->free(gi_dependent_sets.copy_uniform_set);
RD::get_singleton()->free(gi_dependent_sets.process_uniform_set_density);
RD::get_singleton()->free(gi_dependent_sets.process_uniform_set);
RD::get_singleton()->free(gi_dependent_sets.process_uniform_set2);
}

if (sdfgi_uniform_set.is_valid() && RD::get_singleton()->uniform_set_is_valid(sdfgi_uniform_set)) {
RD::get_singleton()->free(sdfgi_uniform_set);
}
Expand Down Expand Up @@ -713,7 +717,10 @@ void Fog::volumetric_fog_update(const VolumetricFogSettings &p_settings, const P
RD::get_singleton()->compute_list_end();
}

if (fog->process_uniform_set_density.is_null() || !RD::get_singleton()->uniform_set_is_valid(fog->process_uniform_set_density)) {
#ifdef DEV_ENABLED
fog->gi_dependent_sets.assert_actual_validity();
#endif
if (!fog->gi_dependent_sets.valid) {
//re create uniform set if needed
Vector<RD::Uniform> uniforms;
Vector<RD::Uniform> copy_uniforms;
Expand Down Expand Up @@ -910,21 +917,23 @@ void Fog::volumetric_fog_update(const VolumetricFogSettings &p_settings, const P
uniforms.push_back(u);
}

fog->copy_uniform_set = RD::get_singleton()->uniform_set_create(copy_uniforms, volumetric_fog.process_shader.version_get_shader(volumetric_fog.process_shader_version, VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_COPY), 0);
fog->gi_dependent_sets.copy_uniform_set = RD::get_singleton()->uniform_set_create(copy_uniforms, volumetric_fog.process_shader.version_get_shader(volumetric_fog.process_shader_version, VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_COPY), 0);

fog->process_uniform_set = RD::get_singleton()->uniform_set_create(uniforms, volumetric_fog.process_shader.version_get_shader(volumetric_fog.process_shader_version, VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_FOG), 0);
fog->gi_dependent_sets.process_uniform_set = RD::get_singleton()->uniform_set_create(uniforms, volumetric_fog.process_shader.version_get_shader(volumetric_fog.process_shader_version, VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_FOG), 0);

RID aux7 = uniforms.write[7].get_id(0);
RID aux8 = uniforms.write[8].get_id(0);

uniforms.write[7].set_id(0, aux8);
uniforms.write[8].set_id(0, aux7);

fog->process_uniform_set2 = RD::get_singleton()->uniform_set_create(uniforms, volumetric_fog.process_shader.version_get_shader(volumetric_fog.process_shader_version, VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_FOG), 0);
fog->gi_dependent_sets.process_uniform_set2 = RD::get_singleton()->uniform_set_create(uniforms, volumetric_fog.process_shader.version_get_shader(volumetric_fog.process_shader_version, VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_FOG), 0);

uniforms.remove_at(8);
uniforms.write[7].set_id(0, aux7);
fog->process_uniform_set_density = RD::get_singleton()->uniform_set_create(uniforms, volumetric_fog.process_shader.version_get_shader(volumetric_fog.process_shader_version, VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_DENSITY), 0);
fog->gi_dependent_sets.process_uniform_set_density = RD::get_singleton()->uniform_set_create(uniforms, volumetric_fog.process_shader.version_get_shader(volumetric_fog.process_shader_version, VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_DENSITY), 0);

fog->gi_dependent_sets.valid = true;
}

bool using_sdfgi = RendererSceneRenderRD::get_singleton()->environment_get_volumetric_fog_gi_inject(p_settings.env) > 0.0001 && RendererSceneRenderRD::get_singleton()->environment_get_sdfgi_enabled(p_settings.env) && (p_settings.sdfgi.is_valid());
Expand Down Expand Up @@ -1067,7 +1076,7 @@ void Fog::volumetric_fog_update(const VolumetricFogSettings &p_settings, const P

RD::get_singleton()->compute_list_bind_compute_pipeline(compute_list, volumetric_fog.process_pipelines[using_sdfgi ? VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_DENSITY_WITH_SDFGI : VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_DENSITY]);

RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->process_uniform_set_density, 0);
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->gi_dependent_sets.process_uniform_set_density, 0);

if (using_sdfgi) {
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->sdfgi_uniform_set, 1);
Expand All @@ -1078,7 +1087,7 @@ void Fog::volumetric_fog_update(const VolumetricFogSettings &p_settings, const P
// Copy fog to history buffer
if (RendererSceneRenderRD::get_singleton()->environment_get_volumetric_fog_temporal_reprojection(p_settings.env)) {
RD::get_singleton()->compute_list_bind_compute_pipeline(compute_list, volumetric_fog.process_pipelines[VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_COPY]);
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->copy_uniform_set, 0);
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->gi_dependent_sets.copy_uniform_set, 0);
RD::get_singleton()->compute_list_dispatch_threads(compute_list, fog->width, fog->height, fog->depth);
RD::get_singleton()->compute_list_add_barrier(compute_list);
}
Expand All @@ -1090,7 +1099,7 @@ void Fog::volumetric_fog_update(const VolumetricFogSettings &p_settings, const P
RENDER_TIMESTAMP("Filter Fog");

RD::get_singleton()->compute_list_bind_compute_pipeline(compute_list, volumetric_fog.process_pipelines[VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_FILTER]);
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->process_uniform_set, 0);
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->gi_dependent_sets.process_uniform_set, 0);
RD::get_singleton()->compute_list_dispatch_threads(compute_list, fog->width, fog->height, fog->depth);

RD::get_singleton()->compute_list_end();
Expand All @@ -1101,7 +1110,7 @@ void Fog::volumetric_fog_update(const VolumetricFogSettings &p_settings, const P

compute_list = RD::get_singleton()->compute_list_begin();
RD::get_singleton()->compute_list_bind_compute_pipeline(compute_list, volumetric_fog.process_pipelines[VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_FILTER]);
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->process_uniform_set2, 0);
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->gi_dependent_sets.process_uniform_set2, 0);
RD::get_singleton()->compute_list_dispatch_threads(compute_list, fog->width, fog->height, fog->depth);

RD::get_singleton()->compute_list_add_barrier(compute_list);
Expand All @@ -1112,7 +1121,7 @@ void Fog::volumetric_fog_update(const VolumetricFogSettings &p_settings, const P
RD::get_singleton()->draw_command_begin_label("Integrate Fog");

RD::get_singleton()->compute_list_bind_compute_pipeline(compute_list, volumetric_fog.process_pipelines[VolumetricFogShader::VOLUMETRIC_FOG_PROCESS_SHADER_FOG]);
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->process_uniform_set, 0);
RD::get_singleton()->compute_list_bind_uniform_set(compute_list, fog->gi_dependent_sets.process_uniform_set, 0);
RD::get_singleton()->compute_list_dispatch_threads(compute_list, fog->width, fog->height, 1);

RD::get_singleton()->compute_list_end(RD::BARRIER_MASK_RASTER);
Expand Down
23 changes: 19 additions & 4 deletions servers/rendering/renderer_rd/environment/fog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,25 @@ class Fog : public RendererFog {
RID emissive_map;

RID fog_uniform_set;
RID copy_uniform_set;
RID process_uniform_set_density;
RID process_uniform_set;
RID process_uniform_set2;

struct {
bool valid = false;
RID copy_uniform_set;
RID process_uniform_set_density;
RID process_uniform_set;
RID process_uniform_set2;

#ifdef DEV_ENABLED
void assert_actual_validity() {
// It's all-or-nothing, or something else has changed that requires dev attention.
DEV_ASSERT(valid == RD::get_singleton()->uniform_set_is_valid(copy_uniform_set));
DEV_ASSERT(valid == RD::get_singleton()->uniform_set_is_valid(process_uniform_set_density));
DEV_ASSERT(valid == RD::get_singleton()->uniform_set_is_valid(process_uniform_set));
DEV_ASSERT(valid == RD::get_singleton()->uniform_set_is_valid(process_uniform_set2));
}
#endif
} gi_dependent_sets;

RID sdfgi_uniform_set;
RID sky_uniform_set;

Expand Down
20 changes: 12 additions & 8 deletions servers/rendering/renderer_rd/environment/gi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3493,6 +3493,9 @@ void GI::init(SkyRD *p_sky) {
if (RendererSceneRenderRD::get_singleton()->is_vrs_supported()) {
defines += "\n#define USE_VRS\n";
}
if (!RD::get_singleton()->sampler_is_format_supported_for_filter(RD::DATA_FORMAT_R8G8_UINT, RD::SAMPLER_FILTER_LINEAR)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Specifying the format here feels too hard-coded to my taste, but I couldn't find a way of querying the format of the actual texture. This is not the end of the world, but has a risk of getting out of sync with the code that creates the texture.

defines += "\n#define SAMPLE_VOXEL_GI_NEAREST\n";
}

Vector<String> gi_modes;

Expand Down Expand Up @@ -3695,14 +3698,16 @@ void GI::setup_voxel_gi_instances(RenderDataRD *p_render_data, Ref<RenderSceneBu
if (p_render_buffers->has_custom_data(RB_SCOPE_FOG)) {
Copy link
Member Author

@RandomShaper RandomShaper Apr 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think this block of checks would belong better to just before using the uniform sets. They will be invalid by that time because of deletion of dependencies, so doing the check there is to me more correct. The idea would be being reactive to what actually happened instead of being proactive in guessing what may have happened. That, of course, without detriment of being proactive as an optimization in cases where that can save some checks (a bit like this PR does by assuming the whole group of sets is either valid or invalid).

In any case, this is a suggestion to the rendering maintainers. I didn't want to attempt something like that in this PR, which would broaden its scope needlessly.

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense. There isn't much of a reason to be caching this so early

Ref<Fog::VolumetricFog> fog = p_render_buffers->get_custom_data(RB_SCOPE_FOG);

if (RD::get_singleton()->uniform_set_is_valid(fog->fog_uniform_set)) {
RD::get_singleton()->free(fog->fog_uniform_set);
RD::get_singleton()->free(fog->process_uniform_set);
RD::get_singleton()->free(fog->process_uniform_set2);
#ifdef DEV_ENABLED
fog->gi_dependent_sets.assert_actual_validity();
#endif
if (fog->gi_dependent_sets.valid) {
RD::get_singleton()->free(fog->gi_dependent_sets.copy_uniform_set);
RD::get_singleton()->free(fog->gi_dependent_sets.process_uniform_set_density);
RD::get_singleton()->free(fog->gi_dependent_sets.process_uniform_set);
RD::get_singleton()->free(fog->gi_dependent_sets.process_uniform_set2);
fog->gi_dependent_sets.valid = false;
}
fog->fog_uniform_set = RID();
fog->process_uniform_set = RID();
fog->process_uniform_set2 = RID();
}
}

Expand Down Expand Up @@ -3929,7 +3934,6 @@ void GI::process_gi(Ref<RenderSceneBuffersRD> p_render_buffers, const RID *p_nor
u.append_id(material_storage->sampler_rd_get_default(RS::CANVAS_ITEM_TEXTURE_FILTER_LINEAR_WITH_MIPMAPS, RS::CANVAS_ITEM_TEXTURE_REPEAT_DISABLED));
uniforms.push_back(u);
}

{
RD::Uniform u;
u.uniform_type = RD::UNIFORM_TYPE_IMAGE;
Expand Down
8 changes: 8 additions & 0 deletions servers/rendering/renderer_rd/shaders/environment/gi.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

#VERSION_DEFINES

#ifdef SAMPLE_VOXEL_GI_NEAREST
#extension GL_EXT_samplerless_texture_functions : enable
#endif

layout(local_size_x = 8, local_size_y = 8, local_size_z = 1) in;

#define M_PI 3.141592
Expand Down Expand Up @@ -625,7 +629,11 @@ void process_gi(ivec2 pos, vec3 vertex, inout vec4 ambient_light, inout vec4 ref

#ifdef USE_VOXEL_GI_INSTANCES
{
#ifdef SAMPLE_VOXEL_GI_NEAREST
uvec2 voxel_gi_tex = texelFetch(voxel_gi_buffer, pos, 0).rg;
#else
uvec2 voxel_gi_tex = texelFetch(usampler2D(voxel_gi_buffer, linear_sampler), pos, 0).rg;
RandomShaper marked this conversation as resolved.
Show resolved Hide resolved
#endif
roughness *= roughness;
//find arbitrary tangent and bitangent, then build a matrix
vec3 v0 = abs(normal.z) < 0.999 ? vec3(0.0, 0.0, 1.0) : vec3(0.0, 1.0, 0.0);
Expand Down
1 change: 1 addition & 0 deletions servers/rendering/rendering_device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,7 @@ void RenderingDevice::_bind_methods() {
ClassDB::bind_method(D_METHOD("framebuffer_is_valid", "framebuffer"), &RenderingDevice::framebuffer_is_valid);

ClassDB::bind_method(D_METHOD("sampler_create", "state"), &RenderingDevice::_sampler_create);
ClassDB::bind_method(D_METHOD("sampler_is_format_supported_for_filter", "format", "sampler_filter"), &RenderingDevice::sampler_is_format_supported_for_filter);

ClassDB::bind_method(D_METHOD("vertex_buffer_create", "size_bytes", "data", "use_as_storage"), &RenderingDevice::vertex_buffer_create, DEFVAL(Vector<uint8_t>()), DEFVAL(false));
ClassDB::bind_method(D_METHOD("vertex_format_create", "vertex_descriptions"), &RenderingDevice::_vertex_format_create);
Expand Down
1 change: 1 addition & 0 deletions servers/rendering/rendering_device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ class RenderingDevice : public Object {
};

virtual RID sampler_create(const SamplerState &p_state) = 0;
virtual bool sampler_is_format_supported_for_filter(DataFormat p_format, SamplerFilter p_sampler_filter) const = 0;

/**********************/
/**** VERTEX ARRAY ****/
Expand Down