Keep empty cels by dacap · Pull Request #5684 · aseprite/aseprite

dacap · 2026-03-02T20:16:49Z

Should fix all issues from #1303.

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2026-03-02T20:21:43Z

      Cel* c = layer->cel(fr);
-      if (c && c->dataRef().get() == celData)
-        add(new cmd::RemoveCel(c));
+      if (c && c->dataRef().get() == celData) {


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

Suggested change

if (c && c->dataRef().get() == celData) {

if ((c != nullptr) && c->dataRef().get() == celData) {

aseprite-bot · 2026-03-02T20:21:43Z

 {
  auto& styles = skinTheme()->styles;
  const ObjectId dataId = (*data->activeIt)->dataId();
+  const bool has_image = ((*data->activeIt)->image() ? true : false);


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

Suggested change

const bool has_image = ((*data->activeIt)->image() ? true : false);

const bool has_image = (static_cast<bool>((*data->activeIt)->image()));

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2026-03-02T22:12:18Z

    else {
      ASSERT(site.layer()->isTilemap());
-      if (auto cel = site.cel()) {
+      if (auto cel = site.cel(); cel && cel->image()) {


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

Suggested change

if (auto cel = site.cel(); cel && cel->image()) {

if (auto *cel = site.cel(); cel && cel->image()) {

aseprite-bot

clang-tidy made some suggestions

aseprite-bot · 2026-03-04T13:14:48Z

    // Same size of cel image and background image, we can just
    // replace pixels.
-    if (bg_image->width() == cel_image->width() && bg_image->height() == cel_image->height()) {
+    if (cel_image && bg_image->size() == cel_image->size()) {


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

Suggested change

if (cel_image && bg_image->size() == cel_image->size()) {

if ((cel_image != nullptr) && bg_image->size() == cel_image->size()) {

aseprite-bot · 2026-03-04T13:14:48Z

    Cel* cel = layer->cel(frame);
    if (!cel) {
-      ImageRef cel_image(Image::create(sprite->pixelFormat(), sprite->width(), sprite->height()));
+      ImageRef cel_image(Image::create(sprite->spec()));


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

Suggested change

ImageRef cel_image(Image::create(sprite->spec()));

ImageRef const cel_image(Image::create(sprite->spec()));

aseprite-bot · 2026-03-04T13:14:48Z

  if (!mask->bitmap())
    return;

+  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-03-04T13:14:48Z


-        doc::PixelFormat pixelFormat = (cel->layer()->isTilemap() ? sprite->pixelFormat() :
-                                                                    cel->image()->pixelFormat());
+        doc::PixelFormat pixelFormat = (!cel->image() || cel->layer()->isTilemap() ?


warning: variable 'pixelFormat' of type 'doc::PixelFormat' can be declared 'const' [misc-const-correctness]

Suggested change

doc::PixelFormat pixelFormat = (!cel->image() || cel->layer()->isTilemap() ?

doc::PixelFormat const pixelFormat = (!cel->image() || cel->layer()->isTilemap() ?

aseprite-bot · 2026-03-04T13:14:48Z

+  else if (Cel* c = cel()) {
+    if (c->image() && m_layer && m_layer->canEditPixels())
      return CelList{ c };
  }


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

Suggested change

}

if (Cel* c = cel()) {

if (c->image() && m_layer && m_layer->canEditPixels())

return CelList{ c };

}

aseprite-bot · 2026-03-04T13:14:48Z

-        if (algorithm::shrink_bounds(dstCel->image(), bg, dstLayer, bounds)) {
-          ImageRef trimmed(doc::crop_image(dstCel->image(), bounds, bg));
+        if (algorithm::shrink_bounds(dstImage.get(), bg, dstLayer, bounds)) {
+          ImageRef trimmed(doc::crop_image(dstImage.get(), bounds, bg));


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

Suggested change

ImageRef trimmed(doc::crop_image(dstImage.get(), bounds, bg));

ImageRef const trimmed(doc::crop_image(dstImage.get(), bounds, bg));

aseprite-bot · 2026-03-04T13:14:48Z

-        ASSERT(!ctx->activeSite().cel() || ctx->activeSite().cel()->image());
+
+        Cel* cel = ctx->activeSite().cel();
+        const bool isAnImageOnDstCel = (cel && cel->image());


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

Suggested change

const bool isAnImageOnDstCel = (cel && cel->image());

const bool isAnImageOnDstCel = ((cel != nullptr) && cel->image());

aseprite-bot · 2026-03-04T13:14:48Z

-        ASSERT(!ctx->activeSite().cel() || ctx->activeSite().cel()->image());
+
+        Cel* cel = ctx->activeSite().cel();
+        const bool isAnImageOnDstCel = (cel && cel->image());


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

Suggested change

const bool isAnImageOnDstCel = (cel && cel->image());

const bool isAnImageOnDstCel = (cel && (cel->image() != nullptr));

aseprite-bot · 2026-03-04T13:14:48Z

+    // Resize the image
+    const int w = std::max(1, int(scale.w * image->width()));
+    const int h = std::max(1, int(scale.h * image->height()));
+    doc::ImageRef newImage(


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

Suggested change

doc::ImageRef newImage(

doc::ImageRef const newImage(

dacap · 2026-03-04T13:25:01Z

@ckaiser would you mind to take a look to this? It's a quite large change, I'd like to add some tests and probably change some APIs as the code looks like a mess for all the given cases.

ckaiser

Some things I found while testing:

FlattenLayers crashed when merging down in some cases
Linked Tilemaps behave differently in the UI than normal layers, if you link two tilemap cels that have no image data, then draw something in one of them and then undo that, the timeline won't go back to showing the "hollow" cel indicator.
When merging a layer with a cel with user data into a layer with an empty cel, the user data is not preserved regardless of it had an image before, I think this happens without this patch, so it's default behavior but it feels strange to not merge user data.

dacap · 2026-03-04T22:11:09Z

Thanks for testing @ckaiser. As we talked I'll see how we can automate all these kind of tests before merging this PR.

…seprite#1833)

aseprite-bot reviewed Mar 2, 2026

View reviewed changes

dacap force-pushed the keep-empty-cels branch from 140cdf0 to edef945 Compare March 2, 2026 22:07

aseprite-bot reviewed Mar 2, 2026

View reviewed changes

dacap force-pushed the keep-empty-cels branch from edef945 to cde669c Compare March 4, 2026 13:05

aseprite-bot reviewed Mar 4, 2026

View reviewed changes

dacap assigned ckaiser Mar 4, 2026

dacap marked this pull request as ready for review March 4, 2026 13:24

ckaiser reviewed Mar 4, 2026

View reviewed changes

dacap assigned dacap and unassigned ckaiser Mar 4, 2026

dacap marked this pull request as draft March 5, 2026 10:40

ckaiser mentioned this pull request Mar 26, 2026

Fix regression crash when removing cel in tool loop (fix #5717) #5720

Merged

dacap linked an issue Jun 30, 2026 that may be closed by this pull request

Associate user data to an empty cel #1303

Open

5 tasks

dacap added 3 commits June 30, 2026 09:04

Preserve empty cels (even links) after erasing all their pixels (fix a…

32a2450

…seprite#1833)

[.aseprite] Add support to load/save empty cels

8822278

Fix several cases where an non-null image was expected for a given cel

716ddbd

dacap force-pushed the keep-empty-cels branch from cde669c to 716ddbd Compare June 30, 2026 12:18

	if (c && c->dataRef().get() == celData) {
	if ((c != nullptr) && c->dataRef().get() == celData) {

	const bool has_image = ((*data->activeIt)->image() ? true : false);
	const bool has_image = (static_cast<bool>((*data->activeIt)->image()));

	if (auto cel = site.cel(); cel && cel->image()) {
	if (auto *cel = site.cel(); cel && cel->image()) {

	if (cel_image && bg_image->size() == cel_image->size()) {
	if ((cel_image != nullptr) && bg_image->size() == cel_image->size()) {

	ImageRef cel_image(Image::create(sprite->spec()));
	ImageRef const cel_image(Image::create(sprite->spec()));

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

	doc::PixelFormat pixelFormat = (!cel->image() \|\| cel->layer()->isTilemap() ?
	doc::PixelFormat const pixelFormat = (!cel->image() \|\| cel->layer()->isTilemap() ?

	ImageRef trimmed(doc::crop_image(dstImage.get(), bounds, bg));
	ImageRef const trimmed(doc::crop_image(dstImage.get(), bounds, bg));

	const bool isAnImageOnDstCel = (cel && cel->image());
	const bool isAnImageOnDstCel = ((cel != nullptr) && cel->image());

	const bool isAnImageOnDstCel = (cel && cel->image());
	const bool isAnImageOnDstCel = (cel && (cel->image() != nullptr));

Uh oh!

Uh oh!

Conversation

dacap commented Mar 2, 2026 • 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!

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!

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!

dacap commented Mar 4, 2026

Uh oh!

ckaiser left a comment

Choose a reason for hiding this comment

Uh oh!

dacap commented Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dacap commented Mar 2, 2026 •

edited

Loading