From 34ac0387d6b8bf20da7bf38236c1a29721a6bfc2 Mon Sep 17 00:00:00 2001 From: Hugo Locurcio Date: Sun, 23 Aug 2020 16:18:33 +0200 Subject: [PATCH] Deprecate the GIProbe Compress property due to known bugs This property introduced green artifacts in GI and didn't improve performance. The new GIProbe in `master` no longer has a Compress property. --- doc/classes/GIProbe.xml | 2 +- drivers/dummy/rasterizer_dummy.h | 1 - drivers/gles2/rasterizer_storage_gles2.cpp | 4 --- drivers/gles2/rasterizer_storage_gles2.h | 1 - drivers/gles3/rasterizer_storage_gles3.cpp | 29 ++++++---------------- drivers/gles3/rasterizer_storage_gles3.h | 1 - scene/3d/gi_probe.cpp | 10 ++++++++ servers/visual/rasterizer.h | 1 - servers/visual/visual_server_scene.cpp | 4 +-- 9 files changed, 19 insertions(+), 34 deletions(-) diff --git a/doc/classes/GIProbe.xml b/doc/classes/GIProbe.xml index f3fb199a509..aed4e8cba50 100644 --- a/doc/classes/GIProbe.xml +++ b/doc/classes/GIProbe.xml @@ -36,7 +36,7 @@ [b]Note:[/b] [code]bias[/code] should usually be above 1.0 as that is the size of the voxels. - If [code]true[/code], the data for this [GIProbe] will be compressed. Compression saves space, but results in far worse visual quality. + [i]Deprecated.[/i] This property has been deprecated due to known bugs and no longer has any effect when enabled. The [GIProbeData] resource that holds the data for this [GIProbe]. diff --git a/drivers/dummy/rasterizer_dummy.h b/drivers/dummy/rasterizer_dummy.h index 2a5bb2bd5fa..c23c8730154 100644 --- a/drivers/dummy/rasterizer_dummy.h +++ b/drivers/dummy/rasterizer_dummy.h @@ -587,7 +587,6 @@ public: uint32_t gi_probe_get_version(RID p_probe) { return 0; } - GIProbeCompression gi_probe_get_dynamic_data_get_preferred_compression() const { return GI_PROBE_UNCOMPRESSED; } RID gi_probe_dynamic_data_create(int p_width, int p_height, int p_depth, GIProbeCompression p_compression) { return RID(); } void gi_probe_dynamic_data_update(RID p_gi_probe_data, int p_depth_slice, int p_slice_count, int p_mipmap, const void *p_data) {} diff --git a/drivers/gles2/rasterizer_storage_gles2.cpp b/drivers/gles2/rasterizer_storage_gles2.cpp index c211f6377b0..905763bbb21 100644 --- a/drivers/gles2/rasterizer_storage_gles2.cpp +++ b/drivers/gles2/rasterizer_storage_gles2.cpp @@ -4373,10 +4373,6 @@ uint32_t RasterizerStorageGLES2::gi_probe_get_version(RID p_probe) { return 0; } -RasterizerStorage::GIProbeCompression RasterizerStorageGLES2::gi_probe_get_dynamic_data_get_preferred_compression() const { - return GI_PROBE_UNCOMPRESSED; -} - RID RasterizerStorageGLES2::gi_probe_dynamic_data_create(int p_width, int p_height, int p_depth, GIProbeCompression p_compression) { return RID(); } diff --git a/drivers/gles2/rasterizer_storage_gles2.h b/drivers/gles2/rasterizer_storage_gles2.h index c814cd38675..e9f2132f8c5 100644 --- a/drivers/gles2/rasterizer_storage_gles2.h +++ b/drivers/gles2/rasterizer_storage_gles2.h @@ -1074,7 +1074,6 @@ public: virtual uint32_t gi_probe_get_version(RID p_probe); - virtual GIProbeCompression gi_probe_get_dynamic_data_get_preferred_compression() const; virtual RID gi_probe_dynamic_data_create(int p_width, int p_height, int p_depth, GIProbeCompression p_compression); virtual void gi_probe_dynamic_data_update(RID p_gi_probe_data, int p_depth_slice, int p_slice_count, int p_mipmap, const void *p_data); diff --git a/drivers/gles3/rasterizer_storage_gles3.cpp b/drivers/gles3/rasterizer_storage_gles3.cpp index 296be971cc1..fa314a23021 100644 --- a/drivers/gles3/rasterizer_storage_gles3.cpp +++ b/drivers/gles3/rasterizer_storage_gles3.cpp @@ -6106,6 +6106,10 @@ bool RasterizerStorageGLES3::gi_probe_is_interior(RID p_probe) const { void RasterizerStorageGLES3::gi_probe_set_compress(RID p_probe, bool p_enable) { + if (p_enable) { + WARN_DEPRECATED_MSG("GIProbe's Compress property has been deprecated due to known bugs and will be removed in Godot 4.0."); + } + GIProbe *gip = gi_probe_owner.getornull(p_probe); ERR_FAIL_COND(!gip); @@ -6159,14 +6163,6 @@ uint32_t RasterizerStorageGLES3::gi_probe_get_version(RID p_probe) { return gip->version; } -RasterizerStorage::GIProbeCompression RasterizerStorageGLES3::gi_probe_get_dynamic_data_get_preferred_compression() const { - if (config.s3tc_supported) { - return GI_PROBE_S3TC; - } else { - return GI_PROBE_UNCOMPRESSED; - } -} - RID RasterizerStorageGLES3::gi_probe_dynamic_data_create(int p_width, int p_height, int p_depth, GIProbeCompression p_compression) { GIProbeData *gipd = memnew(GIProbeData); @@ -6174,7 +6170,7 @@ RID RasterizerStorageGLES3::gi_probe_dynamic_data_create(int p_width, int p_heig gipd->width = p_width; gipd->height = p_height; gipd->depth = p_depth; - gipd->compression = p_compression; + gipd->compression = GI_PROBE_UNCOMPRESSED; glActiveTexture(GL_TEXTURE0); glGenTextures(1, &gipd->tex_id); @@ -6188,13 +6184,7 @@ RID RasterizerStorageGLES3::gi_probe_dynamic_data_create(int p_width, int p_heig } while (true) { - - if (gipd->compression == GI_PROBE_S3TC) { - int size = p_width * p_height * p_depth; - glCompressedTexImage3D(GL_TEXTURE_3D, level, _EXT_COMPRESSED_RGBA_S3TC_DXT5_EXT, p_width, p_height, p_depth, 0, size, NULL); - } else { - glTexImage3D(GL_TEXTURE_3D, level, GL_RGBA8, p_width, p_height, p_depth, 0, GL_RGBA, GL_UNSIGNED_BYTE, NULL); - } + glTexImage3D(GL_TEXTURE_3D, level, GL_RGBA8, p_width, p_height, p_depth, 0, GL_RGBA, GL_UNSIGNED_BYTE, NULL); if (p_width <= min_size || p_height <= min_size || p_depth <= min_size) break; @@ -6241,12 +6231,7 @@ void RasterizerStorageGLES3::gi_probe_dynamic_data_update(RID p_gi_probe_data, i */ glActiveTexture(GL_TEXTURE0); glBindTexture(GL_TEXTURE_3D, gipd->tex_id); - if (gipd->compression == GI_PROBE_S3TC) { - int size = (gipd->width >> p_mipmap) * (gipd->height >> p_mipmap) * p_slice_count; - glCompressedTexSubImage3D(GL_TEXTURE_3D, p_mipmap, 0, 0, p_depth_slice, gipd->width >> p_mipmap, gipd->height >> p_mipmap, p_slice_count, _EXT_COMPRESSED_RGBA_S3TC_DXT5_EXT, size, p_data); - } else { - glTexSubImage3D(GL_TEXTURE_3D, p_mipmap, 0, 0, p_depth_slice, gipd->width >> p_mipmap, gipd->height >> p_mipmap, p_slice_count, GL_RGBA, GL_UNSIGNED_BYTE, p_data); - } + glTexSubImage3D(GL_TEXTURE_3D, p_mipmap, 0, 0, p_depth_slice, gipd->width >> p_mipmap, gipd->height >> p_mipmap, p_slice_count, GL_RGBA, GL_UNSIGNED_BYTE, p_data); //glTexImage3D(GL_TEXTURE_3D,p_mipmap,GL_RGBA8,gipd->width>>p_mipmap,gipd->height>>p_mipmap,gipd->depth>>p_mipmap,0,GL_RGBA,GL_UNSIGNED_BYTE,p_data); //glTexImage3D(GL_TEXTURE_3D,p_mipmap,GL_RGBA8,gipd->width>>p_mipmap,gipd->height>>p_mipmap,gipd->depth>>p_mipmap,0,GL_RGBA,GL_UNSIGNED_BYTE,data.ptr()); } diff --git a/drivers/gles3/rasterizer_storage_gles3.h b/drivers/gles3/rasterizer_storage_gles3.h index 86c97d82126..02cf36b51e3 100644 --- a/drivers/gles3/rasterizer_storage_gles3.h +++ b/drivers/gles3/rasterizer_storage_gles3.h @@ -1110,7 +1110,6 @@ public: mutable RID_Owner gi_probe_data_owner; - virtual GIProbeCompression gi_probe_get_dynamic_data_get_preferred_compression() const; virtual RID gi_probe_dynamic_data_create(int p_width, int p_height, int p_depth, GIProbeCompression p_compression); virtual void gi_probe_dynamic_data_update(RID p_gi_probe_data, int p_depth_slice, int p_slice_count, int p_mipmap, const void *p_data); diff --git a/scene/3d/gi_probe.cpp b/scene/3d/gi_probe.cpp index d020d198037..cf1703da2f0 100644 --- a/scene/3d/gi_probe.cpp +++ b/scene/3d/gi_probe.cpp @@ -327,6 +327,8 @@ void GIProbe::set_compress(bool p_enable) { if (probe_data.is_valid()) { probe_data->set_compress(p_enable); } + + update_configuration_warning(); } bool GIProbe::is_compressed() const { @@ -497,6 +499,14 @@ String GIProbe::get_configuration_warning() const { } warning += TTR("GIProbes are not supported by the GLES2 video driver.\nUse a BakedLightmap instead."); } + + if (is_compressed()) { + if (warning != String()) { + warning += "\n\n"; + } + warning += TTR("The GIProbe Compress property has been deprecated due to known bugs and no longer has any effect.\nTo remove this warning, disable the GIProbe's Compress property."); + } + return warning; } diff --git a/servers/visual/rasterizer.h b/servers/visual/rasterizer.h index f1d1716fb1e..7a1d1c25150 100644 --- a/servers/visual/rasterizer.h +++ b/servers/visual/rasterizer.h @@ -480,7 +480,6 @@ public: GI_PROBE_ETC2 }; - virtual GIProbeCompression gi_probe_get_dynamic_data_get_preferred_compression() const = 0; virtual RID gi_probe_dynamic_data_create(int p_width, int p_height, int p_depth, GIProbeCompression p_compression) = 0; virtual void gi_probe_dynamic_data_update(RID p_gi_probe_data, int p_depth_slice, int p_slice_count, int p_mipmap, const void *p_data) = 0; diff --git a/servers/visual/visual_server_scene.cpp b/servers/visual/visual_server_scene.cpp index f8756490a7d..468022ed858 100644 --- a/servers/visual/visual_server_scene.cpp +++ b/servers/visual/visual_server_scene.cpp @@ -2377,9 +2377,7 @@ void VisualServerScene::_setup_gi_probe(Instance *p_instance) { _gi_probe_fill_local_data(0, 0, 0, 0, 0, cells, header, ldw.ptr(), probe->dynamic.level_cell_lists.ptrw()); - bool compress = VSG::storage->gi_probe_is_compressed(p_instance->base); - - probe->dynamic.compression = compress ? VSG::storage->gi_probe_get_dynamic_data_get_preferred_compression() : RasterizerStorage::GI_PROBE_UNCOMPRESSED; + probe->dynamic.compression = RasterizerStorage::GI_PROBE_UNCOMPRESSED; probe->dynamic.probe_data = VSG::storage->gi_probe_dynamic_data_create(header->width, header->height, header->depth, probe->dynamic.compression);