koffice/krita/ui/canvas

[Adrian Page]

On 1 Feb 2010, at 11:08AM, Dmitry Kazakov wrote:
> It gives the desired result - the smallest integer rectangle containing the QRectF. That's what we're after.
> 
> No, it doesn't. Because truncating and adding 0.5 is not stable over scaling. Just right over the thing we do in KisImage.
> 
> Example:
> 
> QPointF convertToQPointF(QPoint pt) {
>     return QPointF((pt.x()+0.5)*2, (pt.y()+0.5)*2);
> }
> 
> QRectF convertToQRectF(QRect rc) {
>     return QRectF(convertToQPointF(rc.topLeft()), convertToQPointF(rc.bottomRight()));
> }
> 
> void main()
> {
>     QRect rect(0,0,6,4);
>     qDebug()<<"intRect:"<<rect<<rect.topLeft()<<rect.bottomRight();
> 
>     QRectF floatRect(convertToQRectF(rect));
>     qDebug()<<"floatRect:"<<floatRect<<floatRect.topLeft()<<floatRect.bottomRight();
> 
>     QRect alignedRect(floatRect.toAlignedRect());
>     qDebug()<<"alignedRect:"<<alignedRect<<alignedRect.topLeft()<<alignedRect.bottomRight();
> }
> 
> The result will be the following:
> intRect: QRect(0,0 6x4) QPoint(0,0) QPoint(5,3)
> floatRect: QRectF(1,1 10x6) QPointF(1, 1)QPointF(11, 7)
> alignedRect: QRect(1,1 10x6) QPoint(1,1) QPoint(10,6)
> 
> 
> That is surely NOT what we desired. (btw, this example is almost a piece code from KisImage

Well, you've written your own convertToQRectF() and written it wrong ;-). I just checked krita and there is no QRect to QRectF conversion function, and the reason is none is required - it's simply QRectF(QRect). You're mixing up concepts in your function as converting the opposite corner points as points, then putting those into a QRectF gives a different kind of conversion. You're effectively scaling the QRectF(0.5, 0.5, 5, 3) by 2, and getting the result QRectF(1, 1, 10, 6) - which is right. And QRect(1, 1, 10, 6)  is the correct conversion back to QRect.

Converting rectangles is different to converting points. The QRectF(0.0, 0.0, 6.0, 4.0) is the correct conversion of the QRect(0, 0, 6, 4). It is the rectangle equal to the border of the 6x4 rectangle. And if you scale it by 2, you get QRectF(0, 0, 12, 8) - the border of the double sized QRect(0, 0, 6, 4).

> What do i want to say? 
> 1) If we decide to use Qt's style, we must not use these additional (+0.5) anywhere in the code. More than that, we can't even use bottomRight() function of QRect for scaling due to the "historical reasons" mentioned in Qt's documentation for QRect. We should scale width() and height() of the rect instead.
> 

By the way, QPainter uses the same system we do. To draw a line from the centre of the pixel (0, 0), you draw it from (0.5, 0.5). See the QRectF and coordinate system docs (anti-aliased painting).

> 2) If we decide to stay with the majority of the flow, we should remove all the traces of QRectF from Krita's code, for not confusing coders and eliminate their temptation to use toAlignedRect().

Well, you've helped convince me that QRectF(QRect) is correct, and toAlignedRect() is correct, so I don't see a huge problem. 

> 
>  
> No, you shouldn't need to round. If you are rounding, then you will be losing pixels when whatever number you're rounding is > x.0 and < x.5.
> 
> You won't loose anything if you use math for this =) You shouldn't add exactly 0.5. You should add (0.5-EPS), where EPS=1e-10.
> In such a case you'll get stable scale operation and no pixels will be lost.

Not knowing the context, I won't say anymore, and I don't intend to say much more on this subject. The system's not perfect, and if there are bugs, please report them and they'll get fixed. And If you want to improve/replace the system, great, feel free to post some patches and they will stand on their own merit.

Adrian








-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20100201/4ab9dead/attachment-0001.htm