Refactors needed for new layer types by dacap · Pull Request #5347 · aseprite/aseprite

dacap · 2025-08-13T12:07:33Z

This is a draft for an initial refactor to start working in several new layer types:

Audio layer: Audio support (for lip syncing) #70
Mask layer: Mask Layers + Transparent Pixel Lock #459
FX layer: FX Layers #5081
Text layer: Future work for the text tool #4617
Vector layer: Path / vector layer #814 / Live Vector Layers #2704 / Feature Request: Stroke/Fill Vector Shapes #2976
Event & gizmo layer (abstracts/data only layers): Hot spot tool #722 Extend the slices system into a general-purpose "Gizmos" system #3511
Sub-sprites / external sprite layer: Library of sprites #184 / Create scenes/movies with several sprite instances #593 / Improve "reference layer" feature #1321 / Nested Aseprite files i.e. Flash MovieClips #5012
Tweening: Tweening support #96
Associate user data to an empty cel #1303
New "virtual" timeline #3904
Virtual slice layer to show slice temporal data Slice tool #721

With this PR we'll try to incorporate the required concept for these layers, not to implement all these kind of layers at once, but it will be a good starting point to continue working after this is merged.

aseprite-bot

clang-tidy made some suggestions

aseprite-bot

clang-tidy made some suggestions

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2025-08-14T20:58:45Z

  SubObjectsFromSprite io(layer->sprite());
-  bool has_data = (read8(m_stream) != 0);
+  const uint8_t flags = read8(m_stream);
+  const bool has_data = (flags & 1 ? true : false);


warning: redundant boolean literal in ternary expression result [readability-simplify-boolean-expr]

Suggested change

const bool has_data = (flags & 1 ? true : false);

const bool has_data = ((flags & 1) != 0);

aseprite-bot · 2025-08-14T20:58:45Z

-  bool has_data = (read8(m_stream) != 0);
+  const uint8_t flags = read8(m_stream);
+  const bool has_data = (flags & 1 ? true : false);
+  const bool has_image = (flags & 2 ? true : false);


warning: redundant boolean literal in ternary expression result [readability-simplify-boolean-expr]

Suggested change

const bool has_image = (flags & 2 ? true : false);

const bool has_image = ((flags & 2) != 0);

aseprite-bot · 2025-08-14T20:58:45Z

