Skip to content

Commit

Permalink
Only allow valid types in Decal, Light3D projector and PointLight2D t…
Browse files Browse the repository at this point in the history
…exture

If an invalid type is supplied (which can still be done from a script),
a warning is printed (along with a workaround for ViewportTexture).

This also adds support for "negative" resource hints such as
"Texture2D,-ViewportTexture" to exclude one or more subclasses
from a class hint.

Co-authored-by: Tomasz Chabora <kobewi4e@gmail.com>
  • Loading branch information
Calinou and KoBeWi committed May 21, 2024
1 parent 2149682 commit d3344bf
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 18 deletions.
2 changes: 1 addition & 1 deletion core/object/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ enum PropertyHint {
PROPERTY_HINT_DIR, ///< a directory path must be passed
PROPERTY_HINT_GLOBAL_FILE, ///< a file path must be passed, hint_text (optionally) is a filter "*.png,*.wav,*.doc,"
PROPERTY_HINT_GLOBAL_DIR, ///< a directory path must be passed
PROPERTY_HINT_RESOURCE_TYPE, ///< a resource object type
PROPERTY_HINT_RESOURCE_TYPE, ///< a comma-separated resource object type, e.g. "NoiseTexture,GradientTexture2D". Subclasses can be excluded with a "-" prefix if placed *after* the base class, e.g. "Texture2D,-MeshTexture".
PROPERTY_HINT_MULTILINE_TEXT, ///< used for string properties that can contain multiple lines
PROPERTY_HINT_EXPRESSION, ///< used for string properties that can contain multiple lines
PROPERTY_HINT_PLACEHOLDER_TEXT, ///< used to set a placeholder text for string properties
Expand Down
2 changes: 1 addition & 1 deletion doc/classes/Texture2D.xml
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
<description>
Returns an [Image] that is a copy of data from this [Texture2D] (a new [Image] is created each time). [Image]s can be accessed and manipulated directly.
[b]Note:[/b] This will return [code]null[/code] if this [Texture2D] is invalid.
[b]Note:[/b] This will fetch the texture data from the GPU, which might cause performance problems when overused.
[b]Note:[/b] This will fetch the texture data from the GPU, which might cause performance problems when overused. Avoid calling [method get_image] every frame, especially on large textures.
</description>
</method>
<method name="get_size" qualifiers="const">
Expand Down
1 change: 1 addition & 0 deletions doc/classes/ViewportTexture.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
To get a [ViewportTexture] in code, use the [method Viewport.get_texture] method on the target viewport.
[b]Note:[/b] A [ViewportTexture] is always local to its scene (see [member Resource.resource_local_to_scene]). If the scene root is not ready, it may return incorrect data (see [signal Node.ready]).
[b]Note:[/b] Instantiating scenes containing a high-resolution [ViewportTexture] may cause noticeable stutter.
[b]Note:[/b] Some nodes such as [Decal], [Light3D], and [PointLight2D] do not support using [ViewportTexture] directly. To use texture data from a [ViewportTexture] in these nodes, you need to create an [ImageTexture] by calling [method Texture2D.get_image] on the [ViewportTexture] and passing the result to [method ImageTexture.create_from_image]. This conversion is a slow operation, so it should not be performed every frame.
</description>
<tutorials>
<link title="GUI in 3D Viewport Demo">https://godotengine.org/asset-library/asset/2807</link>
Expand Down
26 changes: 18 additions & 8 deletions editor/editor_resource_picker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -563,17 +563,17 @@ String EditorResourcePicker::_get_resource_type(const Ref<Resource> &p_resource)
return res_type;
}

