Pixelorama icon indicating copy to clipboard operation
Pixelorama copied to clipboard
verified publisher icon Orama-Interactive

Basic Layer Groups and Timeline Refactor

Open mrtripie opened this issue 3 years ago • 0 comments

Creating a draft pull request for now to document changes as I go:

This commit: https://github.com/Orama-Interactive/Pixelorama/commit/8b70b56a5efb931d2038480f2e6c747aa92407d9 conflicted with some changes from this branch, and I kept my changes. I think I fixed this (or a similar issue), so this should be checked.

Groups Refactoring:

  • Class splits:
    • [x] Layer is split into BaseLayer, PixelLayer, and GroupLayer
    • [x] Cel is split into BaseCel, PixelCel, and GroupCel
    • [x] LayerButton gets a base scene with inherited scenes for each layer type. To prevent adding too many files with new layer types, they share a LayerButton.gd script that reacts to the buttons it has, and can have export options added to it.
    • [x] CelButton is the same, a base scene with inherited scenes for each cel type. They all share the CelButton.gd script
  • Moving logic that is conditional to a certain layer type to layer/cel classes as overridable functions to make adding new layer types simpler:
    • Layer types:
      • [x] serialize and deserialize functions added to layer classes to move type specific saving/loading logic
        • [x] most of _init's parameters were only used for loading, so they were removed in favor of deserialize (Can do more refactoring in a separate PR if preferred)
      • [x] copy method
      • [x] add set_name_to_default(number) function to layer classes
      • [x] new_empty_cel method
      • [x] copy_cel_method and copy_all_cels methods (may remove during #719)
      • [x] instantiate_layer_button
    • Cel types:
      • [x] save_image_data_to_pxo and load_image_data_from_pxo
      • [x] get/set/create_empty/copy_content methods (used to deal with the unique content of different cel types (ie: PixelCel has image, a TextCel could have a dict with text, font, and position data, GroupCel has null, etc). Used for linking/unlinking cels, deleting cel content, and maybe copying cels in a later PR)
      • [x] get_image (used for read only usage of image data, such as copying a selection or picking a color)
      • [x] update_texture (extracted from the upate_texture methods in Canvas)
      • [x] instantiate_cel_button

Timeline Refactor (copied from the PR onto this PR)

Refactor of how adding/removing/moving Layers, Cels, and Frames works.

  • Created add/remove/move/swap_frame(s)/layer(s)/cel(s) methods to Project class. When the user presses the add layer button, drags and drops a frame, etc. the methods responding to those actions will call these functions in do/undo. These methods are reversible (swap add and remove on undo/redo, or swap parameters on the move and swap methods). When these methods are called, they call the new project_frame/layer/cel_added/removed methods in AnimationTimeline that create or remove frame, layer, and/or cel buttons.
  • Moved the timeline portion of Project.change_project to AnimationTimeline.project_changed and rewrote it.
  • Rewrote and combined Project's _toggle_layer_buttons_layers and _toggle_layer_buttons_current_layer methods into a simpler single _toggle_layer_buttons method.
  • Removed code:
    • Project.frames and Project.layers setters: _frames_changed and _layers_changed
    • Removed the "Frame" and "Move Cels" parts of Global.undo_or_redo (Made changes to make what's needed happen elsewhere)
    • Project.remove_cel_buttons
  • Some small readability tweaks
    • Use a project variable in several methods to prevent super long lines like: Global.current_project.layers[Global.current_project.current_layer]
    • Renamed some method variables
  • Several tweaks to the new versions of layer/cel classes
    • The PackedScene resources for these are now loaded in a variable in Global. (Originally I just used load in the create_layer/cel_button methods right before instancing (preload couldn't be used because of cyclic references), however when testing performance on large projects, I noticed that the loading projects and changing tabs took twice as long, due to the the button PackedScene being constantly freed after creating each instance and reloaded).
    • Modified how is_expanded_in_timeline works
    • Added copy, copy_cel, and copy_all_cels methods to Layer classes
    • Changed get_default_name to set_name_to_default in Layer classes

Features:

  • The entire UI for Layers, Cels, and Frames is no longer destroyed and recreated for each change (just where the change happened), so performance with lots of Layers and/or Frames is greatly improved:

https://user-images.githubusercontent.com/65431647/180324982-dfe6099c-a3b9-466b-9f42-68cf797f4752.mp4

(Borrowed the splash screen by Roroto to have a project with lots of frame to test)

  • On Layers, Cels, and Frames, dragging will now move them by default, with the previous swapping behavior being usable when holding Control
    • Cels will always swap on different layers to make sure each layer has the same amount of cels, would it be better to simply not allow dragging and dropping them to different layers without Control held for consistency?
    • Drag and drop highlight

https://user-images.githubusercontent.com/65431647/180324682-1d8e1730-47c1-4886-ad0b-dcd7fc6ea157.mp4

  • Made move up/down layer buttons and delete layer button work with the hierarchy.

https://user-images.githubusercontent.com/65431647/180324797-a3caee2c-2101-4ffa-8946-b522d02c5958.mp4

Fixes:

  • When changing projects, in the timeline show the project's selected cels as the selection, rather than the current layer/frame (this was a bug in Pixelorama master)

Layer Hierarchy:

  • BaseLayer includes several things for the hierarchy:
    • [x] index convenience variable (used for saving, and was helpful for my shader based group blending, but may not be in the final solution, may be removed if it turns out updating it is less convenient than having it in the future)
    • [x] parent variable that is a reference to the parent layer
    • [x] accepts_child function that asks if a layer can be added as a child of this one (to be overriden)
    • [x] is_visible_in_hierarchy function
    • [x] is_locked_in_hierarchy function
    • [x] get_hierarchy_depth function
    • [x] get_layer_path function (returns a string similar to NodePath's)
    • Some functions that (for now) are only really needed in Group Layers, but were moved to BaseLayer to keep certain code simple:
      • [x] has_children function
      • [x] get_children function
      • [x] get_child_count function
      • [x] is_a_parent_of function (recursive)
    • ~~is_a_child_of function (recursive)~~ (just reverse it and use is_a_parent_of)
  • GroupLayer includes some things for the hierarchy: (may be moved to BaseLayer if we add masks or clipping masks)
    • [x] expanded variable
    • [x] is_expanded_in_hierarchy function

File Format:

  • [x] added "type" to layer data dictionary
  • [x] added "parent" to layer data dictionary (the index of the parent, converted from the layer's "parent" reference)

UI:

  • [x] New Group Icons
  • [x] AnimationTimeline gets an add group button
  • Layer Hierarchy
    • [x] LayerButtons shift value as you go deeper in the hierarchy to make it easier to tell which layer is in which group
    • [x] LayerButton names are now left aligned, and are shifted with a spacer to the right as you go deeper in the hierarchy
    • [x] A line is placed before the name to also help emphasize the hierarchy depth
    • [x] Can collapse a group, layers inside that group will be hidden in the Animation Timeline
    • [x] If you hide a group, child layers will become hidden as well, their visibility icon will become semi-transparent
    • [x] If you lock a group, child layers will become locked as well, their locked icon will become semi-transparent
  • [x] Changing how drag and dropping layers and cels works:
    • [x] By default, dragging and dropping now shifts a single frame/layer/cel around, rather than swapping 2.
      • [x] Cels dropped on a different layer will always be swapped
      • [x] Holding [CTRL] switches to swapping (hopefully we can add something to make this more obvious to the user, such as Splash tips #695 or Bottom hints)
    • [x] How exactly a layer is dropped depends on the "region" of the LayerButton it was dropped on:
      • Top region: Dropped above the the layer, with the same parent as the layer dropped on
      • Bottom region: Dropped below the layer, with the same parent as the layer dropped on (If the layers you're dropping in between have the same parent, it acts the same, but if they're different, this means you can specify which parent to place it in)
      • Center region: Only if the layer you're dropping passes the accepts_child check of the layer you drop on (making this work automatically if we add new layer types) there is also a center region. If you drop here, it will be dropped below this layer, as a child of this layer.
  • [x] AnimationTimeline gets a "DragHighlight" ColorRect that is hidden by default, when dragging layers or cels it will be shown above the region of the LayerButton you're dropping on (it is actually shifted a bit from the true region to make to make it a little clearer where it will be dropped)
    • [x] The highlight also gets shifted slightly to the right (and shrunk) to represent changes in hierarchy depth
    • [x] AnimationTimeline implements the _notification function NOTIFICATION_DROP_ENDED to make sure that even if a dropped failed, that the highlight is hidden

Translation .pot File:

  • [x] "Group" layer type
  • [x] Add group button tooltip
  • [x] Expand/collapse group button tooltip

Bug Fixes:

  • [x] All cels created when opening a .pxo file being PixelCels
  • [x] There being a gap in between cel buttons when collapsing a group when there are layers below that aren't its children
  • [x] Situation at the bottom of the stack there is a group with children, if you try to move a layer to the bottom region of the group (which should put it outside the group, as the lowest layer) the layer disappears (to_index of -1)
  • [x] Prevent moving layers in invalid ways (like trying to parent a group to one of its children)
  • [x] Errors from trying to draw on a GroupCel
  • [x] Image Effects
    • [x] Prevent using image effects on non-pixel cels
  • [x] Multiple layer drawing
    • Hopefully fixing drawing in general will fix this (just want to make sure its not missed)
  • CelButton menu:
    • [x] Delete
    • [x] Link/unlink
  • [x] Open image as new frame or replace frame can select a non Pixel Layer
    • [x] Renamed Replace frame to Replace Cel
    • [x] Replace the AtLayerSpinbox with AtLayerOption, an OptionButton giving a list of layer names for valid layers
  • Bugs fixed in timeline refactor:
    • [x] Layer swapping messing up hierarchy
    • [x] Deleting a group not deleting its children, or un-parenting them
    • [x] Switching projects
    • [x] Layer Merging
    • [x] Frame adding/removing
    • [x] Layer duplicating
  • Cleanup:
    • [x] Check all places with static BasePixel/Group Layer/Cel variables to make sure they are using the correct one
    • [x] Fix any warnings that show up in Godot debugger

Things to work on later:

  • [ ] BUG: Moving cels when there are linked cels on the layer can result in the linked_cels array being out of sync. This may be fixed automatically in my linked cel refactor idea: #719, so it can maybe be left for then.
  • [ ] Generating a texture for Group Cels
    • [ ] CelButtons / Cel previews
    • [ ] This may mess up Onion Skinning, will need to figure out how this should work (Maybe new shader based blending will help here?)
  • [ ] Copying selection on groups (requires Group cels to have textures generated)
    • [x] create get_image() on cel classes, and update_texture() on cel classes
  • [ ] UI improvements:
    • [ ] Better theme support for hierarchy visualization
    • [ ] Hide the drag and drop highlight when not hovering over a Frame/Layer/Cel button
    • [ ] Remove the gaps between layers (visual gaps can remain) so that if you try to drag a layer in between two layers, there isn't a gap where it will fail
    • [ ] Allow the user to automatically select child layers/cels (maybe when holding Alt?)
    • [ ] Image effects: if only a non-pixel cel is selected, perhaps automatically change selected cels to frames
    • [ ] A way to automatically group selected layers (perhaps holding control when pressing the add group button)
  • [ ] It would be nice if moving layers will update the selected layer index (right now if you drag and drop a layer, the selected layer will be the one at the same position as the layer used to be, the selection won't be the layer you just moved (this is the same as before, though cels already update the selected cel position)
  • [ ] Check for any if x is Pixel/GroupCel/Layer usage and see if there's a way to make it more generic (Maybe if both indexed color and MaskLayers are implemented, figure out how to abstract the image data between Pixel and Mask Layers)

mrtripie avatar May 08 '22 16:05 mrtripie