-    ImageRef image(read_image(m_stream));
-    io.addImageRef(image);
+    if (has_image) {
+      ImageRef image(read_image(m_stream));


warning: variable 'image' of type 'ImageRef' (aka 'shared_ptr') can be declared 'const' [misc-const-correctness]

Suggested change

ImageRef image(read_image(m_stream));

ImageRef const image(read_image(m_stream));

aseprite-bot · 2025-08-14T20:58:45Z

+  // image, because there are other undo branches that could try to
+  // modify/re-add this same image ID
+  if (m_oldImageId) {
+    ImageRef oldImage = sprite->getImageRef(m_oldImageId);


warning: variable 'oldImage' of type 'ImageRef' (aka 'shared_ptr') can be declared 'const' [misc-const-correctness]

Suggested change

ImageRef oldImage = sprite->getImageRef(m_oldImageId);

ImageRef const oldImage = sprite->getImageRef(m_oldImageId);

aseprite-bot · 2025-08-14T20:58:45Z

-    ObjectId rightImg = (right ? right->image()->id() : 0);
-    fromLeft = (leftImg == cel->image()->id());
-    fromRight = (rightImg == cel->image()->id());
+    fromLeft = (left && left->image() && left->imageId() == cel->imageId());


warning: implicit conversion 'Cel *' -> 'bool' [readability-implicit-bool-conversion]

Suggested change

fromLeft = (left && left->image() && left->imageId() == cel->imageId());

fromLeft = ((left != nullptr) && left->image() && left->imageId() == cel->imageId());

aseprite-bot · 2025-08-14T20:58:46Z

+
+    // From Tilemap -> Image
+    if (srcCel->layer()->isTilemap() && !dstLayer->isTilemap()) {
+      auto layerTilemap = static_cast<doc::LayerTilemap*>(srcCel->layer());


warning: 'auto layerTilemap' can be declared as 'auto *layerTilemap' [readability-qualified-auto]

Suggested change

auto layerTilemap = static_cast<doc::LayerTilemap*>(srcCel->layer());

auto *layerTilemap = static_cast<doc::LayerTilemap*>(srcCel->layer());

aseprite-bot · 2025-08-14T20:58:46Z

+    }
+    // From Image or Tilemap -> Tilemap
+    else if (dstLayer->isTilemap()) {
+      auto dstLayerTilemap = static_cast<doc::LayerTilemap*>(dstLayer);


warning: 'auto dstLayerTilemap' can be declared as 'auto *dstLayerTilemap' [readability-qualified-auto]

Suggested change

auto dstLayerTilemap = static_cast<doc::LayerTilemap*>(dstLayer);

auto *dstLayerTilemap = static_cast<doc::LayerTilemap*>(dstLayer);

aseprite-bot · 2025-08-14T20:58:46Z

-                                                    // getTrimDstImageBounds()
-                                                    m_grid.tileToCanvas(trimBounds.origin()) :
-                                                    trimBounds.origin()));
+          gfx::Point newPosition = (m_cel->position() +


warning: variable 'newPosition' of type 'gfx::Point' (aka 'PointT') can be declared 'const' [misc-const-correctness]

Suggested change

gfx::Point newPosition = (m_cel->position() +

gfx::Point const newPosition = (m_cel->position() +

aseprite-bot · 2025-08-14T20:58:46Z

-         cel->image()->pixelFormat() == IMAGE_TILEMAP);
+#if _DEBUG
+  if (isTilemap()) {
+    ASSERT(!cel->image() || cel->image()->pixelFormat() == IMAGE_TILEMAP);


warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

ASSERT(!cel->image() || cel->image()->pixelFormat() == IMAGE_TILEMAP); ^

Additional context

laf/base/debug.h:67: expanded from macro 'ASSERT'

if (!(condition)) { \ ^

aseprite-bot · 2025-08-14T20:58:46Z

+    ASSERT(!cel->image() || cel->image()->pixelFormat() == IMAGE_TILEMAP);
+  }
+  else if (isImage()) {
+    ASSERT(!cel->image() || cel->image()->pixelFormat() == sprite()->pixelFormat());


warning: boolean expression can be simplified by DeMorgan's theorem [readability-simplify-boolean-expr]

ASSERT(!cel->image() || cel->image()->pixelFormat() == sprite()->pixelFormat()); ^

Additional context

laf/base/debug.h:67: expanded from macro 'ASSERT'

if (!(condition)) { \ ^

aseprite-bot

clang-tidy made some suggestions

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2025-09-05T20:01:09Z

 class NewLayerCommand : public CommandWithNewParams<NewLayerParams> {
 public:
-  enum class Type { Layer, Group, ReferenceLayer, TilemapLayer };
+  enum class Type {


warning: enum 'Type' uses a larger base type ('int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]

enum class Type { ^

aseprite-bot · 2025-09-05T20:01:09Z

+          return nullptr;
+
+        // Add a new cel to load in the future after we load all layers
+        ObjectId celId = read32(s);


warning: variable 'celId' of type 'ObjectId' (aka 'unsigned int') can be declared 'const' [misc-const-correctness]

Suggested change

ObjectId celId = read32(s);

ObjectId const celId = read32(s);

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2025-09-25T13:19:53Z

  }

-  void writeAllLayersID(std::ofstream& s, ObjectId parentId, const LayerGroup* group)
+  void writeAllLayersID(std::ofstream& s, ObjectId parentId, const Layer* group)


warning: function 'writeAllLayersID' is within a recursive call chain [misc-no-recursion]

void writeAllLayersID(std::ofstream& s, ObjectId parentId, const Layer* group) ^

Additional context

src/app/crash/write_document.cpp:232: example recursive call chain, starting from function 'writeAllLayersID'

void writeAllLayersID(std::ofstream& s, ObjectId parentId, const Layer* group) ^

src/app/crash/write_document.cpp:239: Frame #1: function 'writeAllLayersID' calls function 'writeAllLayersID' here:

writeAllLayersID(s, lay->id(), lay); ^

src/app/crash/write_document.cpp:239: ... which was the starting point of the recursive call chain; there may be other cycles

writeAllLayersID(s, lay->id(), lay); ^

aseprite-bot · 2025-09-25T13:19:54Z

 // Copying

-void Doc::copyLayerContent(const Layer* sourceLayer0, Doc* destDoc, Layer* destLayer0) const
+void Doc::copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer) const


warning: method 'copyLayerContent' can be made static [readability-convert-member-functions-to-static]

Suggested change

void Doc::copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer) const

void Doc::copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer)

src/app/doc.h:248:

- void copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer) const; + static void copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer) ;

aseprite-bot · 2025-09-25T13:19:54Z

 }

-static bool has_child(LayerGroup* parent, Layer* child)
+static bool has_child(Layer* parent, Layer* child)


warning: function 'has_child' is within a recursive call chain [misc-no-recursion]

static bool has_child(Layer* parent, Layer* child) ^

Additional context

src/app/doc_range_ops.cpp:188: example recursive call chain, starting from function 'has_child'

static bool has_child(Layer* parent, Layer* child) ^

src/app/doc_range_ops.cpp:193: Frame #1: function 'has_child' calls function 'has_child' here:

else if (c->isGroup() && has_child(c, child)) ^

src/app/doc_range_ops.cpp:193: ... which was the starting point of the recursive call chain; there may be other cycles

else if (c->isGroup() && has_child(c, child)) ^

aseprite-bot · 2025-09-25T13:19:54Z

      return true;
-    else if (c->isGroup() && has_child(static_cast<LayerGroup*>(c), child))
+    else if (c->isGroup() && has_child(c, child))
      return true;


warning: do not use 'else' after 'return' [readability-else-after-return]

Suggested change

return true;

if (c->isGroup() && has_child(c, child))

return true;

aseprite-bot · 2025-09-25T13:19:54Z

    for (auto moveThis : from.selectedLayers()) {
-      if (moveThis == parent ||
-          (moveThis->isGroup() && has_child(static_cast<LayerGroup*>(moveThis), parent)))
+      if (moveThis == parent || (moveThis->isGroup() && has_child(moveThis, parent)))


warning: 1st argument 'moveThis' (passed to 'parent') looks like it might be swapped with the 2nd, 'parent' (passed to 'child') [readability-suspicious-call-argument]

if (moveThis == parent || (moveThis->isGroup() && has_child(moveThis, parent))) ^

Additional context

src/app/doc_range_ops.cpp:188: in the call to 'has_child', declared here

static bool has_child(Layer* parent, Layer* child) ^

aseprite-bot · 2025-09-25T13:19:55Z

 }

-void LayerGroup::allVisibleReferenceLayers(LayerList& list) const
+void Layer::allVisibleReferenceLayers(LayerList& list) const


warning: function 'allVisibleReferenceLayers' is within a recursive call chain [misc-no-recursion]

void Layer::allVisibleReferenceLayers(LayerList& list) const ^

Additional context

src/doc/layer.cpp:407: example recursive call chain, starting from function 'allVisibleReferenceLayers'

void Layer::allVisibleReferenceLayers(LayerList& list) const ^

src/doc/layer.cpp:414: Frame #1: function 'allVisibleReferenceLayers' calls function 'allVisibleReferenceLayers' here:

child->allVisibleReferenceLayers(list); ^

src/doc/layer.cpp:414: ... which was the starting point of the recursive call chain; there may be other cycles

child->allVisibleReferenceLayers(list); ^

aseprite-bot · 2025-09-25T13:19:55Z

 }

-void LayerGroup::allBrowsableLayers(LayerList& list) const
+void Layer::allBrowsableLayers(LayerList& list) const


warning: function 'allBrowsableLayers' is within a recursive call chain [misc-no-recursion]

void Layer::allBrowsableLayers(LayerList& list) const ^

Additional context

src/doc/layer.cpp:423: example recursive call chain, starting from function 'allBrowsableLayers'

void Layer::allBrowsableLayers(LayerList& list) const ^

src/doc/layer.cpp:427: Frame #1: function 'allBrowsableLayers' calls function 'allBrowsableLayers' here:

child->allBrowsableLayers(list); ^

src/doc/layer.cpp:427: ... which was the starting point of the recursive call chain; there may be other cycles

child->allBrowsableLayers(list); ^

aseprite-bot · 2025-09-25T13:19:56Z

 }

-void LayerGroup::allTilemaps(LayerList& list) const
+void Layer::allTilemaps(LayerList& list) const


warning: function 'allTilemaps' is within a recursive call chain [misc-no-recursion]

void Layer::allTilemaps(LayerList& list) const ^

Additional context

src/doc/layer.cpp:433: example recursive call chain, starting from function 'allTilemaps'

void Layer::allTilemaps(LayerList& list) const ^

src/doc/layer.cpp:437: Frame #1: function 'allTilemaps' calls function 'allTilemaps' here:

child->allTilemaps(list); ^

src/doc/layer.cpp:437: ... which was the starting point of the recursive call chain; there may be other cycles

child->allTilemaps(list); ^

aseprite-bot · 2025-09-25T13:19:56Z

 }