static void _add_allowed_type(const StringName &p_type, HashSet<StringName> *p_vector) {
if (p_vector->has(p_type)) {
// Already added
static void _add_allowed_type(const StringName &p_type, List<StringName> *p_vector) {
if (p_vector->find(p_type)) {
// Already added.
return;
}

if (ClassDB::class_exists(p_type)) {
// Engine class,

if (!ClassDB::is_virtual(p_type)) {
p_vector->insert(p_type);
p_vector->push_back(p_type);
}

List<StringName> inheriters;
Expand All @@ -583,7 +583,7 @@ static void _add_allowed_type(const StringName &p_type, HashSet<StringName> *p_v
}
} else {
// Script class.
p_vector->insert(p_type);
p_vector->push_back(p_type);
}

List<StringName> inheriters;
Expand All @@ -598,12 +598,22 @@ void EditorResourcePicker::_ensure_allowed_types() const {
return;
}

List<StringName> final_allowed;

Vector<String> allowed_types = base_type.split(",");
int size = allowed_types.size();

for (int i = 0; i < size; i++) {
const String base = allowed_types[i].strip_edges();
_add_allowed_type(base, &allowed_types_without_convert);
for (const String &S : allowed_types) {
const String base = S.strip_edges();
if (base.begins_with("-")) {
final_allowed.erase(base.right(-1));
continue;
}
_add_allowed_type(base, &final_allowed);
}

for (const StringName &SN : final_allowed) {
allowed_types_without_convert.insert(SN);
}

allowed_types_with_convert = HashSet<StringName>(allowed_types_without_convert);
Expand Down
12 changes: 12 additions & 0 deletions misc/extension_api_validation/4.2-stable.expected
Original file line number Diff line number Diff line change
Expand Up @@ -357,3 +357,15 @@ Validate extension JSON: Error: Field 'classes/RenderingServer/methods/canvas_it
Validate extension JSON: Error: Field 'classes/RenderingServer/methods/canvas_item_add_rect/arguments': size changed value in new API, from 3 to 4.

Optional arguments added. Compatibility methods registered.


GH-88349
--------
Validate extension JSON: Error: Field 'classes/Decal/properties/texture_albedo': type changed value in new API, from "Texture2D" to "Texture2D,-AnimatedTexture,-AtlasTexture,-CameraTexture,-CanvasTexture,-MeshTexture,-Texture2DRD,-ViewportTexture".
Validate extension JSON: Error: Field 'classes/Decal/properties/texture_emission': type changed value in new API, from "Texture2D" to "Texture2D,-AnimatedTexture,-AtlasTexture,-CameraTexture,-CanvasTexture,-MeshTexture,-Texture2DRD,-ViewportTexture".
Validate extension JSON: Error: Field 'classes/Decal/properties/texture_normal': type changed value in new API, from "Texture2D" to "Texture2D,-AnimatedTexture,-AtlasTexture,-CameraTexture,-CanvasTexture,-MeshTexture,-Texture2DRD,-ViewportTexture".
Validate extension JSON: Error: Field 'classes/Decal/properties/texture_orm': type changed value in new API, from "Texture2D" to "Texture2D,-AnimatedTexture,-AtlasTexture,-CameraTexture,-CanvasTexture,-MeshTexture,-Texture2DRD,-ViewportTexture".
Validate extension JSON: Error: Field 'classes/Light3D/properties/light_projector': type changed value in new API, from "Texture2D" to "Texture2D,-AnimatedTexture,-AtlasTexture,-CameraTexture,-CanvasTexture,-MeshTexture,-Texture2DRD,-ViewportTexture".
Validate extension JSON: Error: Field 'classes/PointLight2D/properties/texture': type changed value in new API, from "Texture2D" to "Texture2D,-AnimatedTexture,-AtlasTexture,-CameraTexture,-CanvasTexture,-MeshTexture,-Texture2DRD,-ViewportTexture".

Property hints modified to disallow resource types that don't work. The types allowed are now more restricted, but this change only impacts the editor and not the actual exposed API. No adjustments should be necessary.
16 changes: 15 additions & 1 deletion scene/2d/light_2d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,19 @@ Rect2 PointLight2D::get_anchorable_rect() const {
void PointLight2D::set_texture(const Ref<Texture2D> &p_texture) {
texture = p_texture;
if (texture.is_valid()) {
#ifdef DEBUG_ENABLED
if (
p_texture->is_class("AnimatedTexture") ||
p_texture->is_class("AtlasTexture") ||
p_texture->is_class("CameraTexture") ||
p_texture->is_class("CanvasTexture") ||
p_texture->is_class("MeshTexture") ||
p_texture->is_class("Texture2DRD") ||
p_texture->is_class("ViewportTexture")) {
WARN_PRINT(vformat("%s cannot be used as a PointLight2D texture (%s). As a workaround, assign the value returned by %s's `get_image()` instead.", p_texture->get_class(), get_path(), p_texture->get_class()));
}
#endif

RS::get_singleton()->canvas_light_set_texture(_get_light(), texture->get_rid());
} else {
RS::get_singleton()->canvas_light_set_texture(_get_light(), RID());
Expand Down Expand Up @@ -461,7 +474,8 @@ void PointLight2D::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_texture_scale", "texture_scale"), &PointLight2D::set_texture_scale);
ClassDB::bind_method(D_METHOD("get_texture_scale"), &PointLight2D::get_texture_scale);

ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "texture", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), "set_texture", "get_texture");
// Only allow texture types that display correctly.
ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "texture", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D,-AnimatedTexture,-AtlasTexture,-CameraTexture,-CanvasTexture,-MeshTexture,-Texture2DRD,-ViewportTexture"), "set_texture", "get_texture");
ADD_PROPERTY(PropertyInfo(Variant::VECTOR2, "offset", PROPERTY_HINT_NONE, "suffix:px"), "set_texture_offset", "get_texture_offset");
ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "texture_scale", PROPERTY_HINT_RANGE, "0.01,50,0.01"), "set_texture_scale", "get_texture_scale");
ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "height", PROPERTY_HINT_RANGE, "0,1024,1,or_greater,suffix:px"), "set_height", "get_height");
Expand Down
24 changes: 20 additions & 4 deletions scene/3d/decal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ void Decal::set_texture(DecalTexture p_type, const Ref<Texture2D> &p_texture) {
ERR_FAIL_INDEX(p_type, TEXTURE_MAX);
textures[p_type] = p_texture;
RID texture_rid = p_texture.is_valid() ? p_texture->get_rid() : RID();

#ifdef DEBUG_ENABLED
if (
p_texture->is_class("AnimatedTexture") ||
p_texture->is_class("AtlasTexture") ||
p_texture->is_class("CameraTexture") ||
p_texture->is_class("CanvasTexture") ||
p_texture->is_class("MeshTexture") ||
p_texture->is_class("Texture2DRD") ||
p_texture->is_class("ViewportTexture")) {
WARN_PRINT(vformat("%s cannot be used as a Decal texture (%s). As a workaround, assign the value returned by %s's `get_image()` instead.", p_texture->get_class(), get_path(), p_texture->get_class()));
}
#endif

RS::get_singleton()->decal_set_texture(decal, RS::DecalTexture(p_type), texture_rid);
update_configuration_warnings();
}
Expand Down Expand Up @@ -225,10 +239,12 @@ void Decal::_bind_methods() {
ADD_PROPERTY(PropertyInfo(Variant::VECTOR3, "size", PROPERTY_HINT_RANGE, "0,1024,0.001,or_greater,suffix:m"), "set_size", "get_size");

ADD_GROUP("Textures", "texture_");
ADD_PROPERTYI(PropertyInfo(Variant::OBJECT, "texture_albedo", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), "set_texture", "get_texture", TEXTURE_ALBEDO);
ADD_PROPERTYI(PropertyInfo(Variant::OBJECT, "texture_normal", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), "set_texture", "get_texture", TEXTURE_NORMAL);
ADD_PROPERTYI(PropertyInfo(Variant::OBJECT, "texture_orm", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), "set_texture", "get_texture", TEXTURE_ORM);
ADD_PROPERTYI(PropertyInfo(Variant::OBJECT, "texture_emission", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), "set_texture", "get_texture", TEXTURE_EMISSION);
// Only allow texture types that display correctly.
const String texture_hint = "Texture2D,-AnimatedTexture,-AtlasTexture,-CameraTexture,-CanvasTexture,-MeshTexture,-Texture2DRD,-ViewportTexture";
ADD_PROPERTYI(PropertyInfo(Variant::OBJECT, "texture_albedo", PROPERTY_HINT_RESOURCE_TYPE, texture_hint), "set_texture", "get_texture", TEXTURE_ALBEDO);
ADD_PROPERTYI(PropertyInfo(Variant::OBJECT, "texture_normal", PROPERTY_HINT_RESOURCE_TYPE, texture_hint), "set_texture", "get_texture", TEXTURE_NORMAL);
ADD_PROPERTYI(PropertyInfo(Variant::OBJECT, "texture_orm", PROPERTY_HINT_RESOURCE_TYPE, texture_hint), "set_texture", "get_texture", TEXTURE_ORM);
ADD_PROPERTYI(PropertyInfo(Variant::OBJECT, "texture_emission", PROPERTY_HINT_RESOURCE_TYPE, texture_hint), "set_texture", "get_texture", TEXTURE_EMISSION);

ADD_GROUP("Parameters", "");
ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "emission_energy", PROPERTY_HINT_RANGE, "0,16,0.01,or_greater"), "set_emission_energy", "get_emission_energy");
Expand Down
21 changes: 18 additions & 3 deletions scene/3d/light_3d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@
/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */
/**************************************************************************/

