Skip to content

Fix crash dropping file on timeline#5295

Merged
dacap merged 2 commits into
aseprite:mainfrom
martincapello:fix-crash-dropping-file-timeline
Aug 11, 2025
Merged

Fix crash dropping file on timeline#5295
dacap merged 2 commits into
aseprite:mainfrom
martincapello:fix-crash-dropping-file-timeline

Conversation

@martincapello

Copy link
Copy Markdown
Member

Fix #5289

@martincapello martincapello requested a review from dacap as a code owner July 23, 2025 19:09

@aseprite-bot aseprite-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread src/app/ui/timeline/timeline.cpp Outdated
@martincapello martincapello force-pushed the fix-crash-dropping-file-timeline branch from 63991e4 to 1e55667 Compare July 23, 2025 20:56
@aseprite-bot

Copy link
Copy Markdown
Collaborator

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

@dacap

dacap commented Jul 25, 2025

Copy link
Copy Markdown
Member

LGTM, just tested it and the fix works! 👍 The only thing we could try to improve is getLayerIndexFromSprite which looks like a function that could be part of the same doc::Layer (probably with the get_layer_index part in doc::LayerGroup).

@dacap dacap assigned martincapello and unassigned dacap Jul 25, 2025
@martincapello martincapello force-pushed the fix-crash-dropping-file-timeline branch from 1e55667 to 1856ab9 Compare August 6, 2025 21:11
@martincapello martincapello assigned dacap and unassigned martincapello Aug 6, 2025

@aseprite-bot aseprite-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread src/doc/layer.cpp
layer->displaceFrames(fromThis, delta);
}

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

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

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

    if ((child->isGroup() && static_cast<LayerGroup*>(child)->getLayerIndex(layer, index) != -1) ||
                             ^

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

    if ((child->isGroup() && static_cast<LayerGroup*>(child)->getLayerIndex(layer, index) != -1) ||
                             ^

@dacap dacap merged commit 1227f9c into aseprite:main Aug 11, 2025
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.

Crash dropping file on timeline

3 participants