-std::string LayerGroup::visibleLayerHierarchyAsString(const std::string& indent) const
+std::string Layer::visibleLayerHierarchyAsString(const std::string& indent) const


warning: function 'visibleLayerHierarchyAsString' is within a recursive call chain [misc-no-recursion]

std::string Layer::visibleLayerHierarchyAsString(const std::string& indent) const ^

Additional context

src/doc/layer.cpp:444: example recursive call chain, starting from function 'visibleLayerHierarchyAsString'

std::string Layer::visibleLayerHierarchyAsString(const std::string& indent) const ^

src/doc/layer.cpp:453: Frame #1: function 'visibleLayerHierarchyAsString' calls function 'visibleLayerHierarchyAsString' here:

str += child->visibleLayerHierarchyAsString(indent + " "); ^

src/doc/layer.cpp:453: ... which was the starting point of the recursive call chain; there may be other cycles

str += child->visibleLayerHierarchyAsString(indent + " "); ^

aseprite-bot · 2025-09-25T13:19:56Z

-}
-
-layer_t LayerGroup::getLayerIndex(const Layer* layer, layer_t& index) const
+layer_t Layer::getLayerIndex(const Layer* layer, layer_t& index) const


warning: function 'getLayerIndex' is within a recursive call chain [misc-no-recursion]

