QPainter: replace an inefficient QList with QVector

QPainterClipInfo is larger than a void*, so holding it in QLists
is needlessly inefficient. Worse, the code could come to depend
on the fragile property of (inefficient) QLists that references
to elements therein never are invalidated.

Fix by marking it movable and holding it in a QVector instead.

Port from op<< to append() to reap rvalue push_back().

Change-Id: I3af17eca2443d26a795381635510b8b9a5795e5f
Reviewed-by: Gunnar Sletta <gunnar@sletta.org>
This commit is contained in:
Marc Mutz 2015-06-24 02:04:09 +02:00
parent 612e082fe2
commit 2523f62939
4 changed files with 22 additions and 25 deletions

View File

@ -182,12 +182,11 @@ void QPaintEngineExPrivate::replayClipOperations()
if (!p || !p->d_ptr) if (!p || !p->d_ptr)
return; return;
QList<QPainterClipInfo> clipInfo = p->d_ptr->state->clipInfo; const QVector<QPainterClipInfo> &clipInfo = p->d_ptr->state->clipInfo;
QTransform transform = q->state()->matrix; QTransform transform = q->state()->matrix;
for (int i = 0; i < clipInfo.size(); ++i) { for (const QPainterClipInfo &info : clipInfo) {
const QPainterClipInfo &info = clipInfo.at(i);
if (info.matrix != q->state()->matrix) { if (info.matrix != q->state()->matrix) {
q->state()->matrix = info.matrix; q->state()->matrix = info.matrix;
@ -233,9 +232,7 @@ bool QPaintEngineExPrivate::hasClipOperations() const
if (!p || !p->d_ptr) if (!p || !p->d_ptr)
return false; return false;
QList<QPainterClipInfo> clipInfo = p->d_ptr->state->clipInfo; return !p->d_ptr->state->clipInfo.isEmpty();
return !clipInfo.isEmpty();
} }
/******************************************************************************* /*******************************************************************************

View File

@ -403,8 +403,7 @@ void QPainterPrivate::draw_helper(const QPainterPath &originalPath, DrawOperatio
if (q->hasClipping()) { if (q->hasClipping()) {
bool hasPerspectiveTransform = false; bool hasPerspectiveTransform = false;
for (int i = 0; i < state->clipInfo.size(); ++i) { for (const QPainterClipInfo &info : qAsConst(state->clipInfo)) {
const QPainterClipInfo &info = state->clipInfo.at(i);
if (info.matrix.type() == QTransform::TxProject) { if (info.matrix.type() == QTransform::TxProject) {
hasPerspectiveTransform = true; hasPerspectiveTransform = true;
break; break;
@ -1635,8 +1634,7 @@ void QPainter::restore()
tmp->clipPath = QPainterPath(); tmp->clipPath = QPainterPath();
d->engine->updateState(*tmp); d->engine->updateState(*tmp);
// replay the list of clip states, // replay the list of clip states,
for (int i=0; i<d->state->clipInfo.size(); ++i) { for (const QPainterClipInfo &info : qAsConst(d->state->clipInfo)) {
const QPainterClipInfo &info = d->state->clipInfo.at(i);
tmp->matrix = info.matrix; tmp->matrix = info.matrix;
tmp->matrix *= d->state->redirectionMatrix; tmp->matrix *= d->state->redirectionMatrix;
tmp->clipOperation = info.operation; tmp->clipOperation = info.operation;
@ -2512,8 +2510,7 @@ QRegion QPainter::clipRegion() const
const_cast<QPainter *>(this)->d_ptr->updateInvMatrix(); const_cast<QPainter *>(this)->d_ptr->updateInvMatrix();
// ### Falcon: Use QPainterPath // ### Falcon: Use QPainterPath
for (int i=0; i<d->state->clipInfo.size(); ++i) { for (const QPainterClipInfo &info : qAsConst(d->state->clipInfo)) {
const QPainterClipInfo &info = d->state->clipInfo.at(i);
switch (info.clipType) { switch (info.clipType) {
case QPainterClipInfo::RegionClip: { case QPainterClipInfo::RegionClip: {
@ -2627,7 +2624,7 @@ QPainterPath QPainter::clipPath() const
} }
// No clip, return empty // No clip, return empty
if (d->state->clipInfo.size() == 0) { if (d->state->clipInfo.isEmpty()) {
return QPainterPath(); return QPainterPath();
} else { } else {
@ -2679,9 +2676,9 @@ QRectF QPainter::clipBoundingRect() const
// precise, but it fits within the guarantee and it is reasonably // precise, but it fits within the guarantee and it is reasonably
// fast. // fast.
QRectF bounds; QRectF bounds;
for (int i=0; i<d->state->clipInfo.size(); ++i) { bool first = true;
for (const QPainterClipInfo &info : qAsConst(d->state->clipInfo)) {
QRectF r; QRectF r;
const QPainterClipInfo &info = d->state->clipInfo.at(i);
if (info.clipType == QPainterClipInfo::RectClip) if (info.clipType == QPainterClipInfo::RectClip)
r = info.rect; r = info.rect;
@ -2694,10 +2691,11 @@ QRectF QPainter::clipBoundingRect() const
r = info.matrix.mapRect(r); r = info.matrix.mapRect(r);
if (i == 0) if (first)
bounds = r; bounds = r;
else if (info.operation == Qt::IntersectClip) else if (info.operation == Qt::IntersectClip)
bounds &= r; bounds &= r;
first = false;
} }
@ -2745,7 +2743,7 @@ void QPainter::setClipRect(const QRectF &rect, Qt::ClipOperation op)
d->extended->clip(vp, op); d->extended->clip(vp, op);
if (op == Qt::ReplaceClip || op == Qt::NoClip) if (op == Qt::ReplaceClip || op == Qt::NoClip)
d->state->clipInfo.clear(); d->state->clipInfo.clear();
d->state->clipInfo << QPainterClipInfo(rect, op, d->state->matrix); d->state->clipInfo.append(QPainterClipInfo(rect, op, d->state->matrix));
d->state->clipOperation = op; d->state->clipOperation = op;
return; return;
} }
@ -2794,7 +2792,7 @@ void QPainter::setClipRect(const QRect &rect, Qt::ClipOperation op)
d->extended->clip(rect, op); d->extended->clip(rect, op);
if (op == Qt::ReplaceClip || op == Qt::NoClip) if (op == Qt::ReplaceClip || op == Qt::NoClip)
d->state->clipInfo.clear(); d->state->clipInfo.clear();
d->state->clipInfo << QPainterClipInfo(rect, op, d->state->matrix); d->state->clipInfo.append(QPainterClipInfo(rect, op, d->state->matrix));
d->state->clipOperation = op; d->state->clipOperation = op;
return; return;
} }
@ -2806,7 +2804,7 @@ void QPainter::setClipRect(const QRect &rect, Qt::ClipOperation op)
d->state->clipOperation = op; d->state->clipOperation = op;
if (op == Qt::NoClip || op == Qt::ReplaceClip) if (op == Qt::NoClip || op == Qt::ReplaceClip)
d->state->clipInfo.clear(); d->state->clipInfo.clear();
d->state->clipInfo << QPainterClipInfo(rect, op, d->state->matrix); d->state->clipInfo.append(QPainterClipInfo(rect, op, d->state->matrix));
d->state->clipEnabled = true; d->state->clipEnabled = true;
d->state->dirtyFlags |= QPaintEngine::DirtyClipRegion | QPaintEngine::DirtyClipEnabled; d->state->dirtyFlags |= QPaintEngine::DirtyClipRegion | QPaintEngine::DirtyClipEnabled;
d->updateState(d->state); d->updateState(d->state);
@ -2853,7 +2851,7 @@ void QPainter::setClipRegion(const QRegion &r, Qt::ClipOperation op)
d->extended->clip(r, op); d->extended->clip(r, op);
if (op == Qt::NoClip || op == Qt::ReplaceClip) if (op == Qt::NoClip || op == Qt::ReplaceClip)
d->state->clipInfo.clear(); d->state->clipInfo.clear();
d->state->clipInfo << QPainterClipInfo(r, op, d->state->matrix); d->state->clipInfo.append(QPainterClipInfo(r, op, d->state->matrix));
d->state->clipOperation = op; d->state->clipOperation = op;
return; return;
} }
@ -2865,7 +2863,7 @@ void QPainter::setClipRegion(const QRegion &r, Qt::ClipOperation op)
d->state->clipOperation = op; d->state->clipOperation = op;
if (op == Qt::NoClip || op == Qt::ReplaceClip) if (op == Qt::NoClip || op == Qt::ReplaceClip)
d->state->clipInfo.clear(); d->state->clipInfo.clear();
d->state->clipInfo << QPainterClipInfo(r, op, d->state->matrix); d->state->clipInfo.append(QPainterClipInfo(r, op, d->state->matrix));
d->state->clipEnabled = true; d->state->clipEnabled = true;
d->state->dirtyFlags |= QPaintEngine::DirtyClipRegion | QPaintEngine::DirtyClipEnabled; d->state->dirtyFlags |= QPaintEngine::DirtyClipRegion | QPaintEngine::DirtyClipEnabled;
d->updateState(d->state); d->updateState(d->state);
@ -3257,7 +3255,7 @@ void QPainter::setClipPath(const QPainterPath &path, Qt::ClipOperation op)
d->extended->clip(path, op); d->extended->clip(path, op);
if (op == Qt::NoClip || op == Qt::ReplaceClip) if (op == Qt::NoClip || op == Qt::ReplaceClip)
d->state->clipInfo.clear(); d->state->clipInfo.clear();
d->state->clipInfo << QPainterClipInfo(path, op, d->state->matrix); d->state->clipInfo.append(QPainterClipInfo(path, op, d->state->matrix));
d->state->clipOperation = op; d->state->clipOperation = op;
return; return;
} }
@ -3269,7 +3267,7 @@ void QPainter::setClipPath(const QPainterPath &path, Qt::ClipOperation op)
d->state->clipOperation = op; d->state->clipOperation = op;
if (op == Qt::NoClip || op == Qt::ReplaceClip) if (op == Qt::NoClip || op == Qt::ReplaceClip)
d->state->clipInfo.clear(); d->state->clipInfo.clear();
d->state->clipInfo << QPainterClipInfo(path, op, d->state->matrix); d->state->clipInfo.append(QPainterClipInfo(path, op, d->state->matrix));
d->state->clipEnabled = true; d->state->clipEnabled = true;
d->state->dirtyFlags |= QPaintEngine::DirtyClipPath | QPaintEngine::DirtyClipEnabled; d->state->dirtyFlags |= QPaintEngine::DirtyClipPath | QPaintEngine::DirtyClipEnabled;
d->updateState(d->state); d->updateState(d->state);

View File

@ -97,6 +97,7 @@ inline bool qbrush_has_transform(const QBrush &b) { return data_ptr(b)->transfor
class QPainterClipInfo class QPainterClipInfo
{ {
public: public:
QPainterClipInfo() {} // for QVector, don't use
enum ClipType { RegionClip, PathClip, RectClip, RectFClip }; enum ClipType { RegionClip, PathClip, RectClip, RectFClip };
QPainterClipInfo(const QPainterPath &p, Qt::ClipOperation op, const QTransform &m) : QPainterClipInfo(const QPainterPath &p, Qt::ClipOperation op, const QTransform &m) :
@ -134,6 +135,7 @@ public:
}; };
Q_DECLARE_TYPEINFO(QPainterClipInfo, Q_MOVABLE_TYPE);
class Q_GUI_EXPORT QPainterState : public QPaintEngineState class Q_GUI_EXPORT QPainterState : public QPaintEngineState
{ {
@ -153,7 +155,7 @@ public:
QPainterPath clipPath; QPainterPath clipPath;
Qt::ClipOperation clipOperation; Qt::ClipOperation clipOperation;
QPainter::RenderHints renderHints; QPainter::RenderHints renderHints;
QList<QPainterClipInfo> clipInfo; // ### Make me smaller and faster to copy around... QVector<QPainterClipInfo> clipInfo; // ### Make me smaller and faster to copy around...
QTransform worldMatrix; // World transformation matrix, not window and viewport QTransform worldMatrix; // World transformation matrix, not window and viewport
QTransform matrix; // Complete transformation matrix, QTransform matrix; // Complete transformation matrix,
QTransform redirectionMatrix; QTransform redirectionMatrix;

View File

@ -1687,7 +1687,7 @@ void QWindowsDirect2DPaintEngine::rasterFill(const QVectorPath &path, const QBru
p.setPen(state()->pen); p.setPen(state()->pen);
QPaintEngineEx *extended = static_cast<QPaintEngineEx *>(engine); QPaintEngineEx *extended = static_cast<QPaintEngineEx *>(engine);
foreach (const QPainterClipInfo &info, state()->clipInfo) { for (const QPainterClipInfo &info : qAsConst(state()->clipInfo)) {
extended->state()->matrix = info.matrix; extended->state()->matrix = info.matrix;
extended->transformChanged(); extended->transformChanged();