From 48326900d9d9f0b32fd435e56fd5a39bbf13fa36 Mon Sep 17 00:00:00 2001 From: Manolis Papadeas <35376950+OverloadedOrama@users.noreply.github.com> Date: Thu, 23 Dec 2021 19:58:07 +0200 Subject: [PATCH] Fix a memory leak coming from the Layer class Removed the "frame_container" property from Layer.gd, which used to hold a reference to a node, leading to orphan nodes being created and never freed. Memory management seems to be working okay now. Previously, every time the user made a change, memory kept going up and never coming down. Now, data that can never be recovered, like undo data that have been rewritten in history, are also removed from memory. --- src/Autoload/OpenSave.gd | 9 +--- src/Classes/Layer.gd | 9 +--- src/Classes/Project.gd | 81 +++++++++++++++++----------- src/UI/Canvas/Canvas.gd | 14 +++-- src/UI/Timeline/AnimationTimeline.gd | 61 +++------------------ src/UI/Timeline/CelButton.gd | 14 +---- src/UI/Timeline/LayerButton.gd | 3 +- src/UI/Timeline/LayerButton.tscn | 12 ++--- 8 files changed, 72 insertions(+), 131 deletions(-) diff --git a/src/Autoload/OpenSave.gd b/src/Autoload/OpenSave.gd index 1e2a294e91e..b9e04e1d577 100644 --- a/src/Autoload/OpenSave.gd +++ b/src/Autoload/OpenSave.gd @@ -170,14 +170,7 @@ func open_old_pxo_file(file: File, new_project: Project, first_line: String) -> var layer_new_cels_linked := file.get_8() linked_cels.append(file.get_var()) - var l := Layer.new( - layer_name, - layer_visibility, - layer_lock, - HBoxContainer.new(), - layer_new_cels_linked, - [] - ) + var l := Layer.new(layer_name, layer_visibility, layer_lock, layer_new_cels_linked, []) new_project.layers.append(l) global_layer_line = file.get_line() diff --git a/src/Classes/Layer.gd b/src/Classes/Layer.gd index 0b455b3767f..7ff4dec1875 100644 --- a/src/Classes/Layer.gd +++ b/src/Classes/Layer.gd @@ -5,23 +5,16 @@ extends Reference var name := "" var visible := true var locked := false -var frame_container: HBoxContainer var new_cels_linked := false var linked_cels := [] # Array of Frames func _init( - _name := "", - _visible := true, - _locked := false, - _frame_container := HBoxContainer.new(), - _new_cels_linked := false, - _linked_cels := [] + _name := "", _visible := true, _locked := false, _new_cels_linked := false, _linked_cels := [] ) -> void: name = _name visible = _visible locked = _locked - frame_container = _frame_container new_cels_linked = _new_cels_linked linked_cels = _linked_cels diff --git a/src/Classes/Project.gd b/src/Classes/Project.gd index 80e12700f0f..79221361357 100644 --- a/src/Classes/Project.gd +++ b/src/Classes/Project.gd @@ -85,8 +85,6 @@ func _init(_frames := [], _name := tr("untitled"), _size := Vector2(64, 64)) -> func remove() -> void: - for layer in layers: - layer.frame_container.queue_free() undo_redo.free() for guide in guides: guide.queue_free() @@ -160,7 +158,8 @@ func change_project() -> void: layer_container.label.text = layers[i].name layer_container.line_edit.text = layers[i].name - Global.frames_container.add_child(layers[i].frame_container) + var layer_cel_container := HBoxContainer.new() + Global.frames_container.add_child(layer_cel_container) for j in range(frames.size()): # Create Cel buttons var cel_button = cel_button_node.instance() cel_button.frame = j @@ -169,7 +168,7 @@ func change_project() -> void: if j == current_frame and i == current_layer: cel_button.pressed = true - layers[i].frame_container.add_child(cel_button) + layer_cel_container.add_child(cel_button) for j in range(frames.size()): # Create frame ID labels var button: Button = frame_button_node.instance() @@ -335,7 +334,6 @@ func serialize() -> Dictionary: cel_data.append( { "opacity": cel.opacity, - # "image_data" : cel.image.get_data() } ) frame_data.append({"cels": cel_data, "duration": frame.duration}) @@ -402,7 +400,6 @@ func deserialize(dict: Dictionary) -> void: saved_layer.name, saved_layer.visible, saved_layer.locked, - HBoxContainer.new(), saved_layer.new_cels_linked, linked_cels ) @@ -463,7 +460,15 @@ func _frames_changed(value: Array) -> void: frame_id.queue_free() for i in range(layers.size() - 1, -1, -1): - Global.frames_container.add_child(layers[i].frame_container) + var layer_cel_container := HBoxContainer.new() + layer_cel_container.name = "FRAMESS " + str(i) + Global.frames_container.add_child(layer_cel_container) + for j in range(frames.size()): + var cel_button = cel_button_node.instance() + cel_button.frame = j + cel_button.layer = i + cel_button.get_child(0).texture = frames[j].cels[i].image_texture + layer_cel_container.add_child(cel_button) for j in range(frames.size()): var button: Button = frame_button_node.instance() @@ -472,14 +477,6 @@ func _frames_changed(value: Array) -> void: button.text = str(j + 1) Global.frame_ids.add_child(button) - for i in range(layers.size() - 1, -1, -1): - var cel_button = cel_button_node.instance() - cel_button.frame = j - cel_button.layer = i - cel_button.get_child(0).texture = frames[j].cels[i].image_texture - - layers[i].frame_container.add_child(cel_button) - _set_timeline_first_and_last_frames() @@ -497,23 +494,24 @@ func _layers_changed(value: Array) -> void: _remove_cel_buttons() for i in range(layers.size() - 1, -1, -1): - var layer_container = layer_button_node.instance() - layer_container.layer = i + var layer_button: LayerButton = layer_button_node.instance() + layer_button.layer = i if layers[i].name == "": layers[i].name = tr("Layer") + " %s" % i - Global.layers_container.add_child(layer_container) - layer_container.label.text = layers[i].name - layer_container.line_edit.text = layers[i].name + Global.layers_container.add_child(layer_button) + layer_button.label.text = layers[i].name + layer_button.line_edit.text = layers[i].name - Global.frames_container.add_child(layers[i].frame_container) + var layer_cel_container := HBoxContainer.new() + layer_cel_container.name = "LAYERSSS " + str(i) + Global.frames_container.add_child(layer_cel_container) for j in range(frames.size()): var cel_button = cel_button_node.instance() cel_button.frame = j cel_button.layer = i cel_button.get_child(0).texture = frames[j].cels[i].image_texture - - layers[i].frame_container.add_child(cel_button) + layer_cel_container.add_child(cel_button) var layer_button = Global.layers_container.get_child( Global.layers_container.get_child_count() - 1 - current_layer @@ -525,10 +523,8 @@ func _layers_changed(value: Array) -> void: func _remove_cel_buttons() -> void: for container in Global.frames_container.get_children(): - for button in container.get_children(): - container.remove_child(button) - button.queue_free() Global.frames_container.remove_child(container) + container.queue_free() func _frame_changed(value: int) -> void: @@ -544,9 +540,9 @@ func _frame_changed(value: int) -> void: ): text_color = Color.black Global.frame_ids.get_child(i).add_color_override("font_color", text_color) - for layer in layers: # De-select all the other frames - if i < layer.frame_container.get_child_count(): - layer.frame_container.get_child(i).pressed = false + for container in Global.frames_container.get_children(): # De-select all the other frames + if i < container.get_child_count(): + container.get_child(i).pressed = false if selected_cels.empty(): selected_cels.append([current_frame, current_layer]) @@ -558,9 +554,13 @@ func _frame_changed(value: int) -> void: Global.frame_ids.get_child(current_frame_tmp).add_color_override( "font_color", Global.control.theme.get_color("Selected Color", "Label") ) - if layers: - if current_frame_tmp < layers[current_layer_tmp].frame_container.get_child_count(): - var fbutton = layers[current_layer_tmp].frame_container.get_child(current_frame_tmp) + var container_child_count: int = Global.frames_container.get_child_count() + if current_layer_tmp < container_child_count: + var container = Global.frames_container.get_child( + container_child_count - 1 - current_layer_tmp + ) + if current_frame_tmp < container.get_child_count(): + var fbutton = container.get_child(current_frame_tmp) fbutton.pressed = true Global.disable_button(Global.remove_frame_button, frames.size() == 1) @@ -704,6 +704,23 @@ func is_empty() -> bool: ) +func duplicate_layers() -> Array: + var new_layers: Array = layers.duplicate() + # Loop through the array to create new classes for each element, so that they + # won't be the same as the original array's classes. Needed for undo/redo to work properly. + for i in new_layers.size(): + var new_linked_cels = new_layers[i].linked_cels.duplicate() + new_layers[i] = Layer.new( + new_layers[i].name, + new_layers[i].visible, + new_layers[i].locked, + new_layers[i].new_cels_linked, + new_linked_cels + ) + + return new_layers + + func can_pixel_get_drawn( pixel: Vector2, bitmap: BitMap = selection_bitmap, diff --git a/src/UI/Canvas/Canvas.gd b/src/UI/Canvas/Canvas.gd index 21737446018..12f45cdbd8a 100644 --- a/src/UI/Canvas/Canvas.gd +++ b/src/UI/Canvas/Canvas.gd @@ -112,10 +112,12 @@ func update_texture(layer_i: int, frame_i := -1, project: Project = Global.curre current_cel.image_texture.create_from_image(current_cel.image, 0) if project == Global.current_project: - var frame_button = project.layers[layer_i].frame_container.get_child(frame_i) - var frame_texture_rect: TextureRect - frame_texture_rect = frame_button.find_node("CelTexture") - frame_texture_rect.texture = current_cel.image_texture + var container_index = Global.frames_container.get_child_count() - 1 - layer_i + var layer_cel_container = Global.frames_container.get_child(container_index) + var cel_button = layer_cel_container.get_child(frame_i) + var cel_texture_rect: TextureRect + cel_texture_rect = cel_button.find_node("CelTexture") + cel_texture_rect.texture = current_cel.image_texture func update_selected_cels_textures(project: Project = Global.current_project) -> void: @@ -127,7 +129,9 @@ func update_selected_cels_textures(project: Project = Global.current_project) -> current_cel.image_texture.create_from_image(current_cel.image, 0) if project == Global.current_project: - var cel_button = project.layers[layer_index].frame_container.get_child(frame_index) + var container_index = Global.frames_container.get_child_count() - 1 - layer_index + var layer_cel_container = Global.frames_container.get_child(container_index) + var cel_button = layer_cel_container.get_child(frame_index) var cel_texture_rect: TextureRect = cel_button.find_node("CelTexture") cel_texture_rect.texture = current_cel.image_texture diff --git a/src/UI/Timeline/AnimationTimeline.gd b/src/UI/Timeline/AnimationTimeline.gd index 9afb78f31ee..9127bf20fc8 100644 --- a/src/UI/Timeline/AnimationTimeline.gd +++ b/src/UI/Timeline/AnimationTimeline.gd @@ -54,8 +54,8 @@ func cel_size_changed(value: int) -> void: for layer_button in Global.layers_container.get_children(): layer_button.rect_min_size.y = cel_size layer_button.rect_size.y = cel_size - for layer in Global.current_project.layers: - for cel_button in layer.frame_container.get_children(): + for container in Global.frames_container.get_children(): + for cel_button in container.get_children(): cel_button.rect_min_size.x = cel_size cel_button.rect_min_size.y = cel_size cel_button.rect_size.x = cel_size @@ -83,20 +83,8 @@ func add_frame() -> void: var project: Project = Global.current_project var frame: Frame = project.new_empty_frame() var new_frames: Array = project.frames.duplicate() - var new_layers: Array = project.layers.duplicate() + var new_layers: Array = project.duplicate_layers() new_frames.insert(project.current_frame + 1, frame) - # Loop through the array to create new classes for each element, so that they - # won't be the same as the original array's classes. Needed for undo/redo to work properly. - for i in new_layers.size(): - var new_linked_cels = new_layers[i].linked_cels.duplicate() - new_layers[i] = Layer.new( - new_layers[i].name, - new_layers[i].visible, - new_layers[i].locked, - new_layers[i].frame_container, - new_layers[i].new_cels_linked, - new_linked_cels - ) for l_i in range(new_layers.size()): if new_layers[l_i].new_cels_linked: # If the link button is pressed @@ -161,19 +149,7 @@ func delete_frame(frame := -1) -> void: # Check if one of the cels of the frame is linked # if they are, unlink them too # this prevents removed cels being kept in linked memory - var new_layers: Array = Global.current_project.layers.duplicate() - # Loop through the array to create new classes for each element, so that they - # won't be the same as the original array's classes. Needed for undo/redo to work properly. - for i in new_layers.size(): - var new_linked_cels = new_layers[i].linked_cels.duplicate() - new_layers[i] = Layer.new( - new_layers[i].name, - new_layers[i].visible, - new_layers[i].locked, - new_layers[i].frame_container, - new_layers[i].new_cels_linked, - new_linked_cels - ) + var new_layers: Array = Global.current_project.duplicate_layers() for layer in new_layers: for linked in layer.linked_cels: @@ -221,7 +197,7 @@ func copy_frame(frame := -1) -> void: var new_frame := Frame.new() var new_frames := Global.current_project.frames.duplicate() - var new_layers: Array = Global.current_project.layers.duplicate() + var new_layers: Array = Global.current_project.duplicate_layers() new_frames.insert(frame + 1, new_frame) for cel in Global.current_project.frames[frame].cels: # Copy every cel @@ -231,19 +207,6 @@ func copy_frame(frame := -1) -> void: sprite_texture.create_from_image(sprite, 0) new_frame.cels.append(Cel.new(sprite, cel.opacity, sprite_texture)) - # Loop through the array to create new classes for each element, so that they - # won't be the same as the original array's classes. Needed for undo/redo to work properly. - for i in new_layers.size(): - var new_linked_cels = new_layers[i].linked_cels.duplicate() - new_layers[i] = Layer.new( - new_layers[i].name, - new_layers[i].visible, - new_layers[i].locked, - new_layers[i].frame_container, - new_layers[i].new_cels_linked, - new_linked_cels - ) - for l_i in range(new_layers.size()): if new_layers[l_i].new_cels_linked: # If the link button is pressed new_layers[l_i].linked_cels.append(new_frame) @@ -651,19 +614,7 @@ func change_layer_order(rate: int) -> void: func _on_MergeDownLayer_pressed() -> void: - var new_layers: Array = Global.current_project.layers.duplicate() - # Loop through the array to create new classes for each element, so that they - # won't be the same as the original array's classes. Needed for undo/redo to work properly. - for i in new_layers.size(): - var new_linked_cels = new_layers[i].linked_cels.duplicate() - new_layers[i] = Layer.new( - new_layers[i].name, - new_layers[i].visible, - new_layers[i].locked, - new_layers[i].frame_container, - new_layers[i].new_cels_linked, - new_linked_cels - ) + var new_layers: Array = Global.current_project.duplicate_layers() Global.current_project.undos += 1 Global.current_project.undo_redo.create_action("Merge Layer") diff --git a/src/UI/Timeline/CelButton.gd b/src/UI/Timeline/CelButton.gd index 878c5603ab1..f981b16a71d 100644 --- a/src/UI/Timeline/CelButton.gd +++ b/src/UI/Timeline/CelButton.gd @@ -105,19 +105,7 @@ func _on_PopupMenu_id_pressed(id: int) -> void: MenuOptions.LINK: var f: Frame = Global.current_project.frames[frame] var cel_index: int = Global.current_project.layers[layer].linked_cels.find(f) - var new_layers: Array = Global.current_project.layers.duplicate() - # Loop through the array to create new classes for each element, so that they - # won't be the same as the original array's classes. Needed for undo/redo to work properly. - for i in new_layers.size(): - var new_linked_cels: Array = new_layers[i].linked_cels.duplicate() - new_layers[i] = Layer.new( - new_layers[i].name, - new_layers[i].visible, - new_layers[i].locked, - new_layers[i].frame_container, - new_layers[i].new_cels_linked, - new_linked_cels - ) + var new_layers: Array = Global.current_project.duplicate_layers() var new_cels: Array = f.cels.duplicate() for i in new_cels.size(): new_cels[i] = Cel.new( diff --git a/src/UI/Timeline/LayerButton.gd b/src/UI/Timeline/LayerButton.gd index 3295db21cc5..41022a103a3 100644 --- a/src/UI/Timeline/LayerButton.gd +++ b/src/UI/Timeline/LayerButton.gd @@ -116,7 +116,8 @@ func _on_LinkButton_pressed() -> void: layer_class.linked_cels.append( Global.current_project.frames[Global.current_project.current_frame] ) - layer_class.frame_container.get_child(Global.current_project.current_frame).button_setup() + var container = Global.frames_container.get_child(Global.current_project.current_layer) + container.get_child(Global.current_project.current_frame).button_setup() Global.current_project.layers = Global.current_project.layers # Call the setter diff --git a/src/UI/Timeline/LayerButton.tscn b/src/UI/Timeline/LayerButton.tscn index c0c001e66a5..c17453134af 100644 --- a/src/UI/Timeline/LayerButton.tscn +++ b/src/UI/Timeline/LayerButton.tscn @@ -36,9 +36,7 @@ margin_right = 90.0 margin_bottom = 36.0 custom_constants/separation = 10 -[node name="VisibilityButton" type="Button" parent="HBoxContainer/LayerButtons" groups=[ -"UIButtons", -]] +[node name="VisibilityButton" type="Button" parent="HBoxContainer/LayerButtons" groups=["UIButtons"]] margin_top = 7.0 margin_right = 22.0 margin_bottom = 29.0 @@ -65,9 +63,7 @@ __meta__ = { "_edit_use_anchors_": false } -[node name="LockButton" type="Button" parent="HBoxContainer/LayerButtons" groups=[ -"UIButtons", -]] +[node name="LockButton" type="Button" parent="HBoxContainer/LayerButtons" groups=["UIButtons"]] margin_left = 32.0 margin_top = 7.0 margin_right = 54.0 @@ -95,9 +91,7 @@ __meta__ = { "_edit_use_anchors_": false } -[node name="LinkButton" type="Button" parent="HBoxContainer/LayerButtons" groups=[ -"UIButtons", -]] +[node name="LinkButton" type="Button" parent="HBoxContainer/LayerButtons" groups=["UIButtons"]] margin_left = 64.0 margin_top = 7.0 margin_right = 86.0