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

[3.x] Backport CanvasLayer visibility #57900

Merged
merged 1 commit into from
Feb 10, 2022
Merged

[3.x] Backport CanvasLayer visibility #57900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions doc/classes/CanvasLayer.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,18 @@
<member name="transform" type="Transform2D" setter="set_transform" getter="get_transform" default="Transform2D( 1, 0, 0, 1, 0, 0 )">
The layer's transform.
</member>
<member name="visible" type="bool" setter="set_visible" getter="is_visible" default="true">
If [code]false[/code], any [CanvasItem] under this [CanvasLayer] will be hidden.
Unlike [member CanvasItem.visible], visibility of a [CanvasLayer] isn't propagated to underlying layers.
</member>
</members>
<signals>
<signal name="visibility_changed">
<description>
Emitted when visibility of the layer is changed. See [member visible].
</description>
</signal>
</signals>
<constants>
</constants>
</class>
16 changes: 15 additions & 1 deletion editor/scene_tree_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,17 @@ bool SceneTreeEditor::_add_nodes(Node *p_node, TreeItem *p_parent, bool p_scroll
}

_update_visibility_color(p_node, item);
} else if (p_node->is_class("CanvasLayer")) {
bool v = p_node->call("is_visible");
if (v) {
item->add_button(0, get_icon("GuiVisibilityVisible", "EditorIcons"), BUTTON_VISIBILITY, false, TTR("Toggle Visibility"));
} else {
item->add_button(0, get_icon("GuiVisibilityHidden", "EditorIcons"), BUTTON_VISIBILITY, false, TTR("Toggle Visibility"));
}

if (!p_node->is_connected("visibility_changed", this, "_node_visibility_changed")) {
p_node->connect("visibility_changed", this, "_node_visibility_changed", varray(p_node));
}
} else if (p_node->is_class("Spatial")) {
bool is_locked = p_node->has_meta("_edit_lock_");
if (is_locked) {
Expand Down Expand Up @@ -470,6 +481,9 @@ void SceneTreeEditor::_node_visibility_changed(Node *p_node) {
if (p_node->is_class("CanvasItem")) {
visible = p_node->call("is_visible");
CanvasItemEditor::get_singleton()->get_viewport_control()->update();
} else if (p_node->is_class("CanvasLayer")) {
visible = p_node->call("is_visible");
CanvasItemEditor::get_singleton()->get_viewport_control()->update();
} else if (p_node->is_class("Spatial")) {
visible = p_node->call("is_visible");
}
Expand Down Expand Up @@ -528,7 +542,7 @@ void SceneTreeEditor::_node_removed(Node *p_node) {
p_node->disconnect("script_changed", this, "_node_script_changed");
}

if (p_node->is_class("Spatial") || p_node->is_class("CanvasItem")) {
if (p_node->is_class("Spatial") || p_node->is_class("CanvasItem") || p_node->is_class("CanvasLayer")) {
if (p_node->is_connected("visibility_changed", this, "_node_visibility_changed")) {
p_node->disconnect("visibility_changed", this, "_node_visibility_changed");
}
Expand Down
85 changes: 35 additions & 50 deletions scene/2d/canvas_item.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,31 +350,19 @@ Transform2D CanvasItem::_edit_get_transform() const {
#endif

bool CanvasItem::is_visible_in_tree() const {
if (!is_inside_tree()) {
return false;
}

const CanvasItem *p = this;

while (p) {
if (!p->visible) {
return false;
}
p = p->get_parent_item();
}

return true;
return visible && parent_visible_in_tree;
}
Copy link
Member

Choose a reason for hiding this comment

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

Why is the new variable called visible_in_tree, if it is not whether the item is visible in the tree? Should this be parent_visible_in_tree?

Copy link
Member

Choose a reason for hiding this comment

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

In 4.0 it can depend on Window, which might not be a direct parent.

Copy link
Member

@lawnjelly lawnjelly Feb 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Could there be a better name for this? It strikes me as confusing.

In a sense with a window too, it is still the parent. Because the window is similar to a parent, and if the window isn't a direct parent, then the effects of the window are mediated by the direct parent?

At least that seems better than calling it something which it isn't, which is the case at the moment.

Essentially if I'm understanding correctly the logic is:

IF MY PARENT IS VISIBLE IN THE TREE and I'M VISIBLE then I'M VISIBLE IN THE TREE

Copy link
Member Author

Choose a reason for hiding this comment

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

Does ancestors_visible_in_tree make sense?

Copy link
Member

Choose a reason for hiding this comment

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

It's much better than is_visible_in_tree yes. Personally I think to stretch to using parent is fine here too, but we can see what others think. 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, the classref uses antecedents.

if the node is present in the SceneTree, its visible property is true and all its antecedents are also visible

Copy link
Member

@lawnjelly lawnjelly Feb 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure about antecedent - it is not really a commonly used English word, and most people would have to look it up to find the definition (including me as a native speaker 😀 ).

You could also use parent_and_window_visible_in_tree potentially if worried about the window thing (not so relevant in 3.x but no problem having this carryover). Ancestors is fine too, to me it's a bit overkill because if your parent is visible in the tree, then by definition the ancestors are also visible.

But yeah ancestors is fine if you are struggling and don't want to use parent.

They do say naming things is one of the most difficult problems in computer science lol.

Copy link
Member

Choose a reason for hiding this comment

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

Either way, changing is_visible_in_tree() would break compatibility in 3.x.

Copy link
Member

Choose a reason for hiding this comment

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

The discussion is about variable name, which is the same as the method name, but does different thing.

Copy link
Member

Choose a reason for hiding this comment

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

Then parent_visible_in_tree is what it tracks I think? There's no Window visibility involved in 3.x.


void CanvasItem::_propagate_visibility_changed(bool p_visible) {
void CanvasItem::_propagate_visibility_changed(bool p_visible, bool p_was_visible) {
if (p_visible && first_draw) { //avoid propagating it twice
first_draw = false;
}
parent_visible_in_tree = p_visible;
notification(NOTIFICATION_VISIBILITY_CHANGED);

if (p_visible) {
update(); //todo optimize
} else {
if (visible && p_visible) {
update();
} else if (!p_visible && (visible || p_was_visible)) {
emit_signal(SceneStringNames::get_singleton()->hide);
}
_block();
Expand All @@ -383,43 +371,39 @@ void CanvasItem::_propagate_visibility_changed(bool p_visible) {
CanvasItem *c = Object::cast_to<CanvasItem>(get_child(i));

if (c && c->visible) { //should the toplevels stop propagation? i think so but..
c->_propagate_visibility_changed(p_visible);
c->_propagate_visibility_changed(p_visible, !p_visible);
}
}

_unblock();
}

void CanvasItem::show() {
if (visible) {
void CanvasItem::set_visible(bool p_visible) {
if (visible == p_visible) {
return;
}

visible = true;
VisualServer::get_singleton()->canvas_item_set_visible(canvas_item, true);
visible = p_visible;
VisualServer::get_singleton()->canvas_item_set_visible(canvas_item, p_visible);

if (!is_inside_tree()) {
return;
}

_propagate_visibility_changed(true);
_propagate_visibility_changed(p_visible, !p_visible);
_change_notify("visible");
}

void CanvasItem::hide() {
if (!visible) {
return;
}

visible = false;
VisualServer::get_singleton()->canvas_item_set_visible(canvas_item, false);
void CanvasItem::show() {
set_visible(true);
}

if (!is_inside_tree()) {
return;
}
void CanvasItem::hide() {
set_visible(false);
}

_propagate_visibility_changed(false);
_change_notify("visible");
bool CanvasItem::is_visible() const {
return visible;
}

CanvasItem *CanvasItem::current_item_drawn = nullptr;
Expand All @@ -435,7 +419,7 @@ void CanvasItem::_update_callback() {

VisualServer::get_singleton()->canvas_item_clear(get_canvas_item());
//todo updating = true - only allow drawing here
if (is_visible_in_tree()) { //todo optimize this!!
if (is_visible_in_tree()) {
if (first_draw) {
notification(NOTIFICATION_VISIBILITY_CHANGED);
first_draw = false;
Expand Down Expand Up @@ -556,10 +540,20 @@ void CanvasItem::_notification(int p_what) {
case NOTIFICATION_ENTER_TREE: {
ERR_FAIL_COND(!is_inside_tree());
first_draw = true;
if (get_parent()) {
CanvasItem *ci = Object::cast_to<CanvasItem>(get_parent());

Node *parent = get_parent();
if (parent) {
CanvasItem *ci = Object::cast_to<CanvasItem>(parent);
if (ci) {
parent_visible_in_tree = ci->is_visible_in_tree();
C = ci->children_items.push_back(this);
} else {
CanvasLayer *cl = Object::cast_to<CanvasLayer>(parent);
if (cl) {
parent_visible_in_tree = cl->is_visible();
} else {
parent_visible_in_tree = true;
}
}
}
_enter_canvas();
Expand Down Expand Up @@ -591,6 +585,7 @@ void CanvasItem::_notification(int p_what) {
C = nullptr;
}
global_invalid = true;
parent_visible_in_tree = false;
} break;
case NOTIFICATION_DRAW:
case NOTIFICATION_TRANSFORM_CHANGED: {
Expand All @@ -601,17 +596,6 @@ void CanvasItem::_notification(int p_what) {
}
}

void CanvasItem::set_visible(bool p_visible) {
if (p_visible) {
show();
} else {
hide();
}
}
bool CanvasItem::is_visible() const {
return visible;
}

void CanvasItem::update() {
if (!is_inside_tree()) {
return;
Expand Down Expand Up @@ -1267,6 +1251,7 @@ CanvasItem::CanvasItem() :
xform_change(this) {
canvas_item = RID_PRIME(VisualServer::get_singleton()->canvas_item_create());
visible = true;
parent_visible_in_tree = false;
pending_update = false;
modulate = Color(1, 1, 1, 1);
self_modulate = Color(1, 1, 1, 1);
Expand Down
5 changes: 4 additions & 1 deletion scene/2d/canvas_item.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ VARIANT_ENUM_CAST(CanvasItemMaterial::LightMode)
class CanvasItem : public Node {
GDCLASS(CanvasItem, Node);

friend class CanvasLayer;

public:
enum BlendMode {

Expand Down Expand Up @@ -191,6 +193,7 @@ class CanvasItem : public Node {

bool first_draw;
bool visible;
bool parent_visible_in_tree;
bool pending_update;
bool toplevel;
bool drawing;
Expand All @@ -207,7 +210,7 @@ class CanvasItem : public Node {

void _toplevel_raise_self();

void _propagate_visibility_changed(bool p_visible);
void _propagate_visibility_changed(bool p_visible, bool p_was_visible = false);

void _update_callback();

Expand Down
34 changes: 34 additions & 0 deletions scene/main/canvas_layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
/*************************************************************************/

#include "canvas_layer.h"
#include "scene/2d/canvas_item.h"
#include "viewport.h"

void CanvasLayer::set_layer(int p_xform) {
Expand All @@ -42,6 +43,32 @@ int CanvasLayer::get_layer() const {
return layer;
}

void CanvasLayer::set_visible(bool p_visible) {
if (p_visible == visible) {
return;
}

visible = p_visible;
emit_signal("visibility_changed");

for (int i = 0; i < get_child_count(); i++) {
CanvasItem *c = Object::cast_to<CanvasItem>(get_child(i));
if (c) {
VisualServer::get_singleton()->canvas_item_set_visible(c->get_canvas_item(), p_visible && c->is_visible());

if (c->is_visible()) {
c->_propagate_visibility_changed(p_visible);
} else {
c->notification(CanvasItem::NOTIFICATION_VISIBILITY_CHANGED);
}
}
}
}

bool CanvasLayer::is_visible() const {
return visible;
}

void CanvasLayer::set_transform(const Transform2D &p_xform) {
transform = p_xform;
locrotscale_dirty = true;
Expand Down Expand Up @@ -265,6 +292,9 @@ void CanvasLayer::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_layer", "layer"), &CanvasLayer::set_layer);
ClassDB::bind_method(D_METHOD("get_layer"), &CanvasLayer::get_layer);

ClassDB::bind_method(D_METHOD("set_visible", "visible"), &CanvasLayer::set_visible);
ClassDB::bind_method(D_METHOD("is_visible"), &CanvasLayer::is_visible);

ClassDB::bind_method(D_METHOD("set_transform", "transform"), &CanvasLayer::set_transform);
ClassDB::bind_method(D_METHOD("get_transform"), &CanvasLayer::get_transform);

Expand Down Expand Up @@ -293,6 +323,7 @@ void CanvasLayer::_bind_methods() {

ADD_GROUP("Layer", "");
ADD_PROPERTY(PropertyInfo(Variant::INT, "layer", PROPERTY_HINT_RANGE, "-128,128,1"), "set_layer", "get_layer");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "visible"), "set_visible", "is_visible");
ADD_GROUP("Transform", "");
ADD_PROPERTY(PropertyInfo(Variant::VECTOR2, "offset"), "set_offset", "get_offset");
ADD_PROPERTY(PropertyInfo(Variant::REAL, "rotation_degrees", PROPERTY_HINT_RANGE, "-1080,1080,0.1,or_lesser,or_greater", PROPERTY_USAGE_EDITOR), "set_rotation_degrees", "get_rotation_degrees");
Expand All @@ -304,6 +335,8 @@ void CanvasLayer::_bind_methods() {
ADD_GROUP("Follow Viewport", "follow_viewport");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "follow_viewport_enable"), "set_follow_viewport", "is_following_viewport");
ADD_PROPERTY(PropertyInfo(Variant::REAL, "follow_viewport_scale", PROPERTY_HINT_RANGE, "0.001,1000,0.001,or_greater,or_lesser"), "set_follow_viewport_scale", "get_follow_viewport_scale");

ADD_SIGNAL(MethodInfo("visibility_changed"));
}

#ifdef TOOLS_ENABLED
Expand All @@ -326,6 +359,7 @@ CanvasLayer::CanvasLayer() {
custom_viewport = nullptr;
custom_viewport_id = 0;
sort_index = 0;
visible = true;
follow_viewport = false;
follow_viewport_scale = 1.0;
}
Expand Down
4 changes: 4 additions & 0 deletions scene/main/canvas_layer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class CanvasLayer : public Node {
Viewport *vp;

int sort_index;
bool visible;

bool follow_viewport;
float follow_viewport_scale;
Expand All @@ -69,6 +70,9 @@ class CanvasLayer : public Node {
void set_layer(int p_xform);
int get_layer() const;

void set_visible(bool p_visible);
bool is_visible() const;

void set_transform(const Transform2D &p_xform);
Transform2D get_transform() const;

Expand Down