Fix issue #310 - crash on export sprite sheet

The timeline wasn't being added as a Document observer, so it wasn't
getting notifications about layers modifications. In this way the timeline
wasn't able to update its m_layers[] internal field and was accessing to
removed layers/invalid memory.
This commit is contained in:
David Capello 2014-03-29 16:31:39 -03:00
parent 03020d7bcf
commit 395be62b03
7 changed files with 83 additions and 63 deletions

View File

@ -632,7 +632,7 @@ void DocumentApi::removeLayer(Layer* layer)
DocumentEvent ev(m_document);
ev.sprite(layer->getSprite());
ev.layer(layer);
m_document->notifyObservers<DocumentEvent&>(&DocumentObserver::onRemoveLayer, ev);
m_document->notifyObservers<DocumentEvent&>(&DocumentObserver::onBeforeRemoveLayer, ev);
// Add undoers.
if (undoEnabled())
@ -640,6 +640,9 @@ void DocumentApi::removeLayer(Layer* layer)
// Do the action.
layer->getParent()->removeLayer(layer);
m_document->notifyObservers<DocumentEvent&>(&DocumentObserver::onAfterRemoveLayer, ev);
delete layer;
}

View File

@ -41,7 +41,8 @@ namespace app {
virtual void onRemoveSprite(DocumentEvent& ev) { }
virtual void onRemoveLayer(DocumentEvent& ev) { }
virtual void onBeforeRemoveLayer(DocumentEvent& ev) { }
virtual void onAfterRemoveLayer(DocumentEvent& ev) { }
// Called when a frame is removed. It's called after the frame was
// removed, and the sprite's total number of frames is modified.

View File

@ -218,7 +218,7 @@ void DocumentView::onAddLayer(DocumentEvent& ev)
}
}
void DocumentView::onRemoveLayer(DocumentEvent& ev)
void DocumentView::onBeforeRemoveLayer(DocumentEvent& ev)
{
Sprite* sprite = ev.sprite();
Layer* layer = ev.layer();

View File

@ -64,7 +64,7 @@ namespace app {
void onSpritePixelsModified(DocumentEvent& ev) OVERRIDE;
void onLayerMergedDown(DocumentEvent& ev) OVERRIDE;
void onAddLayer(DocumentEvent& ev) OVERRIDE;
void onRemoveLayer(DocumentEvent& ev) OVERRIDE;
void onBeforeRemoveLayer(DocumentEvent& ev) OVERRIDE;
void onAddFrame(DocumentEvent& ev) OVERRIDE;
void onRemoveFrame(DocumentEvent& ev) OVERRIDE;
void onTotalFramesChanged(DocumentEvent& ev) OVERRIDE;

View File

@ -180,6 +180,8 @@ void Timeline::updateUsingEditor(Editor* editor)
DocumentView* view = m_editor->getDocumentView();
view->getDocumentLocation(&location);
location.document()->addObserver(this);
// If we are already in the same position as the "editor", we don't
// need to update the at all timeline.
if (m_document == location.document() &&
@ -859,72 +861,82 @@ void Timeline::onPreferredSize(PreferredSizeEvent& ev)
void Timeline::onPaint(ui::PaintEvent& ev)
{
Graphics* g = ev.getGraphics();
bool noDoc = (m_document == NULL);
if (noDoc)
goto paintNoDoc;
if (!m_document) {
drawPart(g, getClientBounds(), NULL, m_timelinePaddingStyle);
return;
}
try {
// Lock the sprite to read/render it.
const DocumentReader documentReader(m_document);
int layer, first_layer, last_layer;
FrameNumber frame, first_frame, last_frame;
int layer, first_layer, last_layer;
FrameNumber frame, first_frame, last_frame;
getDrawableLayers(g, &first_layer, &last_layer);
getDrawableFrames(g, &first_frame, &last_frame);
getDrawableLayers(g, &first_layer, &last_layer);
getDrawableFrames(g, &first_frame, &last_frame);
// Draw the header for layers.
drawHeader(g);
// Draw the header for layers.
drawHeader(g);
// Draw the header for each visible frame.
{
IntersectClip clip(g, getFrameHeadersBounds());
if (clip) {
for (frame=first_frame; frame<=last_frame; ++frame)
drawHeaderFrame(g, frame);
// Draw the header for each visible frame.
{
IntersectClip clip(g, getFrameHeadersBounds());
if (clip) {
for (frame=first_frame; frame<=last_frame; ++frame)
drawHeaderFrame(g, frame);
// Draw onionskin indicators.
gfx::Rect bounds = getOnionskinFramesBounds();
if (!bounds.isEmpty()) {
drawPart(g, bounds,
NULL, m_timelineOnionskinRangeStyle,
false, false, false);
// Draw onionskin indicators.
gfx::Rect bounds = getOnionskinFramesBounds();
if (!bounds.isEmpty()) {
drawPart(g, bounds,
NULL, m_timelineOnionskinRangeStyle,
false, false, false);
}
}
}
// Draw each visible layer.
for (layer=first_layer; layer<=last_layer; layer++) {
{
IntersectClip clip(g, getLayerHeadersBounds());
if (clip)
drawLayer(g, layer);
}
// Get the first CelIterator to be drawn (it is the first cel with cel->frame >= first_frame)
CelIterator it, end;
Layer* layerPtr = m_layers[layer];
if (layerPtr->isImage()) {
it = static_cast<LayerImage*>(layerPtr)->getCelBegin();
end = static_cast<LayerImage*>(layerPtr)->getCelEnd();
for (; it != end && (*it)->getFrame() < first_frame; ++it)
;
}
IntersectClip clip(g, getCelsBounds());
if (!clip)
continue;
// Draw every visible cel for each layer.
for (frame=first_frame; frame<=last_frame; ++frame) {
Cel* cel = (layerPtr->isImage() && it != end && (*it)->getFrame() == frame ? *it: NULL);
drawCel(g, layer, frame, cel);
if (cel)
++it; // Go to next cel
}
}
drawPaddings(g);
}
catch (const LockedDocumentException&) {
noDoc = true;
}
// Draw each visible layer.
for (layer=first_layer; layer<=last_layer; layer++) {
{
IntersectClip clip(g, getLayerHeadersBounds());
if (clip)
drawLayer(g, layer);
}
// Get the first CelIterator to be drawn (it is the first cel with cel->frame >= first_frame)
CelIterator it, end;
Layer* layerPtr = m_layers[layer];
if (layerPtr->isImage()) {
it = static_cast<LayerImage*>(layerPtr)->getCelBegin();
end = static_cast<LayerImage*>(layerPtr)->getCelEnd();
for (; it != end && (*it)->getFrame() < first_frame; ++it)
;
}
IntersectClip clip(g, getCelsBounds());
if (!clip)
continue;
// Draw every visible cel for each layer.
for (frame=first_frame; frame<=last_frame; ++frame) {
Cel* cel = (layerPtr->isImage() && it != end && (*it)->getFrame() == frame ? *it: NULL);
drawCel(g, layer, frame, cel);
if (cel)
++it; // Go to next cel
}
}
drawPaddings(g);
paintNoDoc:;
if (noDoc)
drawPart(g, getClientBounds(), NULL, m_timelinePaddingStyle);
}
void Timeline::onCommandAfterExecution(Context* context)
@ -954,7 +966,7 @@ void Timeline::onAddLayer(DocumentEvent& ev)
invalidate();
}
void Timeline::onRemoveLayer(DocumentEvent& ev)
void Timeline::onAfterRemoveLayer(DocumentEvent& ev)
{
Sprite* sprite = ev.sprite();
Layer* layer = ev.layer();

View File

@ -114,7 +114,7 @@ namespace app {
// DocumentObserver impl.
void onAddLayer(DocumentEvent& ev) OVERRIDE;
void onRemoveLayer(DocumentEvent& ev) OVERRIDE;
void onAfterRemoveLayer(DocumentEvent& ev) OVERRIDE;
void onAddFrame(DocumentEvent& ev) OVERRIDE;
void onRemoveFrame(DocumentEvent& ev) OVERRIDE;

View File

@ -39,6 +39,7 @@ public:
// collection and will be destroyed calling the T::dispose() member
// function.
void addObserver(observer_type* observer) {
ASSERT(std::find(m_observers.begin(), m_observers.end(), observer) == m_observers.end() && "You've tried to add an observer that already is in the collection");
m_observers.push_back(observer);
}
@ -48,6 +49,9 @@ public:
iterator it = std::find(m_observers.begin(), m_observers.end(), observer);
if (it != end())
m_observers.erase(it);
else {
ASSERT(false && "You've tried to remove an observer that isn't in the collection");
}
}
void notifyObservers(void (observer_type::*method)()) {