layer_t Layer::getLayerIndex(const Layer* layer, layer_t& index) const ^

Additional context

src/doc/layer.cpp:516: example recursive call chain, starting from function 'getLayerIndex'

layer_t Layer::getLayerIndex(const Layer* layer, layer_t& index) const ^

src/doc/layer.cpp:519: Frame #1: function 'getLayerIndex' calls function 'getLayerIndex' here:

if ((child->hasSublayers() && child->getLayerIndex(layer, index) != -1) || (child == layer)) { ^

src/doc/layer.cpp:519: ... which was the starting point of the recursive call chain; there may be other cycles

if ((child->hasSublayers() && child->getLayerIndex(layer, index) != -1) || (child == layer)) { ^

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2025-11-20T19:54:31Z

 // Copying

-void Doc::copyLayerContent(const Layer* sourceLayer0, Doc* destDoc, Layer* destLayer0) const
+void Doc::copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer) const


warning: method 'copyLayerContent' can be made static [readability-convert-member-functions-to-static]

Suggested change

void Doc::copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer) const

void Doc::copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer)

src/app/doc.h:249:

- void copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer) const; + static void copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer) ;

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2026-02-26T18:04:01Z

+  return size;
+}
+
+void Layer::suspendObject()


warning: function 'suspendObject' is within a recursive call chain [misc-no-recursion]

void Layer::suspendObject() ^

Additional context

src/doc/layer.cpp:59: example recursive call chain, starting from function 'suspendObject'

void Layer::suspendObject() ^

src/doc/layer.cpp:62: Frame #1: function 'suspendObject' calls function 'suspendObject' here:

child->suspendObject(); ^

src/doc/layer.cpp:62: ... which was the starting point of the recursive call chain; there may be other cycles

child->suspendObject(); ^

aseprite-bot · 2026-02-26T18:04:01Z

+    child->suspendObject();
+
+  CelIterator it = getCelBegin();
+  CelIterator end = getCelEnd();


warning: variable 'end' of type 'CelIterator' (aka '__normal_iterator<doc::Cel **, std::vector<doc::Cel *, std::allocator<doc::Cel *>>>') can be declared 'const' [misc-const-correctness]

Suggested change

CelIterator end = getCelEnd();

CelIterator const end = getCelEnd();

aseprite-bot · 2026-02-26T18:04:01Z

+  WithUserData::suspendObject();
+}
+
+void Layer::restoreObject()


warning: function 'restoreObject' is within a recursive call chain [misc-no-recursion]

void Layer::restoreObject() ^

Additional context

src/doc/layer.cpp:74: example recursive call chain, starting from function 'restoreObject'

void Layer::restoreObject() ^

src/doc/layer.cpp:86: Frame #1: function 'restoreObject' calls function 'restoreObject' here:

child->restoreObject(); ^

src/doc/layer.cpp:86: ... which was the starting point of the recursive call chain; there may be other cycles

child->restoreObject(); ^

aseprite-bot · 2026-02-26T18:04:01Z

+  WithUserData::restoreObject();
+
+  CelIterator it = getCelBegin();
+  CelIterator end = getCelEnd();


