Skip to content

Commit

Permalink
Remove the need to allocate an empty QPointF to draw pixmaps
Browse files Browse the repository at this point in the history
  • Loading branch information
MrStevns committed Sep 29, 2023
1 parent 03ba9fa commit f3501ca
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 10 deletions.
18 changes: 9 additions & 9 deletions core_lib/src/canvaspainter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void CanvasPainter::paintCached(const QRect& blitRect)
QPainter mainPainter;
initializePainter(mainPainter, mCanvas, blitRect);
mainPainter.setWorldMatrixEnabled(false);
mainPainter.drawPixmap(QPointF(), mPreLayersPixmap);
mainPainter.drawPixmap(pointZero, mPreLayersPixmap);
mainPainter.setWorldMatrixEnabled(true);

paintCurrentFrame(mainPainter, blitRect, mCurrentLayerIndex, mCurrentLayerIndex);
Expand All @@ -112,7 +112,7 @@ void CanvasPainter::paintCached(const QRect& blitRect)
}

mainPainter.setWorldMatrixEnabled(false);
mainPainter.drawPixmap(QPointF(), mPostLayersPixmap);
mainPainter.drawPixmap(pointZero, mPostLayersPixmap);
mainPainter.setWorldMatrixEnabled(true);
}

Expand Down Expand Up @@ -184,7 +184,7 @@ void CanvasPainter::paint(const QRect& blitRect)
preLayerPainter.end();

mainPainter.setWorldMatrixEnabled(false);
mainPainter.drawPixmap(QPointF(), mPreLayersPixmap);
mainPainter.drawPixmap(pointZero, mPreLayersPixmap);
mainPainter.setWorldMatrixEnabled(true);

paintCurrentFrame(mainPainter, blitRect, mCurrentLayerIndex, mCurrentLayerIndex);
Expand All @@ -194,7 +194,7 @@ void CanvasPainter::paint(const QRect& blitRect)
postLayerPainter.end();

mainPainter.setWorldMatrixEnabled(false);
mainPainter.drawPixmap(QPointF(), mPostLayersPixmap);
mainPainter.drawPixmap(pointZero, mPostLayersPixmap);
mainPainter.setWorldMatrixEnabled(true);

mPreLayersPixmapCacheValid = true;
Expand Down Expand Up @@ -280,7 +280,7 @@ void CanvasPainter::paintOnionSkinFrame(QPainter& painter, QPainter& onionSkinPa
onionSkinPainter.setBrush(colorBrush);
onionSkinPainter.drawRect(painter.viewport());
}
painter.drawPixmap(QPointF(), mOnionSkinPixmap);
painter.drawPixmap(pointZero, mOnionSkinPixmap);
}

void CanvasPainter::paintCurrentBitmapFrame(QPainter& painter, const QRect& blitRect, Layer* layer, bool isCurrentLayer)
Expand Down Expand Up @@ -313,7 +313,7 @@ void CanvasPainter::paintCurrentBitmapFrame(QPainter& painter, const QRect& blit

const auto tiles = mTiledBuffer->tiles();
for (const Tile* tile : tiles) {
currentBitmapPainter.drawPixmap(tile->pos(), tile->pixmap());
currentBitmapPainter.drawPixmap(tile->posF(), tile->pixmap());
}
} else {
// When we're drawing using a tool, the surface will be painted by the tiled buffer,
Expand All @@ -327,7 +327,7 @@ void CanvasPainter::paintCurrentBitmapFrame(QPainter& painter, const QRect& blit
paintTransformedSelection(currentBitmapPainter, paintedImage, mSelection);
}

painter.drawPixmap(QPointF(), mCurrentLayerPixmap);
painter.drawPixmap(pointZero, mCurrentLayerPixmap);
}

void CanvasPainter::paintCurrentVectorFrame(QPainter& painter, const QRect& blitRect, Layer* layer, bool isCurrentLayer)
Expand All @@ -353,7 +353,7 @@ void CanvasPainter::paintCurrentVectorFrame(QPainter& painter, const QRect& blit

const auto tiles = mTiledBuffer->tiles();
for (const Tile* tile : tiles) {
currentVectorPainter.drawPixmap(tile->pos(), tile->pixmap());
currentVectorPainter.drawPixmap(tile->posF(), tile->pixmap());
}
} else if (mRenderTransform) {
vectorImage->setSelectionTransformation(mSelectionTransform);
Expand All @@ -366,7 +366,7 @@ void CanvasPainter::paintCurrentVectorFrame(QPainter& painter, const QRect& blit

// Remember to adjust opacity based on additional opacity value from the keyframe
painter.setOpacity(vectorImage->getOpacity() - (1.0-painter.opacity()));
painter.drawPixmap(QPointF(), mCurrentLayerPixmap);
painter.drawPixmap(pointZero, mCurrentLayerPixmap);
}

void CanvasPainter::paintTransformedSelection(QPainter& painter, BitmapImage* bitmapImage, const QRect& selection) const
Expand Down
4 changes: 4 additions & 0 deletions core_lib/src/canvaspainter.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ class CanvasPainter
bool mPreLayersPixmapCacheValid = false;
bool mPostLayersPixmapCacheValid = false;

// There's a considerable amount of overhead in simply allocating a QPointF on the fly.
// Since we just need to draw it at 0,0, we might as well make a const value for that purpose
const QPointF pointZero;


OnionSkinSubPainter mOnionSkinSubPainter;
OnionSkinPainterOptions mOnionSkinPainterOptions;
Expand Down
1 change: 1 addition & 0 deletions core_lib/src/graphics/bitmap/tile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ GNU General Public License for more details.
Tile::Tile(const QPoint& pos, QSize size):
mTilePixmap(size),
mPos(pos),
mPosF(pos),
mBounds(pos, size),
mSize(size)
{
Expand Down
4 changes: 3 additions & 1 deletion core_lib/src/graphics/bitmap/tile.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class Tile
QPixmap& pixmap() { return mTilePixmap; }

const QPoint& pos() const { return mPos; }
const QPointF& posF() const { return mPosF; }
const QRect& bounds() const { return mBounds; }
const QSize& size() const { return mSize; }

Expand All @@ -39,7 +40,8 @@ class Tile
void clear();

private:
QPixmap mTilePixmap;
QPixmap mTilePixmap;
QPointF mPosF;
QPoint mPos;
QRect mBounds;
QSize mSize;
Expand Down

3 comments on commit f3501ca

@J5lx
Copy link

@J5lx J5lx commented on f3501ca Sep 29, 2023

Choose a reason for hiding this comment

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

How much overhead are we talking here? And is the pos → posF change for a similar reason?

@MrStevns
Copy link
Owner Author

@MrStevns MrStevns commented on f3501ca Sep 30, 2023

Choose a reason for hiding this comment

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

On the various paint operations in CanvasPainter, I had 34 ms of overhead to create QPointF objects across a 2 second drawing session. Aside from just painting the pixmaps, this was amongst the slowest operations in my time profiler.

It might seem silly but considering that we'll need it to be as fast as possible to draw a smooth stroke, the optimization seemed reasonable. If you disagree, then i'll remove it again.

Yeah the pos -> posF is the same, although the overhead was smaller, just 11 ms + it would convert it to a QPointF internally for painting every time.

@J5lx
Copy link

@J5lx J5lx commented on f3501ca Sep 30, 2023

Choose a reason for hiding this comment

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

Okay, fair enough. Thanks for clarifying!

Please sign in to comment.