Skip to content

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

Merged
dacap merged 1 commit into
aseprite:mainfrom
ckaiser:fix-05f2a1e-regression
Apr 8, 2026
Merged

Fix regression crash when removing cel in tool loop (fix #5717)#5720
dacap merged 1 commit into
aseprite:mainfrom
ckaiser:fix-05f2a1e-regression

Conversation

@ckaiser

@ckaiser ckaiser commented Mar 26, 2026

Copy link
Copy Markdown
Member

Regression from 05f2a1e, because of the reworked logic we were calling getDestTileset() even when we created a new cel, so the new cel was never attached to a layer and we crashed when we removed it later.

This logic could probably be simplified a bunch with #5684. Related: #5678

@ckaiser ckaiser requested a review from dacap as a code owner March 26, 2026 07:52
@aseprite-bot

Copy link
Copy Markdown
Collaborator

clang-tidy review says "All clean, LGTM! 👍"

miguelfabiao125-bit added a commit to miguelfabiao125-bit/aseprite that referenced this pull request Mar 27, 2026
Upon investigating the cell-addition logic for tilemap layers, I found that Manual Mode was creating new cels without properly attaching them. While PR aseprite#5720 resolved the immediate crash, the first interaction in Manual Mode was still incorrectly committing a tile.

To fix this, I implemented an early exit in the commit function that clears the pending data and opts out of the commit process when in Manual Mode. Currently, this leaves "ghost pixels" on the canvas, which I suspect are uncleaned preview tiles. I am working on a fix for the ghosting, but I would appreciate feedback on whether this "early exit" approach is the correct architectural path for the original bug.
miguelfabiao125-bit added a commit to miguelfabiao125-bit/aseprite that referenced this pull request Mar 27, 2026
Upon investigating the cell-addition logic for tilemap layers, I found
that Manual Mode was creating new cels without properly attaching them.
While PR aseprite#5720 resolved the immediate crash, the first interaction in
Manual Mode was still incorrectly committing a tile.

To fix this, I implemented an early exit in the commit function that
clears the pending data and opts out of the commit process when in
Manual Mode. Currently, this leaves "ghost pixels" on the canvas, which
I suspect are uncleaned preview tiles. I am working on a fix for the
ghosting, but I would appreciate feedback on whether this "early exit"
approach is the correct architectural path for the original bug.
miguelfabiao125-bit added a commit to miguelfabiao125-bit/aseprite that referenced this pull request Mar 27, 2026
Upon investigating the cell-addition logic for tilemap layers, I found
that Manual Mode was creating new cels without properly attaching them.
While PR aseprite#5720 resolved the immediate crash, the first interaction in
Manual Mode was still incorrectly committing a tile.

To fix this, I implemented an early exit in the commit function that
clears the pending data and opts out of the commit process when in
Manual Mode. Currently, this leaves "ghost pixels" on the canvas, which
I suspect are uncleaned preview tiles. I am working on a fix for the
ghosting, but I would appreciate feedback on whether this "early exit"
approach is a good fix for the original bug.
miguelfabiao125-bit added a commit to miguelfabiao125-bit/aseprite that referenced this pull request Mar 27, 2026
Upon investigating the cell-addition logic for tilemap layers, I found
that Manual Mode was creating new cels without properly attaching them.
While PR aseprite#5720 resolved the immediate crash, the first interaction in
Manual Mode was still incorrectly committing a tile.

To fix this, I implemented an early exit in the commit function that
clears the pending data and opts out of the commit process when in
Manual Mode. Currently, this leaves "ghost pixels" on the canvas, which
I suspect are uncleaned preview tiles. I am working on a fix for the
ghosting, but I would appreciate feedback on whether this "early exit"
approach is the correct architectural path for the original bug.
miguelfabiao125-bit added a commit to miguelfabiao125-bit/aseprite that referenced this pull request Mar 30, 2026
Upon investigating the cell-addition logic for tilemap layers, I found
that Manual Mode was creating new cels without properly attaching them.
While PR aseprite#5720 resolved the immediate crash, the first interaction in
Manual Mode was still incorrectly committing a tile.

To resolve this, I implemented an early exit within the commit function to bypass the data-writing process when in Manual Mode. However, this initially prevented the preview layer from being cleared.

To ensure a consistent UI state, I updated drawing_state to explicitly clean the preview buffer when the drawing loop is destroyed. This ensures that "ghost" pixels are removed even when a commit is opted out.
miguelfabiao125-bit added a commit to miguelfabiao125-bit/aseprite that referenced this pull request Mar 30, 2026
Upon investigating the cell-addition logic for tilemap layers, I found that Manual Mode was creating new cels without properly attaching them. While PR aseprite#5720 resolved the immediate crash, the first interaction in Manual Mode was still incorrectly committing a tile.

To resolve this, I implemented an early exit within the commit function to bypass the data-writing process when in Manual Mode. However, this initially prevented the preview layer from being cleared.

To ensure a consistent UI state, I updated drawing_state to explicitly clean the preview buffer when the drawing loop is destroyed. This ensures that "ghost" pixels are removed even when a commit is opted out.

I have also added a test to make sure no other crash or tile creation is added by the manual tilemode in the future.
miguelfabiao125-bit added a commit to miguelfabiao125-bit/aseprite that referenced this pull request Apr 4, 2026
Upon investigating the cell-addition logic for tilemap layers, I found that Manual Mode was creating new cels without properly attaching them. While PR aseprite#5720 resolved the immediate crash, the first interaction in Manual Mode was still incorrectly committing a tile.

To resolve this, I implemented an early exit within the commit function to bypass the data-writing process when in Manual Mode. However, this initially prevented the preview layer from being cleared.

To ensure a consistent UI state, I updated drawing_state to explicitly clean the preview buffer when the drawing loop is destroyed. This ensures that "ghost" pixels are removed even when a commit is opted out.

I have also added a test to make sure no other crash or tile creation is added by the manual tilemode in the future.
miguelfabiao125-bit added a commit to miguelfabiao125-bit/aseprite that referenced this pull request Apr 4, 2026
Upon investigating the cell-addition logic for tilemap layers, I found that Manual Mode was
creating new cels without properly attaching them. While PR aseprite#5720 resolved the immediate
crash, the first interaction in Manual Mode was still incorrectly committing a tile.

To resolve this, I implemented an early exit within the commit function to bypass the
data-writing process when in Manual Mode. However, this initially prevented the preview
layer from being cleared. To ensure a consistent UI state, I updated drawing_state to
explicitly clean the preview buffer when the drawing loop is destroyed.

This ensures that "ghost" pixels are removed even when a commit is opted out. I have also
added a test to make sure no other crash or tile creation is added by the manual tilemode
in the future.
@dacap dacap self-assigned this Apr 8, 2026
@dacap

dacap commented Apr 8, 2026

Copy link
Copy Markdown
Member

I'll merge this one as it fixes the crash, and then continue with #5732 as I think the Manual mode shouldn't create new tiles.

@dacap dacap merged commit 4f7df5f into aseprite:main Apr 8, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants