1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QPicture: (almost) work around overflow in bounding box calculations

Browse Source 
QPicture keeps a bounding box of its drawing, and updates it as it
processes paint commands, extending it if necessary. The problem is that
these calculations are prone to overflows:

* drawing (and thus the bounding box) is floating point based, but
  QPicture only stores a QRect. If the drawing's BB overflows the
  integer range, QPicture overflows.
* Since the stored QRect is `int` based (signed), if the drawing spans
  "too far" (e.g.  from INT_MIN/2 to INT_MAX/2), it'll again overflow
  the range. This impacts serialization, as the protocol serializes the
  bounding box's topleft+size.

For the former, clamp the qreal-based into the integer range before
the conversion.

For the latter, as a dirty work around, when calculating a new bounding
box, directly set the bounding box corners instead of topleft+size. This
is NOT A FIX by any means. I'm trading one source of UB for another; the
only objective is hiding an assertion from a test.

Some design change is needed to properly fix this (keeping in mind the
stability of the serialized format).

Task-number: QTBUG-133293
Change-Id: Ia741e1e73bdf7eee21f66b4ea5c1fae3430b468d
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
This commit is contained in:
Giuseppe D'Angelo 2025-01-28 14:33:48 +01:00
parent 7736823500
commit a66f51fe82
1 changed files with 12 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/gui/image/qpaintengine_pic.cpp
View File

@ -317,19 +317,25 @@ void QPicturePaintEngine::writeCmdLength(int pos, const QRectF &r, bool corr)
}
if (br.width() > 0.0 || br.height() > 0.0) {
int minx = qFloor(br.left());
int miny = qFloor(br.top());
int maxx = qCeil(br.right());
int maxy = qCeil(br.bottom());
const auto clampToIntRange = [](qreal v)
{
return qBound(qreal((std::numeric_limits<int>::min)()),
v,
qreal((std::numeric_limits<int>::max)()));
};
int minx = qFloor(clampToIntRange(br.left()));
int miny = qFloor(clampToIntRange(br.top()));
int maxx = qCeil(clampToIntRange(br.right()));
int maxy = qCeil(clampToIntRange(br.bottom()));
if (d->pic_d->brect.width() > 0 || d->pic_d->brect.height() > 0) {
minx = qMin(minx, d->pic_d->brect.left());
miny = qMin(miny, d->pic_d->brect.top());
maxx = qMax(maxx, d->pic_d->brect.x() + d->pic_d->brect.width());
maxy = qMax(maxy, d->pic_d->brect.y() + d->pic_d->brect.height());
d->pic_d->brect = QRect(minx, miny, maxx - minx, maxy - miny);
d->pic_d->brect.setCoords(minx, miny, maxx - 1, maxy - 1);
} else {
d->pic_d->brect = QRect(minx, miny, maxx - minx, maxy - miny);
d->pic_d->brect.setCoords(minx, miny, maxx - 1, maxy - 1);
}
}
}