warning: variable 'end' of type 'CelIterator' (aka '__normal_iterator<doc::Cel **, std::vector<doc::Cel *, std::allocator<doc::Cel *>>>') can be declared 'const' [misc-const-correctness]

Suggested change

CelIterator end = getCelEnd();

CelIterator const end = getCelEnd();

aseprite-bot · 2026-02-26T18:04:01Z

 }

-void LayerImage::getCels(CelList& cels) const
+void Layer::getCels(CelList& cels) const


warning: function 'getCels' is within a recursive call chain [misc-no-recursion]

void Layer::getCels(CelList& cels) const ^

Additional context

src/doc/layer.cpp:274: example recursive call chain, starting from function 'getCels'

void Layer::getCels(CelList& cels) const ^

src/doc/layer.cpp:280: Frame #1: function 'getCels' calls function 'getCels' here:

layer->getCels(cels); ^

src/doc/layer.cpp:280: ... which was the starting point of the recursive call chain; there may be other cycles

layer->getCels(cels); ^

aseprite-bot · 2026-02-26T18:04:02Z

 }

-void LayerGroup::allVisibleReferenceLayers(LayerList& list) const
+void Layer::allVisibleReferenceLayers(LayerList& list) const


warning: function 'allVisibleReferenceLayers' is within a recursive call chain [misc-no-recursion]

void Layer::allVisibleReferenceLayers(LayerList& list) const ^

Additional context

src/doc/layer.cpp:437: example recursive call chain, starting from function 'allVisibleReferenceLayers'

void Layer::allVisibleReferenceLayers(LayerList& list) const ^

src/doc/layer.cpp:444: Frame #1: function 'allVisibleReferenceLayers' calls function 'allVisibleReferenceLayers' here:

child->allVisibleReferenceLayers(list); ^

src/doc/layer.cpp:444: ... which was the starting point of the recursive call chain; there may be other cycles

child->allVisibleReferenceLayers(list); ^

aseprite-bot · 2026-02-26T18:04:02Z

 }

-void LayerGroup::allBrowsableLayers(LayerList& list) const
+void Layer::allBrowsableLayers(LayerList& list) const


warning: function 'allBrowsableLayers' is within a recursive call chain [misc-no-recursion]

void Layer::allBrowsableLayers(LayerList& list) const ^

Additional context

src/doc/layer.cpp:453: example recursive call chain, starting from function 'allBrowsableLayers'

void Layer::allBrowsableLayers(LayerList& list) const ^

src/doc/layer.cpp:457: Frame #1: function 'allBrowsableLayers' calls function 'allBrowsableLayers' here:

child->allBrowsableLayers(list); ^

src/doc/layer.cpp:457: ... which was the starting point of the recursive call chain; there may be other cycles

child->allBrowsableLayers(list); ^

aseprite-bot · 2026-02-26T18:04:02Z

 }

-void LayerGroup::allTilemaps(LayerList& list) const
+void Layer::allTilemaps(LayerList& list) const


warning: function 'allTilemaps' is within a recursive call chain [misc-no-recursion]

void Layer::allTilemaps(LayerList& list) const ^

Additional context

src/doc/layer.cpp:463: example recursive call chain, starting from function 'allTilemaps'

void Layer::allTilemaps(LayerList& list) const ^

src/doc/layer.cpp:467: Frame #1: function 'allTilemaps' calls function 'allTilemaps' here:

child->allTilemaps(list); ^

src/doc/layer.cpp:467: ... which was the starting point of the recursive call chain; there may be other cycles

child->allTilemaps(list); ^

aseprite-bot · 2026-02-26T18:04:02Z

 }

-std::string LayerGroup::visibleLayerHierarchyAsString(const std::string& indent) const
+std::string Layer::visibleLayerHierarchyAsString(const std::string& indent) const


warning: function 'visibleLayerHierarchyAsString' is within a recursive call chain [misc-no-recursion]

std::string Layer::visibleLayerHierarchyAsString(const std::string& indent) const ^

Additional context

src/doc/layer.cpp:474: example recursive call chain, starting from function 'visibleLayerHierarchyAsString'

std::string Layer::visibleLayerHierarchyAsString(const std::string& indent) const ^

src/doc/layer.cpp:483: Frame #1: function 'visibleLayerHierarchyAsString' calls function 'visibleLayerHierarchyAsString' here:

str += child->visibleLayerHierarchyAsString(indent + " "); ^

src/doc/layer.cpp:483: ... which was the starting point of the recursive call chain; there may be other cycles

str += child->visibleLayerHierarchyAsString(indent + " "); ^

aseprite-bot · 2026-02-26T18:04:02Z

