Skip to content

Commit

Permalink
Optimize ToolLoop with modified regions of pixels
Browse files Browse the repository at this point in the history
Here we avoid copying and clearing pixels that will not be used
in the whole tool loop process.
Changes:
* Add several member functions in ToolLoop to validate/invalidate regions
  of source/destination images so we know what regions are safe to use
  by inks and can be shown in the editor
* Add new DocumentObserver::onExposeSpritePixels() member to validate
  pixels that will be displayed in the editor
* Add Ink::needs/createSpecialSourceArea() member functions to validate
  extra areas for inks like blur or jumble
* Add undoers::ModifiedRegion to save the undo information about the
  modified region
* Add ShowHideDrawingCursor class
* Change "blur" tool policy from overlap to accumulate

(This is a real fix for issue #239)
  • Loading branch information
dacap committed Dec 8, 2014
1 parent 07c7756 commit afbd3b2
Show file tree
Hide file tree
Showing 49 changed files with 1,057 additions and 599 deletions.
14 changes: 7 additions & 7 deletions data/gui.xml
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@
controller="freehand"
pointshape="pixel"
intertwine="as_lines"
tracepolicy="accumulative">
tracepolicy="accumulate">
<tooltip>*
Left-button: Replace/add to current selection.&#10;*
Right-button: Remove from current selection.
Expand All @@ -723,7 +723,7 @@
ink="selection"
controller="one_point"
pointshape="floodfill"
tracepolicy="accumulative">
tracepolicy="accumulate">
<tooltip>*
Left-button: Replace/add to current selection.&#10;*
Right-button: Remove from current selection.
Expand All @@ -738,7 +738,7 @@
controller="freehand"
pointshape="brush"
intertwine="as_lines"
tracepolicy="accumulative"
tracepolicy="accumulate"
/>
<tool id="spray"
text="Spray Tool"
Expand All @@ -757,7 +757,7 @@
controller="freehand"
pointshape="brush"
intertwine="as_lines"
tracepolicy="accumulative"
tracepolicy="accumulate"
default_brush_size="8">
<tooltip>*
Left-button: Erase with the background color in `Background' layer&#10;
Expand Down Expand Up @@ -810,7 +810,7 @@
ink="paint"
controller="one_point"
pointshape="floodfill"
tracepolicy="accumulative"
tracepolicy="accumulate"
/>
</group>

Expand Down Expand Up @@ -880,7 +880,7 @@
controller="freehand"
pointshape="brush"
intertwine="as_lines"
tracepolicy="accumulative"
tracepolicy="accumulate"
/>
<tool id="polygon"
text="Polygon Tool"
Expand All @@ -900,7 +900,7 @@
controller="freehand"
pointshape="brush"
intertwine="as_lines"
tracepolicy="overlap"
tracepolicy="accumulate"
default_brush_size="16"
/>
<tool id="jumble"
Expand Down
1 change: 1 addition & 0 deletions src/app/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ add_library(app-lib
undoers/dirty_area.cpp
undoers/flip_image.cpp
undoers/image_area.cpp
undoers/modified_region.cpp
undoers/move_layer.cpp
undoers/open_group.cpp
undoers/remap_palette.cpp
Expand Down
3 changes: 2 additions & 1 deletion src/app/commands/cmd_export_sprite_sheet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,8 @@ void ExportSpriteSheetCommand::onExecute(Context* context)
// destination clipping bounds in Sprite::render() function.
tempImage->clear(0);
sprite->render(tempImage, 0, 0, frame);
resultImage->copy(tempImage, column*sprite->width(), row*sprite->height());
resultImage->copy(tempImage, column*sprite->width(), row*sprite->height(),
0, 0, tempImage->width(), tempImage->height());

if (++column >= columns) {
column = 0;
Expand Down
3 changes: 1 addition & 2 deletions src/app/commands/cmd_preview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,7 @@ class PreviewWindow : public Window {

ImageBufferPtr buf = Editor::getRenderImageBuffer();
m_render.reset(
renderEngine.renderSprite(
0, 0, m_sprite->width(), m_sprite->height(),
renderEngine.renderSprite(m_sprite->bounds(),
m_editor->frame(), Zoom(1, 1), false, false, buf));
}

Expand Down
8 changes: 8 additions & 0 deletions src/app/document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ void Document::notifySpritePixelsModified(Sprite* sprite, const gfx::Region& reg
notifyObservers<doc::DocumentEvent&>(&doc::DocumentObserver::onSpritePixelsModified, ev);
}

void Document::notifyExposeSpritePixels(Sprite* sprite, const gfx::Region& region)
{
doc::DocumentEvent ev(this);
ev.sprite(sprite);
ev.region(region);
notifyObservers<doc::DocumentEvent&>(&doc::DocumentObserver::onExposeSpritePixels, ev);
}

void Document::notifyLayerMergedDown(Layer* srcLayer, Layer* targetLayer)
{
doc::DocumentEvent ev(this);
Expand Down
1 change: 1 addition & 0 deletions src/app/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ namespace app {

void notifyGeneralUpdate();
void notifySpritePixelsModified(Sprite* sprite, const gfx::Region& region);
void notifyExposeSpritePixels(Sprite* sprite, const gfx::Region& region);
void notifyLayerMergedDown(Layer* srcLayer, Layer* targetLayer);
void notifyCelMoved(Layer* fromLayer, FrameNumber fromFrame, Layer* toLayer, FrameNumber toFrame);
void notifyCelCopied(Layer* fromLayer, FrameNumber fromFrame, Layer* toLayer, FrameNumber toFrame);
Expand Down
14 changes: 6 additions & 8 deletions src/app/document_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1093,11 +1093,10 @@ void DocumentApi::flattenLayers(Sprite* sprite)

// We have to save the current state of `cel_image' in the undo.
if (undoEnabled()) {
Dirty* dirty = new Dirty(cel_image, image, image->bounds());
dirty->saveImagePixels(cel_image);
Dirty dirty(cel_image, image, image->bounds());
dirty.saveImagePixels(cel_image);
m_undoers->pushUndoer(new undoers::DirtyArea(
getObjects(), cel_image, dirty));
delete dirty;
getObjects(), cel_image, &dirty));
}
}
else {
Expand Down Expand Up @@ -1296,10 +1295,9 @@ void DocumentApi::flipImageWithMask(Layer* layer, Image* image, const Mask* mask

// Insert the undo operation.
if (undoEnabled()) {
base::UniquePtr<Dirty> dirty((new Dirty(image, flippedImage, image->bounds())));
dirty->saveImagePixels(image);

m_undoers->pushUndoer(new undoers::DirtyArea(getObjects(), image, dirty));
Dirty dirty(image, flippedImage, image->bounds());
dirty.saveImagePixels(image);
m_undoers->pushUndoer(new undoers::DirtyArea(getObjects(), image, &dirty));
}

// Copy the flipped image into the image specified as argument.
Expand Down
11 changes: 8 additions & 3 deletions src/app/settings/ui_settings_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1026,18 +1026,23 @@ class UIToolSettingsImpl

tools::ToolBox* toolBox = App::instance()->getToolBox();
for (int i=0; i<2; ++i) {
if (m_tool->getTracePolicy(i) != tools::TracePolicy::Accumulate &&
m_tool->getTracePolicy(i) != tools::TracePolicy::AccumulateUpdateLast) {
continue;
}

switch (algorithm) {
case kDefaultFreehandAlgorithm:
m_tool->setIntertwine(i, toolBox->getIntertwinerById(tools::WellKnownIntertwiners::AsLines));
m_tool->setTracePolicy(i, tools::TracePolicyAccumulate);
m_tool->setTracePolicy(i, tools::TracePolicy::Accumulate);
break;
case kPixelPerfectFreehandAlgorithm:
m_tool->setIntertwine(i, toolBox->getIntertwinerById(tools::WellKnownIntertwiners::AsPixelPerfect));
m_tool->setTracePolicy(i, tools::TracePolicyLast);
m_tool->setTracePolicy(i, tools::TracePolicy::AccumulateUpdateLast);
break;
case kDotsFreehandAlgorithm:
m_tool->setIntertwine(i, toolBox->getIntertwinerById(tools::WellKnownIntertwiners::None));
m_tool->setTracePolicy(i, tools::TracePolicyAccumulate);
m_tool->setTracePolicy(i, tools::TracePolicy::Accumulate);
break;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/app/thumbnail_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ class ThumbnailGenerator::Worker {

doc::ImageBufferPtr thumbnail_buffer(new doc::ImageBuffer);
base::UniquePtr<Image> image(renderEngine.renderSprite(
0, 0, sprite->width(), sprite->height(),
FrameNumber(0), Zoom(1, 1), true, false,
sprite->bounds(), FrameNumber(0),
Zoom(1, 1), true, false,
thumbnail_buffer));

// Calculate the thumbnail size
Expand Down
10 changes: 10 additions & 0 deletions src/app/tools/ink.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
#define APP_TOOLS_INK_H_INCLUDED
#pragma once

namespace gfx {
class Region;
}

namespace app {
namespace tools {

Expand Down Expand Up @@ -64,6 +68,12 @@ namespace app {
// Returns true if this ink is used to mark slices
virtual bool isSlice() const { return false; }

// Returns true if this ink needs a special source area. For
// example, blur tool needs one extra pixel to all sides of the
// modified area, so it can use a 3x3 convolution matrix.
virtual bool needsSpecialSourceArea() const { return false; }
virtual void createSpecialSourceArea(const gfx::Region& dirtyArea, gfx::Region& sourceArea) const { }

// It is called when the tool-loop start (generally when the user
// presses a mouse button over a sprite editor)
virtual void prepareInk(ToolLoop* loop) { }
Expand Down
2 changes: 1 addition & 1 deletion src/app/tools/ink_processing.h
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ class JumbleInkProcessing : public DoubleInkProcessing<JumbleInkProcessing<Image
Point m_speed;
int m_opacity;
TiledMode m_tiledMode;
Image* m_srcImage;
const Image* m_srcImage;
int m_srcImageWidth;
int m_srcImageHeight;
color_t m_color;
Expand Down
21 changes: 20 additions & 1 deletion src/app/tools/inks.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* Aseprite
* Copyright (C) 2001-2013 David Capello
* Copyright (C) 2001-2014 David Capello
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -27,6 +27,7 @@
#include "app/tools/pick_ink.h"
#include "app/undoers/set_mask.h"
#include "doc/mask.h"
#include "gfx/region.h"

namespace app {
namespace tools {
Expand Down Expand Up @@ -220,6 +221,7 @@ class BlurInk : public Ink {
public:
bool isPaint() const { return true; }
bool isEffect() const { return true; }
bool needsSpecialSourceArea() const { return true; }

void prepareInk(ToolLoop* loop)
{
Expand All @@ -230,6 +232,14 @@ class BlurInk : public Ink {
{
(*m_proc)(x1, y, x2, loop);
}

void createSpecialSourceArea(const gfx::Region& dirtyArea, gfx::Region& sourceArea) const {
// We need one pixel more for each side, to use a 3x3 convolution matrix.
for (const auto& rc : dirtyArea) {
sourceArea.createUnion(sourceArea,
gfx::Region(gfx::Rect(rc).enlarge(1)));
}
}
};


Expand All @@ -239,6 +249,7 @@ class JumbleInk : public Ink {
public:
bool isPaint() const { return true; }
bool isEffect() const { return true; }
bool needsSpecialSourceArea() const { return true; }

void prepareInk(ToolLoop* loop)
{
Expand All @@ -249,6 +260,14 @@ class JumbleInk : public Ink {
{
(*m_proc)(x1, y, x2, loop);
}

void createSpecialSourceArea(const gfx::Region& dirtyArea, gfx::Region& sourceArea) const {
// We need one pixel more for each side.
for (const auto& rc : dirtyArea) {
sourceArea.createUnion(sourceArea,
gfx::Region(gfx::Rect(rc).enlarge(1)));
}
}
};


Expand Down
4 changes: 4 additions & 0 deletions src/app/tools/intertwine.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ namespace app {
namespace tools {
class ToolLoop;

// Converts a sequence of points in several call to
// Intertwine::doPointshapePoint(). Basically each implementation
// says which pixels should be drawn between a sequence of
// user-defined points.
class Intertwine {
public:
typedef std::vector<gfx::Point> Points;
Expand Down
11 changes: 5 additions & 6 deletions src/app/tools/intertwiners.h
Original file line number Diff line number Diff line change
Expand Up @@ -254,12 +254,11 @@ class IntertwineAsPixelPerfect : public Intertwine {
PPData data(m_pts, loop);

for (size_t c=0; c+1<points.size(); ++c) {
int x1 = points[c].x;
int y1 = points[c].y;
int x2 = points[c+1].x;
int y2 = points[c+1].y;

algo_line(x1, y1, x2, y2,
algo_line(
points[c].x,
points[c].y,
points[c+1].x,
points[c+1].y,
(void*)&data,
(AlgoPixel)&IntertwineAsPixelPerfect::pixelPerfectLine);
}
Expand Down
3 changes: 2 additions & 1 deletion src/app/tools/point_shapes.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ class FloodFillPointShape : public PointShape {

void transformPoint(ToolLoop* loop, int x, int y)
{
doc::algorithm::floodfill(loop->getSrcImage(), x, y,
doc::algorithm::floodfill(
const_cast<Image*>(loop->getSrcImage()), x, y,
paintBounds(loop, x, y),
loop->getTolerance(),
loop->getContiguous(),
Expand Down
10 changes: 5 additions & 5 deletions src/app/tools/tool_box.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -275,14 +275,14 @@ void ToolBox::loadToolProperties(TiXmlElement* xmlTool, Tool* tool, int button,
throw base::Exception("Invalid intertwiner '%s' specified in '%s' tool.\n", intertwine, tool_id);

// Trace policy
TracePolicy tracepolicy_value = TracePolicyLast;
TracePolicy tracepolicy_value = TracePolicy::Last;
if (tracepolicy) {
if (strcmp(tracepolicy, "accumulative") == 0)
tracepolicy_value = TracePolicyAccumulate;
if (strcmp(tracepolicy, "accumulate") == 0)
tracepolicy_value = TracePolicy::Accumulate;
else if (strcmp(tracepolicy, "last") == 0)
tracepolicy_value = TracePolicyLast;
tracepolicy_value = TracePolicy::Last;
else if (strcmp(tracepolicy, "overlap") == 0)
tracepolicy_value = TracePolicyOverlap;
tracepolicy_value = TracePolicy::Overlap;
else
throw base::Exception("Invalid trace-policy '%s' specified in '%s' tool.\n", tracepolicy, tool_id);
}
Expand Down
25 changes: 24 additions & 1 deletion src/app/tools/tool_loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,34 @@ namespace app {
virtual FrameNumber getFrame() = 0;

// Should return an image where we can read pixels (readonly image)
virtual Image* getSrcImage() = 0;
virtual const Image* getSrcImage() = 0;

// Should return an image where we can write pixels
virtual Image* getDstImage() = 0;

// Makes the specified region valid in the source
// image. Basically the implementation should copy from the
// original cel the given region to the source image. The source
// image is used by inks to create blur effects or similar.
virtual void validateSrcImage(const gfx::Region& rgn) = 0;

// Makes the specified destination image region valid to be
// painted. The destination image is used by inks to compose the
// brush, so we've to make sure that the destination image
// matches the original cel when we make that composition.
virtual void validateDstImage(const gfx::Region& rgn) = 0;

// Invalidates the whole destination image. It's used for tools
// like line or rectangle which don't accumulate the effect so
// they need to start with a fresh destination image on each
// loop step/cycle.
virtual void invalidateDstImage() = 0;
virtual void invalidateDstImage(const gfx::Region& rgn) = 0;

// Copies the given region from the destination to the source
// image, used by "overlap" tools like jumble or spray.
virtual void copyValidDstToSrcImage(const gfx::Region& rgn) = 0;

// Returns the RGB map used to convert RGB values to palette index.
virtual RgbMap* getRgbMap() = 0;

Expand Down
Loading

0 comments on commit afbd3b2

Please sign in to comment.