#include "core/config/project_settings.h"

#include "light_3d.h"

#include "core/config/project_settings.h"

void Light3D::set_param(Param p_param, real_t p_value) {
ERR_FAIL_INDEX(p_param, PARAM_MAX);
param[p_param] = p_value;
Expand Down Expand Up @@ -191,6 +191,20 @@ Light3D::BakeMode Light3D::get_bake_mode() const {
void Light3D::set_projector(const Ref<Texture2D> &p_texture) {
projector = p_texture;
RID tex_id = projector.is_valid() ? projector->get_rid() : RID();

#ifdef DEBUG_ENABLED
if (
p_texture->is_class("AnimatedTexture") ||
p_texture->is_class("AtlasTexture") ||
p_texture->is_class("CameraTexture") ||
p_texture->is_class("CanvasTexture") ||
p_texture->is_class("MeshTexture") ||
p_texture->is_class("Texture2DRD") ||
p_texture->is_class("ViewportTexture")) {
WARN_PRINT(vformat("%s cannot be used as a Light3D projector texture (%s). As a workaround, assign the value returned by %s's `get_image()` instead.", p_texture->get_class(), get_path(), p_texture->get_class()));
}
#endif

RS::get_singleton()->light_set_projector(light, tex_id);
update_configuration_warnings();
}
Expand Down Expand Up @@ -372,7 +386,8 @@ void Light3D::_bind_methods() {
ADD_PROPERTYI(PropertyInfo(Variant::FLOAT, "light_energy", PROPERTY_HINT_RANGE, "0,16,0.001,or_greater"), "set_param", "get_param", PARAM_ENERGY);
ADD_PROPERTYI(PropertyInfo(Variant::FLOAT, "light_indirect_energy", PROPERTY_HINT_RANGE, "0,16,0.001,or_greater"), "set_param", "get_param", PARAM_INDIRECT_ENERGY);
ADD_PROPERTYI(PropertyInfo(Variant::FLOAT, "light_volumetric_fog_energy", PROPERTY_HINT_RANGE, "0,16,0.001,or_greater"), "set_param", "get_param", PARAM_VOLUMETRIC_FOG_ENERGY);
ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "light_projector", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D"), "set_projector", "get_projector");
// Only allow texture types that display correctly.
ADD_PROPERTY(PropertyInfo(Variant::OBJECT, "light_projector", PROPERTY_HINT_RESOURCE_TYPE, "Texture2D,-AnimatedTexture,-AtlasTexture,-CameraTexture,-CanvasTexture,-MeshTexture,-Texture2DRD,-ViewportTexture"), "set_projector", "get_projector");
ADD_PROPERTYI(PropertyInfo(Variant::FLOAT, "light_size", PROPERTY_HINT_RANGE, "0,1,0.001,or_greater,suffix:m"), "set_param", "get_param", PARAM_SIZE);
ADD_PROPERTYI(PropertyInfo(Variant::FLOAT, "light_angular_distance", PROPERTY_HINT_RANGE, "0,90,0.01,degrees"), "set_param", "get_param", PARAM_SIZE);
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "light_negative"), "set_negative", "is_negative");
Expand Down

0 comments on commit d3344bf

Please sign in to comment.