-}
-
-layer_t LayerGroup::getLayerIndex(const Layer* layer, layer_t& index) const
+layer_t Layer::getLayerIndex(const Layer* layer, layer_t& index) const


warning: function 'getLayerIndex' is within a recursive call chain [misc-no-recursion]

layer_t Layer::getLayerIndex(const Layer* layer, layer_t& index) const ^

Additional context

src/doc/layer.cpp:546: example recursive call chain, starting from function 'getLayerIndex'

layer_t Layer::getLayerIndex(const Layer* layer, layer_t& index) const ^

src/doc/layer.cpp:549: Frame #1: function 'getLayerIndex' calls function 'getLayerIndex' here:

if ((child->hasSublayers() && child->getLayerIndex(layer, index) != -1) || (child == layer)) { ^

src/doc/layer.cpp:549: ... which was the starting point of the recursive call chain; there may be other cycles

if ((child->hasSublayers() && child->getLayerIndex(layer, index) != -1) || (child == layer)) { ^

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2026-02-27T14:51:22Z

+void SetCelImage::onExecute()
+{
+  Cel* cel = this->cel();
+  ImageRef oldImage = cel->imageRef();


warning: variable 'oldImage' of type 'ImageRef' (aka 'shared_ptr') can be declared 'const' [misc-const-correctness]

Suggested change

ImageRef oldImage = cel->imageRef();

ImageRef const oldImage = cel->imageRef();

aseprite-bot · 2026-02-27T14:51:23Z

+void SetCelImage::onUndo()
+{
+  Cel* cel = this->cel();
+  ImageRef currentImage = cel->imageRef();


warning: variable 'currentImage' of type 'ImageRef' (aka 'shared_ptr') can be declared 'const' [misc-const-correctness]

Suggested change

ImageRef currentImage = cel->imageRef();

ImageRef const currentImage = cel->imageRef();

aseprite-bot · 2026-02-27T14:51:23Z

+{
+  Cel* cel = this->cel();
+  ImageRef currentImage = cel->imageRef();
+  gfx::Rect currentBounds = cel->bounds();


warning: variable 'currentBounds' of type 'gfx::Rect' (aka 'RectT') can be declared 'const' [misc-const-correctness]

Suggested change

gfx::Rect currentBounds = cel->bounds();

gfx::Rect const currentBounds = cel->bounds();

ckaiser

Had a look and I have some minor thoughts on the API design, only bug I found was with the layer properties window for groups: it does not adjust the height properly when opening (if you open it for a layer and then minimize the other properties though it works?):

ckaiser · 2026-03-02T06:28:53Z

  , m_opacity(255)
 {
-  ASSERT(type == ObjectType::LayerImage || type == ObjectType::LayerGroup ||
-         type == ObjectType::LayerTilemap);


new-layer-type.md mentions adding the type to this ASSERT, should probably either keep the assert or remove the mention in the documentation.

I think I'll just remove the ASSERT from docs.

ckaiser · 2026-03-02T06:48:08Z

-  virtual bool isBrowsable() const { return false; }
+  bool isBrowsable() const { return isExpanded() && !m_layers.empty(); }
+  // If this layer is a Mask or FX
+  bool isMaskOrFx() const


Is there a more generic word that can encompass "mask or fx" that would allow for future expansion? It feels a little specific to have a check for something that is one of two things, and in future there may be another type of layer that requires the same kind of behavior. Overlay layer? Blanket layer? Modifier layer?

I don't like the name either, but at the moment might be isEffect() if in the future LayerMask is an effect too (both kind of layers have in common that don't make sense alone, and they need to be linked to another layer to apply the effect).

ckaiser · 2026-03-02T07:14:01Z


 public:
+  // Disable assigment
+  Layer& operator=(const Layer& other) = delete;


Probably more consistent to use the DISABLE_COPYING macro

Actually the DISABLE_COPYING is from a pre-C++11 era. Someday we could get rid of it (at the moment we have a mix of both options).

ckaiser · 2026-03-02T07:21:57Z

+  LayerText = 20,
+  LayerVector = 21,
+  LayerAudio = 22,
+  LayerHitbox = 23,


I dunno about pre-defining all these layers types in this PR, I feel like it locks us in to keeping these as TODOs in the code for a long time before they're implemented, and maybe in the meantime we might change our mind about either their existence/necessity/naming, I'd rather we just leave things open for future expansion in the broadest, most generic way, and then in each implementation we can adjust to what we need.

Right now I'll split the PR in two big changes: One with the refactors, and another new PR with the "new layer types" to continue working. So the refactors can be integrated, but the new types are not yet added to the codebase.

We're going to make the Cel concept as a container of any kind of data related to a layer/frame (e.g. audio, vector, text, etc.).

This will be a work-in-progress, as many operations might fail as they still expect a cel with an image.

Now any kind of layer can be a parent layer (not only groups). This is because we want to be able to associate FX/Mask layers as children of a specific layer in the future.

…timeline

We also save the bounds of the cel as it's required to restore the bounds when we restore the image if we SetCelImage(cel, nullptr) and then undo.

…ct types

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2026-03-02T18:07:11Z

-    ObjectId rightImg = (right ? right->image()->id() : 0);
-    fromLeft = (leftImg == cel->image()->id());
-    fromRight = (rightImg == cel->image()->id());
+    fromLeft = (left && left->dataId() == cel->dataId());


warning: implicit conversion 'Cel *' -> 'bool' [readability-implicit-bool-conversion]

Suggested change

fromLeft = (left && left->dataId() == cel->dataId());

fromLeft = ((left != nullptr) && left->dataId() == cel->dataId());

aseprite-bot · 2026-03-02T18:07:11Z

-    fromLeft = (leftImg == cel->image()->id());
-    fromRight = (rightImg == cel->image()->id());
+    fromLeft = (left && left->dataId() == cel->dataId());
+    fromRight = (right && right->dataId() == cel->dataId());


warning: implicit conversion 'Cel *' -> 'bool' [readability-implicit-bool-conversion]

Suggested change

fromRight = (right && right->dataId() == cel->dataId());

fromRight = ((right != nullptr) && right->dataId() == cel->dataId());

dacap added the refactor complexity This issue needs some refactor / modifying several files / rethinking some part of the architecture label Aug 13, 2025

dacap force-pushed the new-layers branch from 1f10e04 to 08168e6 Compare August 13, 2025 12:18

aseprite-bot reviewed Aug 13, 2025

View reviewed changes

Comment thread src/app/commands/cmd_new_layer.cpp Outdated

Comment thread src/doc/layer_tilemap.h Outdated

Comment thread src/doc/object_type.h

dacap force-pushed the new-layers branch from 08168e6 to 0c8ac14 Compare August 13, 2025 12:53

aseprite-bot reviewed Aug 13, 2025

View reviewed changes

Comment thread src/doc/layer.cpp Outdated

Comment thread src/doc/layer.cpp Outdated

Comment thread src/doc/layer.cpp Outdated

Comment thread src/doc/layer.cpp Outdated

dacap force-pushed the new-layers branch 2 times, most recently from cdc75c1 to 5211ba3 Compare August 14, 2025 17:35

aseprite-bot reviewed Aug 14, 2025

View reviewed changes

dacap force-pushed the new-layers branch from 8124506 to d65166c Compare August 21, 2025 21:55

aseprite-bot reviewed Aug 22, 2025

View reviewed changes

Comment thread src/app/doc.cpp Outdated

Comment thread src/doc/layer_io.cpp Outdated

dacap force-pushed the new-layers branch 2 times, most recently from de2632a to 0e7350d Compare August 22, 2025 12:47

dacap force-pushed the new-layers branch from 0e7350d to 04fc206 Compare September 5, 2025 12:19

dacap self-assigned this Sep 5, 2025

aseprite-bot reviewed Sep 5, 2025

View reviewed changes

dacap force-pushed the new-layers branch from 64d1a86 to 9cdcb36 Compare September 25, 2025 13:11

aseprite-bot reviewed Sep 25, 2025

View reviewed changes

dacap force-pushed the new-layers branch from 147a230 to dbc4f35 Compare November 20, 2025 19:45

aseprite-bot reviewed Nov 20, 2025

View reviewed changes

This was referenced Dec 9, 2025

Selecting a slice flags the file as modified #5578

Closed

Fix slices issues preferring just one slice keyframe #5580

Merged

dacap force-pushed the new-layers branch from dbc4f35 to 383e99d Compare February 26, 2026 17:55

dacap marked this pull request as ready for review February 26, 2026 17:56

aseprite-bot reviewed Feb 26, 2026

View reviewed changes

dacap force-pushed the new-layers branch from 383e99d to a97d76c Compare February 26, 2026 18:12

aseprite-bot reviewed Feb 27, 2026

View reviewed changes

ckaiser reviewed Mar 2, 2026

View reviewed changes

dacap changed the title ~~New layer types~~ Refactors needed for new layer types Mar 2, 2026

dacap added 2 commits March 2, 2026 14:10

Initial work for new layer types: Add "type" arg to NewLayerCommand

be1e1ad

Move the list of cels from doc::LayerImage to doc::Layer

a41de6d

We're going to make the Cel concept as a container of any kind of data related to a layer/frame (e.g. audio, vector, text, etc.).

dacap added 9 commits March 2, 2026 14:24

Add support for cels with no image (aseprite#1303)

a1e2940

This will be a work-in-progress, as many operations might fail as they still expect a cel with an image.

Minor changes in DuplicateLayerCommand for a more generic approach

a1f3fcf

Hide blend mode/opacity from Layer Properties when they are unnecessary

540553c

Make serialization/crash data more generic for future layer types

afe47d3

Move children/sublayers from LayerGroup to Layer

964153a

Now any kind of layer can be a parent layer (not only groups). This is because we want to be able to associate FX/Mask layers as children of a specific layer in the future.

Avoid creating cels in layer groups w/drag & drop or copy & paste on …

1bd83d3

…timeline

Implement cmd::SetCelImage with a suspended image (due to aseprite#5437)

532f569

We also save the bounds of the cel as it's required to restore the bounds when we restore the image if we SetCelImage(cel, nullptr) and then undo.

Add cmd::SetCelPosition() ctor with gfx::Point

4e1fa3a

Limit doc::ObjectType to uint16_t as it's our limit to serialize obje…

2e55451

…ct types

dacap force-pushed the new-layers branch from 7b3de1c to 2e55451 Compare March 2, 2026 17:58

aseprite-bot reviewed Mar 2, 2026

View reviewed changes

dacap added this to the v1.3.18-beta2 milestone Mar 2, 2026

dacap merged commit 2e55451 into aseprite:beta Mar 2, 2026
12 checks passed

dacap deleted the new-layers branch March 2, 2026 18:29

dacap added a commit to dacap/aseprite that referenced this pull request Mar 2, 2026

[wip] Add new layer types (continuation of aseprite#5347)

858c800

dacap mentioned this pull request Mar 2, 2026

[wip] Add new layer types (continuation of #5347) #5683

Draft

dacap mentioned this pull request Apr 1, 2026

Latest features of Aseprite v1.3.x #4180

Open

8 tasks

	const bool has_data = (flags & 1 ? true : false);
	const bool has_data = ((flags & 1) != 0);

	const bool has_image = (flags & 2 ? true : false);
	const bool has_image = ((flags & 2) != 0);

	ImageRef image(read_image(m_stream));
	ImageRef const image(read_image(m_stream));

	ImageRef oldImage = sprite->getImageRef(m_oldImageId);
	ImageRef const oldImage = sprite->getImageRef(m_oldImageId);

	fromLeft = (left && left->image() && left->imageId() == cel->imageId());
	fromLeft = ((left != nullptr) && left->image() && left->imageId() == cel->imageId());

	auto layerTilemap = static_cast<doc::LayerTilemap*>(srcCel->layer());
	auto layerTilemap = static_cast<doc::LayerTilemap>(srcCel->layer());

	auto dstLayerTilemap = static_cast<doc::LayerTilemap*>(dstLayer);
	auto dstLayerTilemap = static_cast<doc::LayerTilemap>(dstLayer);

	gfx::Point newPosition = (m_cel->position() +
	gfx::Point const newPosition = (m_cel->position() +

	ObjectId celId = read32(s);
	ObjectId const celId = read32(s);

	void Doc::copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer) const
	void Doc::copyLayerContent(const Layer* sourceLayer, Doc* destDoc, Layer* destLayer)

	return true;
	if (c->isGroup() && has_child(c, child))
	return true;

	CelIterator end = getCelEnd();
	CelIterator const end = getCelEnd();

	ImageRef oldImage = cel->imageRef();
	ImageRef const oldImage = cel->imageRef();

	ImageRef currentImage = cel->imageRef();
	ImageRef const currentImage = cel->imageRef();

	gfx::Rect currentBounds = cel->bounds();
	gfx::Rect const currentBounds = cel->bounds();

	fromLeft = (left && left->dataId() == cel->dataId());
	fromLeft = ((left != nullptr) && left->dataId() == cel->dataId());

	fromRight = (right && right->dataId() == cel->dataId());
	fromRight = ((right != nullptr) && right->dataId() == cel->dataId());

Uh oh!

Conversation

dacap commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aseprite-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

dacap commented Aug 13, 2025 •

edited

Loading