GraphicsView performance problems

Hi,
Actually this is continuation of this thread but the question is quite different.
I am running into performance problems when multiple items are moved. The peformance is better when I disable the children of resistor(nodes and text item). But I want those features in my app. Also the grid look corrupted when items are moved. Any suggestion for that ?
Here is the profile output

Quote:

---

Flat profile:

Each sample counts as 0.01 seconds.
% cumulative self self total
time seconds seconds calls Ts/call Ts/call name
75.08 0.03 0.03 GridScene::drawBackground(QPainter*, QRectF const&)
25.03 0.04 0.01 Resistor::boundingRect() const
0.00 0.04 0.00 119168 0.00 0.00 QVarLengthArray<QLineF, 100>::realloc(int, int)

---

Here is the shorter version of the code. Select all items and move so that you can see what I mean. Please help me. :)

Code:

---

#include <QtGui>
 
class GridScene : public QGraphicsScene
{
   public:
      GridScene(qreal x, qreal y, qreal w, qreal h)
         : QGraphicsScene(x, y, w, h)
      { }
 
   protected:
      void drawBackground(QPainter *painter, const QRectF &rect)
      {
         const int gridSize = 25;
         painter->setPen(QPen(Qt::darkGreen,0));
         qreal left = int(rect.left()) - (int(rect.left()) % gridSize);
         qreal top = int(rect.top()) - (int(rect.top()) % gridSize);
 
         QVarLengthArray<QLineF, 100> lines;
 
         for (qreal x = left; x < rect.right(); x += gridSize)
            lines.append(QLineF(x, rect.top(), x, rect.bottom()));
         for (qreal y = top; y < rect.bottom(); y += gridSize)
            lines.append(QLineF(rect.left(), y, rect.right(), y));
 
         painter->drawLines(lines.data(), lines.size());
      }
};
 
class Node : public QGraphicsItem
{
   public:
      Node(QPointF pos ,QGraphicsItem *par = 0, QGraphicsScene *sc = 0) : QGraphicsItem(par,sc)
      {
         setPos(pos);
         setFlags(0);
         setAcceptedMouseButtons(0);
      }
 
      void paint(QPainter* p,const QStyleOptionGraphicsItem *, QWidget *)
      {
         p->setPen(Qt::darkRed);
         p->drawEllipse(QRectF(-3, -3, 2*3, 2*3));
      }
 
      QPainterPath shape() const
      {
         QPainterPath path;
         path.addEllipse(QRectF(-3, -3, 2*3, 2*3));
         return path;
      }
 
      QRectF boundingRect() const
      {
         return QRectF(-3, -3, 2*3, 2*3);
      }
 
};
 
class Resistor : public QGraphicsItem
{
   public:
      Resistor(QGraphicsItem *par = 0, QGraphicsScene *scene = 0) : QGraphicsItem(par,scene)
      {
         setFlags(ItemIsMovable | ItemIsSelectable | ItemIsFocusable);
         // comment the following  4 lines to see the performance difference
         QGraphicsTextItem *t = new QGraphicsTextItem("R1 = 100k",this,scene);
         t->setPos(0,-35);
         new Node(QPointF(-30,0),this,scene);
         new Node(QPointF(30,0),this,scene);
      }
 
      void paint(QPainter *p, const QStyleOptionGraphicsItem *o, QWidget *)
      {
         if(!(o->state & QStyle::State_Open))
            p->setPen(Qt::darkBlue);
         if(o->state & QStyle::State_Selected)
            p->setPen(QPen(Qt::darkGray,1));
 
         p->drawLine(-18, -9, 18, -9);
         p->drawLine( 18, -9, 18,  9);
         p->drawLine( 18,  9,-18,  9);
         p->drawLine(-18,  9,-18, -9);
         p->drawLine(-27,  0,-18,  0);
         p->drawLine( 18,  0, 27,  0);
      }
 
      QRectF boundingRect() const
      {
         qreal pw = 0.5;
         return QRectF(-27,-9,54,18).adjusted(-pw,-pw,pw,pw);
      }
};
 
int main(int argc,char *argv[])
{
   QApplication app(argc,argv);
   GridScene scene(0,0,1024,768);
   for(int j = 2; j < 4; ++j)
      for(int i = 1; i <11; ++i)
      {
         Resistor *r = new Resistor(0,&scene);
         r->setPos(j*100, i * 50);
      }
   QGraphicsView *view = new QGraphicsView(&scene);
   view->setRenderHints(QPainter::Antialiasing | QPainter::SmoothPixmapTransform);
   view->setDragMode(QGraphicsView::RubberBandDrag);
   view->show();
   return app.exec();
}

---

In your main function just add

view->setViewport(new QGLWidget);

I am sure, this will improve the performence.
please add QGLWidget include file & path in settings.

Quote:

---

Originally Posted by rajesh

In your main function just add

view->setViewport(new QGLWidget);

I am sure, this will improve the performence.
please add QGLWidget include file & path in settings.

---

Wish I could have done that but I can't use opengl since the app is targeted to platforms even without opengl support.
But I tested it now and found some improvement but sadly I can't use it.

What if you disable the render hints while painting the background? Do you get a significant performance gain? Could you also please include information about profiling for drawBackground()? It might require a recompilation of Qt!

Quote:

---

Originally Posted by wysota

What if you disable the render hints while painting the background? Do you get a significant performance gain?

---

Disabling render hints didn't improve the performance significantly.

Quote:

---

Originally Posted by wysota

Could you also please include information about profiling for drawBackground()?!It might require a recompilation of Qt

---

I can do that but i may have to do re-recompilation later :(
Before that, do you see any problems in my code ? Did you see performance difference after commenting the lines ? Atleast I would like to wait till Bitto answers something since compiling qt is time consuming!

As I already pointed out that performance is better without those nodes and textitems as children. Does that mean drawBackground() is being called twice for same area ? Does the framework lack any optimization in calculating this before hand ?

drawBackground() get called every time whenever your window painted, means when ever you move window or resize..

Quote:

---

Originally Posted by rajesh

drawBackground() get called every time whenever your window painted, means when ever you move window or resize..

---

No. That is just one of the case. drawBackground() is called always before updating the required item(eg when its moved) with the probably the item's bounding rect. You can check that by just printing something say qDebug() << rect; in the method.

Quote:

---

Originally Posted by Gopala Krishna

Also the grid look corrupted when items are moved. Any suggestion for that ?

---

In my experience floating point arithmetic is always a nice way to get unexpected results...

Have you tried using QLine instead of QLineF
(It might give you a little speedup too, since often integer calculations are faster than floating point calculations.)


Also, do you know which rounding functions the int cast uses?
Does it "round to nearest"? Does it do a "floor"? Does it do a "ceil"?
It might make a difference, so you might want to be explicit there.


Good luck :-)

Quote:

---

Originally Posted by camel

In my experience loating point arithmetic is always a nice way to get unexpected results (in my experience)...

Have you tried using QLine instead of QLineF
(It might give you a little speedup too, since often integer calculations are faster than floating point calculations.)


Also, do you know which rounding functions the int cast uses?
Does it "round to nearest"? Does it do a "floor"? Does it do a "ceil"?
It might make a difference, so you might want to be explicit there.


Good luck :-)

---

I think you are right. I will test with QLine since I don't need floating points for the grid. Thanks for pointing this. :)

I tested with QLine but that didn't make a difference. Also i am not sure what the int cast does - i feel it just takes int part without rounding. But do you think this will make a difference since I am following one convention in all places in drawBackground() ?

Quote:

---

Originally Posted by Gopala Krishna

I tested with QLine but that didn't make a difference. Also i am not sure what the int cast does - i feel it just takes int part without rounding. But do you think this will make a difference since I am following one convention in all places in drawBackground() ?

---

It does make a difference:

Code:

---

void drawBackground(QPainter *painter, const QRectF &rect)
      {
         const int gridSize = 25;
 
         const int realLeft = static_cast<int>(std::floor(rect.left()));
         const int realRight = static_cast<int>(std::ceil(rect.right()));
         const int realTop = static_cast<int>(std::floor(rect.top()));
         const int realBottom = static_cast<int>(std::ceil(rect.bottom()));
 
 
         const int firstLeftGridLine = realLeft - (realLeft % gridSize);
         const int firstTopGridLine = realTop - (realTop % gridSize);
 
         QVarLengthArray<QLine, 100> lines;
 
         for (int x = firstLeftGridLine; x <= realRight; x += gridSize)
            lines.append(QLine(x, realTop, x, realBottom));
         for (int y = firstTopGridLine; y <= realBottom; y += gridSize)
            lines.append(QLine(realLeft, y, realRight, y));
 
 
         painter->setPen(QPen(Qt::darkGreen,0));
         painter->drawLines(lines.data(), lines.size());
      }

---

As you see I make three changes
a) integer instead of float
b) using the containing integer rect as basis (i.e. the smallest integer rect that contains the float rect)
c) I changed the "<" in your for loops to a "<=", i thing part of your corruption was that it did not always repaint the grid when the line was at the border of the exposed rect.


The result: no more corruption of the grid...only weird color changes that I am trying to track down ;-)


EDIT: Of course you now need:

Code:

---

#include <cmath>

---

for the floor, and ceil to work ;-)

Quote:

---

Originally Posted by camel

...
The result: no more corruption of the grid...only weird color changes that I am trying to track down ;-)
for the floor, and ceil to work ;-)

---

As you said no more corruption but yeah even I am cracking my head a lot on how to remove those weird color changes.
But I tell you I found one magic while experimenting. Try , its fun.

change the bounding rect of Resistor to

Code:

---

QRectF Resistor::boundingRect() const
{
     qreal pw = 0.5;
    return QRectF(-47,-9,47*2,18).adjusted(-pw,-pw,pw,pw);
}

---

Check for performance now!! :p

I really suggest to recompile Qt with profiling information. You're shooting blind now. Profiling will give you the exact bottleneck point. Furthermore make sure your app is compiled correctly, I think you should be able to have a decomposition of the drawBackground() method in the profiling information as well. It should then tell you which part of the method takes most time and it should be possible to obtain without recompiling Qt.

Hey,

I just changed your source a bit so that the boundingrects and shapes are only calculated once...

check if that helps with the performance a bit...

(since I cannot realy judge it here... :-)



I also found that the grid-color-problem is lessend (but not gone) when you fill the rect in the draw background,

And the error is gone completely when you turn off anti-aliasing.

Might be a Qt-Bug :-/


Anyways, my version of your code :-)

Code:

---

#include <QtGui>
#include <cmath>
 
class GridScene : public QGraphicsScene
{
   public:
      GridScene(qreal x, qreal y, qreal w, qreal h)
         : QGraphicsScene(x, y, w, h)
      { }
 
   protected:
      void drawBackground(QPainter *painter, const QRectF &rect)
      {
         const int gridSize = 25;
 
         const int realLeft = static_cast<int>(std::floor(rect.left()));
         const int realRight = static_cast<int>(std::ceil(rect.right()));
         const int realTop = static_cast<int>(std::floor(rect.top()));
         const int realBottom = static_cast<int>(std::ceil(rect.bottom()));
 
 
         const int firstLeftGridLine = realLeft - (realLeft % gridSize);
         const int firstTopGridLine = realTop - (realTop % gridSize);
 
         QVarLengthArray<QLine, 100> lines;
 
         for (int x = firstLeftGridLine; x <= realRight; x += gridSize)
            lines.append(QLine(x, realTop, x, realBottom));
         for (int y = firstTopGridLine; y <= realBottom; y += gridSize)
            lines.append(QLine(realLeft, y, realRight, y));
 
//If you uncomment this, you will find the color problem lessened...
//why? well your guess is as good as mine ;-)
//         painter->fillRect(QRect(QPoint(realLeft, realTop), QPoint(realRight, realBottom)), QBrush(Qt::white));
 
         painter->setOpacity(1.0);
         painter->setPen(QPen(Qt::darkGreen,0));
         painter->drawLines(lines.data(), lines.size());
      }
};
 
namespace {
    static inline QPainterPath constructNodeShape(const QRect& elipseRect)
    {
        QPainterPath path;
        path.addEllipse(elipseRect);
        return path;
    }
}
 
class Node : public QGraphicsItem
{
   public:
      Node(QPointF pos ,QGraphicsItem *par = 0, QGraphicsScene *sc = 0)
          : QGraphicsItem(par,sc),
            elipseRect(-3, -3, 2*3, 2*3),
            elipseShape(constructNodeShape(elipseRect))
      {
         setPos(pos);
         setFlags(0);
         setAcceptedMouseButtons(0);
      }
 
      void paint(QPainter* p,const QStyleOptionGraphicsItem *, QWidget *)
      {
         p->setPen(Qt::darkRed);
         p->setBrush(Qt::NoBrush);
         p->setOpacity(1.0);
         p->drawEllipse(elipseRect);
      }
 
      QPainterPath shape() const
      {
         return elipseShape;
      }
 
      QRectF boundingRect() const
      {
         return elipseRect;
      }
 
    protected:
      const QRect elipseRect;
      const QPainterPath elipseShape;
};
 
class Resistor : public QGraphicsItem
{
   public:
      Resistor(QGraphicsItem *par = 0, QGraphicsScene *scene = 0)
          : QGraphicsItem(par,scene),
            resistorBox(-18, -9, 36, 18),
            connectors(new QLine[2])
      {
         setFlags(ItemIsMovable | ItemIsSelectable | ItemIsFocusable);
         // comment the following  4 lines to see the performance difference
         QGraphicsTextItem *t = new QGraphicsTextItem("R1 = 100k",this,scene);
         t->setPos(0,-35);
         new Node(QPointF(-30,0),this,scene);
         new Node(QPointF(30,0),this,scene);
 
        connectors[0] = QLine(-27, 0, -18, 0);
        connectors[1] = QLine(18, 0, 27, 0);
 
        boundingBox |= resistorBox;
        boundingBox |= QRectF(connectors[0].x1(), connectors[0].y1(), connectors[0].x2(), connectors[0].y2() );
        boundingBox |= QRectF(connectors[1].x1(), connectors[1].y1(), connectors[1].x2(), connectors[1].y2() );
        boundingBox.adjust(-0.5, -0.5, 0.5, 0.5);
      }
 
      ~Resistor()
      {
          delete[] connectors;
      }
 
      void paint(QPainter *p, const QStyleOptionGraphicsItem *o, QWidget *)
      {
         if(!(o->state & QStyle::State_Open))
            p->setPen(Qt::darkBlue);
         if(o->state & QStyle::State_Selected)
            p->setPen(QPen(Qt::darkGray,1));
 
         p->setBrush(Qt::NoBrush);
         p->drawRect(resistorBox);
         p->drawLines(connectors, 2);
      }
 
      QRectF boundingRect() const
      {
        return boundingBox;
      }
 
   private:
      QRect resistorBox;
      QLine* connectors;
      QRectF boundingBox;
};
 
int main(int argc,char *argv[])
{
   QApplication app(argc,argv);
   GridScene scene(0,0,1024,768);
   for(int j = 2; j < 4; ++j)
      for(int i = 1; i <11; ++i)
      {
         Resistor *r = new Resistor(0,&scene);
         r->setPos(j*100, i * 50);
      }
   QGraphicsView *view = new QGraphicsView(&scene);
// Turn of antialiasing and the color problem is gone...
   view->setRenderHints(QPainter::Antialiasing | QPainter::SmoothPixmapTransform);
   view->setDragMode(QGraphicsView::RubberBandDrag);
   view->show();
   return app.exec();
}

---

Quote:

---

Originally Posted by wysota

I really suggest to recompile Qt with profiling information. You're shooting blind now. Profiling will give you the exact bottleneck point. Furthermore make sure your app is compiled correctly, I think you should be able to have a decomposition of the drawBackground() method in the profiling information as well. It should then tell you which part of the method takes most time and it should be possible to obtain without recompiling Qt.

---

ok finally I agree. You are right . I guess there is no decomposition because most method are inlined. Well I'll do that recompilation tomorrow(nice job in the weekend ;) )

First do this (or something simmilar):

Code:

---

void drawLines(QPainter *painter, QVarLengthArray<QLineF, 100> &lines){
         painter->drawLines(lines.data(), lines.size());
}
 
void setupArray(QVarLengthArray<QLineF, 100> &lines){
         const int gridSize = 25;
         qreal left = int(rect.left()) - (int(rect.left()) % gridSize);
         qreal top = int(rect.top()) - (int(rect.top()) % gridSize);
         for (qreal x = left; x < rect.right(); x += gridSize)
            lines.append(QLineF(x, rect.top(), x, rect.bottom()));
         for (qreal y = top; y < rect.bottom(); y += gridSize)
            lines.append(QLineF(rect.left(), y, rect.right(), y));
}
void drawBackground(QPainter *painter, const QRectF &rect){
         painter->setPen(QPen(Qt::darkGreen,0));         
         QVarLengthArray<QLineF, 100> lines;
         setupArray(lines);
         drawLines(painter, lines);
}

---

This should allow you to measure how much time does it take to setup the array and how much to paint it.

Quote:

---

Originally Posted by camel

Hey,

I just changed your source a bit so that the boundingrects and shapes are only calculated once...

check if that helps with the performance a bit...

(since I cannot realy judge it here... :-)



I also found that the grid-color-problem is lessend (but not gone) when you fill the rect in the draw background,

And the error is gone completely when you turn off anti-aliasing.

Might be a Qt-Bug :-/


Anyways, my version of your code :-)

Code:

---

#include <QtGui>
#include <cmath>
 
class GridScene : public QGraphicsScene
{
   public:
   ....

---

---

I am really thankful to your support. :)

Yeah you are right. The colouring is gone without antialiasing. But I also experimented with one more thing.
I did this in drawBackground() which didn't alter much

Code:

---

void GridScene::drawBackground(QPainter *p, const QRectF& rect) const
{
        QPainter::RenderHints rh = p->renderHints();
        p->setRenderHints(0);
      ...
       p->setRenderHints(rh);
}

---

Should be Qt bug I believe. :confused:

Calculating bounding rect beforehand did show improvement in performance. But I feel the main performance blow is in drawBackground(). Somehow I feel that drawBackground() is called more than required times which is causing all these. For example try with this

Code:

---

view->setCacheMode(QGraphicsView::CacheBackground);

---

The performance improves but the background corrupts when we scroll. Also as I pointed in my last but one post, increasing bounding rect of resistor so that it covers node also sigificantly improved performance. This suggests that the rects are not ORred properly before updating that region. ( I may be wrong, correct me if I am)
Well to exactly solve this we need to profile the code (with profiling enabled for qt lib) or use some intelligent techniques (which I am not aware of )

Well, I already did profile it globally and found something rather interesting.

There is no use, trying to draw anything via the qpainter->draw(Rect, Elipsis, Polygons) whatever in Graphics view, since it is going to be converted to a painterpath anyways.


That means, that for every paint, you create a new painterpath, which means that you need to allocate memory, which is slow.

Why a painterpath?
If you check the x11 paintengine, you see that as soon as antialiasing is enabled (or there is a certain amount of TxTranslate, whatever that means), drawing is done using the "path_fallback", which means primitives are first converted to paths before being drawn.
Why? No clue ;-)

But as a result that means, if you draw a lot of (rects, ellipses and polygons, lines...), use a painterpath instead. Preferably even one that you do not recalculate every time you are asked to draw something.. :-)


The source after this transition:

Code:

---

#include <QtGui>
#include <cmath>
 
class GridScene : public QGraphicsScene
{
   public:
      GridScene(qreal x, qreal y, qreal w, qreal h)
         : QGraphicsScene(x, y, w, h)
      { }
 
   protected:
      void drawBackground(QPainter *painter, const QRectF &rect)
      {
         const int gridSize = 25;
 
         const int realLeft = static_cast<int>(std::floor(rect.left()));
         const int realRight = static_cast<int>(std::ceil(rect.right()));
         const int realTop = static_cast<int>(std::floor(rect.top()));
         const int realBottom = static_cast<int>(std::ceil(rect.bottom()));
 
 
         const int firstLeftGridLine = realLeft - (realLeft % gridSize);
         const int firstTopGridLine = realTop - (realTop % gridSize);
 
         QPainterPath background;
 
         for (int x = firstLeftGridLine; x <= realRight; x += gridSize) {
             background.moveTo(x, realTop);
             background.lineTo(x, realBottom);
             background.closeSubpath();
         }
         for (int y = firstTopGridLine; y <= realBottom; y += gridSize) {
             background.moveTo(realLeft, y);
             background.lineTo(realRight, y);
             background.closeSubpath();
         }
 
//If you uncomment this, you will find the color problem lessened...
//why? well your guess is as good as mine ;-)
//         painter->fillRect(QRect(QPoint(realLeft, realTop), QPoint(realRight, realBottom)), QBrush(Qt::white));
 
         painter->setOpacity(1.0);
         painter->setPen(QPen(Qt::darkGreen,0));
         painter->drawPath(background);
      }
};
 
namespace {
    static inline QPainterPath constructNodeShape(const QRect& elipseRect)
    {
        QPainterPath path;
        path.addEllipse(elipseRect);
        return path;
    }
}
 
class Node : public QGraphicsItem
{
   public:
      Node(QPointF pos ,QGraphicsItem *par = 0, QGraphicsScene *sc = 0)
          : QGraphicsItem(par,sc),
            elipseRect(-3, -3, 2*3, 2*3),
            elipseShape(constructNodeShape(elipseRect))
      {
         setPos(pos);
         setFlags(0);
         setAcceptedMouseButtons(0);
      }
 
      void paint(QPainter* p,const QStyleOptionGraphicsItem *, QWidget *)
      {
         p->setPen(Qt::darkRed);
         p->setBrush(Qt::NoBrush);
         p->setOpacity(1.0);
 
         p->drawPath(elipseShape);
      }
 
      QPainterPath shape() const
      {
         return elipseShape;
      }
 
      QRectF boundingRect() const
      {
         return elipseRect;
      }
 
    protected:
      const QRect elipseRect;
      const QPainterPath elipseShape;
};
 
class Resistor : public QGraphicsItem
{
   public:
      Resistor(QGraphicsItem *par = 0, QGraphicsScene *scene = 0)
          : QGraphicsItem(par,scene)
      {
         setFlags(ItemIsMovable | ItemIsSelectable | ItemIsFocusable);
         // comment the following  4 lines to see the performance difference
         QGraphicsTextItem *t = new QGraphicsTextItem("R1 = 100k",this,scene);
         t->setPos(0,-35);
         new Node(QPointF(-30,0),this,scene);
         new Node(QPointF(30,0),this,scene);
 
 
        resistorPath.addRect(QRectF(-18.0, -9.0, 36.0, 18.0));
        resistorPath.moveTo(-27, 0);
        resistorPath.lineTo(-18, 0);
        resistorPath.closeSubpath();
        resistorPath.moveTo(18, 0);
        resistorPath.lineTo(27, 0);
        resistorPath.closeSubpath();
 
        boundingBox = resistorPath.boundingRect();
      }
 
      void paint(QPainter *p, const QStyleOptionGraphicsItem *o, QWidget *)
      {
         if(!(o->state & QStyle::State_Open))
            p->setPen(Qt::darkBlue);
         if(o->state & QStyle::State_Selected)
            p->setPen(QPen(Qt::darkGray,1));
 
         p->setBrush(Qt::NoBrush);
         p->drawPath(resistorPath);
      }
 
      QRectF boundingRect() const
      {
        return boundingBox;
      }
 
   private:
      QPainterPath resistorPath;
      QRectF boundingBox;
};
 
int main(int argc,char *argv[])
{
   QApplication app(argc,argv);
   GridScene scene(0,0,1024,768);
   for(int j = 2; j < 4; ++j)
      for(int i = 1; i <11; ++i)
      {
         Resistor *r = new Resistor(0,&scene);
         r->setPos(j*100, i * 50);
      }
   QGraphicsView *view = new QGraphicsView(&scene);
// Turn of antialiasing and the color problem is gone...
   view->setRenderHints(QPainter::Antialiasing | QPainter::SmoothPixmapTransform);
   view->setDragMode(QGraphicsView::RubberBandDrag);
   view->show();
   return app.exec();
}

---

Quote:

---

Originally Posted by wysota

First do this (or something simmilar):

Code:

---

void drawLines(QPainter *painter, QVarLengthArray<QLineF, 100> &lines){
         painter->drawLines(lines.data(), lines.size());
}
 
void setupArray(QVarLengthArray<QLineF, 100> &lines){
         const int gridSize = 25;
         qreal left = int(rect.left()) - (int(rect.left()) % gridSize);
         qreal top = int(rect.top()) - (int(rect.top()) % gridSize);
         for (qreal x = left; x < rect.right(); x += gridSize)
            lines.append(QLineF(x, rect.top(), x, rect.bottom()));
         for (qreal y = top; y < rect.bottom(); y += gridSize)
            lines.append(QLineF(rect.left(), y, rect.right(), y));
}
void drawBackground(QPainter *painter, const QRectF &rect){
         painter->setPen(QPen(Qt::darkGreen,0));         
         QVarLengthArray<QLineF, 100> lines;
         setupArray(lines);
         drawLines(painter, lines);
}

---

This should allow you to measure how much time does it take to setup the array and how much to paint it.

---

Ok I did that, the profile now says

Quote:

---

Flat profile:

Each sample counts as 0.01 seconds.
% cumulative self self total
time seconds seconds calls Ts/call Ts/call name
50.05 0.02 0.02 Node:: paint(QPainter*, QStyleOptionGraphicsItem const*, QWidget*)
25.03 0.03 0.01 GridScene::drawBackground(QPainter*, QRectF const&)
25.03 0.04 0.01 Resistor::boundingRect() const
0.00 0.04 0.00 192004 0.00 0.00 QVarLengthArray<QLineF, 100>::realloc(int, int)
0.00 0.04 0.00 36506 0.00 0.00 GridScene::setupArray(QVarLengthArray<QLineF, 100>&, QRectF const&)
0.00 0.04 0.00 36506 0.00 0.00 GridScene::drawLines(QPainter*, QVarLengthArray<QLineF, 100>&)
0.00 0.04 0.00 1 0.00 0.00 global constructors keyed to _ZN9GridScene9drawLinesEP8QPainterR15QVarLengthArr ayI6QLineFLi100EE
0.00 0.04 0.00 1 0.00 0.00 __static_initialization_and_destruction_0(int, int)

---

Further

Quote:

---

Call graph (explanation follows)


granularity: each sample hit covers 2 byte(s) for 24.97% of 0.04 seconds

index % time self children called name
<spontaneous>
[1] 50.0 0.02 0.00 Node:: paint(QPainter*, QStyleOptionGraphicsItem const*, QWidget*) [1]
-----------------------------------------------
<spontaneous>
[2] 25.0 0.01 0.00 GridScene::drawBackground(QPainter*, QRectF const&) [2]
0.00 0.00 36506/36506 GridScene::setupArray(QVarLengthArray<QLineF, 100>&, QRectF const&) [12]
0.00 0.00 36506/36506 GridScene::drawLines(QPainter*, QVarLengthArray<QLineF, 100>&) [13]
-----------------------------------------------
<spontaneous>
[3] 25.0 0.01 0.00 Resistor::boundingRect() const [3]
-----------------------------------------------
0.00 0.00 192004/192004 GridScene::setupArray(QVarLengthArray<QLineF, 100>&, QRectF const&) [12]
[11] 0.0 0.00 0.00 192004 QVarLengthArray<QLineF, 100>::realloc(int, int) [11]
-----------------------------------------------
0.00 0.00 36506/36506 GridScene::drawBackground(QPainter*, QRectF const&) [2]
[12] 0.0 0.00 0.00 36506 GridScene::setupArray(QVarLengthArray<QLineF, 100>&, QRectF const&) [12]
0.00 0.00 192004/192004 QVarLengthArray<QLineF, 100>::realloc(int, int) [11]
-----------------------------------------------
0.00 0.00 36506/36506 GridScene::drawBackground(QPainter*, QRectF const&) [2]
[13] 0.0 0.00 0.00 36506 GridScene::drawLines(QPainter*, QVarLengthArray<QLineF, 100>&) [13]
-----------------------------------------------
0.00 0.00 1/1 __do_global_ctors_aux [25]
[14] 0.0 0.00 0.00 1 global constructors keyed to _ZN9GridScene9drawLinesEP8QPainterR15QVarLengthArr ayI6QLineFLi100EE [14]
0.00 0.00 1/1 __static_initialization_and_destruction_0(int, int) [15]
-----------------------------------------------
0.00 0.00 1/1 global constructors keyed to _ZN9GridScene9drawLinesEP8QPainterR15QVarLengthArr ayI6QLineFLi100EE [14]
[15] 0.0 0.00 0.00 1 __static_initialization_and_destruction_0(int, int) [15]
-----------------------------------------------

---

Well...[CUT BECAUSE SOMETHING WEIRD IS GOING ON...it seems to be as if drawing the lines one by one is faster...but somehow that is rather unbelievable on second thought]...


Another method of getting rid of spurious mallocs is to reuse Pens and Brushes...

I guess i will not repost all of the code for these little changes...



Does anyone has access to a profiler on windows/mac? I would like to know if those changes that were good for linux (ellipses, rect -> painterpath) lines drawn one by one, is also good for these palatforms ;-)

Quote:

---

Originally Posted by camel

Well, after some more testing I found out that it is cheaper (in the drawBackground) method to draw the lines one by one, than to regenerate a new painterpath everytime.

Another method of getting rid of spurious mallocs is to reuse Pens and Brushes...

I guess i will not repost all of the code for these little changes...



Does anyone has access to a profiler on windows/mac? I would like to know if those changes that were good for linux (ellipses, rect -> painterpath) lines drawn one by one, is also good for these palatforms ;-)

---

Yeah exactly I too would like to know opinion on windows especially. Did anyone notice performance difference between windows and linux when you select and move all items ?

Quote:

---

Originally Posted by Gopala Krishna

[2] 25.0 0.01 0.00 GridScene::drawBackground(QPainter*, QRectF const&) [2]
0.00 0.00 36506/36506 GridScene::setupArray(QVarLengthArray<QLineF, 100>&, QRectF const&) [12]
0.00 0.00 36506/36506 GridScene::drawLines(QPainter*, QVarLengthArray<QLineF, 100>&) [13]

---

That looks really weird.
The reason probably once again that the time is spent inside of Qt-Code and not yours, so sadly that does not help.
Or it could have been inlined, again not really helpfull :-/


Have you tried valgrind/callgrind and KCacheGrind as a profiler? It is rather nice :-)

Quote:

---

Originally Posted by camel

That looks really weird.
The reason probably once again that the time is spent inside of Qt-Code and not yours, so sadly that does not help.

---

These figures are wrong. All functions from inside Qt should be counted on behalf of the functions calling them and here all functions have nothing but zeros. The application needs to run longer.

Quote:

---

Or it could have been inlined, again not really helpfull :-/

---

Yes, that's possible, especially if they were implemented inside the class header and optimisations were enabled.

You must be sure to disable inlining optimisations. You can use -fno-inline to disable inlining.

Yup! you both are right. Anyway I am recompiling Qt tomorrow. I'll surely post after I do that. In the meanwhile I'd like to inform you people that I have never used cachegrind. Is that better than gprof ? If yes how should I use that ?

Did you try my last modification? Did it change anything for you?

Quote:

---

Originally Posted by Gopala Krishna

Yup! you both are right. Anyway I am recompiling Qt tomorrow. I'll surely post after I do that. In the meanwhile I'd like to inform you people that I have never used cachegrind. Is that better than gprof ? If yes how should I use that ?

---

You do not need to compile anything special (besides DEBUG of course ;-)

Then call

Quote:

---

$valgrind --tool=callgrind ./YOUR_APP

---

this will produce a file called callgrind.out.PID_OF_PROCESS

then you can call

Quote:

---

$kcachegrind callgrind.out.PID_OF_PROCESS

---

http://kcachegrind.sourceforge.net/cgi-bin/show.cgi

I have modified my application which uses QGraphicsView framework to draw a grid simmilar to yours and I implemented it in a worst possible way:

Code:

---

void drawBackground ( QPainter * painter, const QRectF & rect ){
  static QPen p(QColor(150,150,150));
  painter->setPen(p);
  for(int row=10+((int)(rect.top())/10)*10; row<rect.bottom(); row+=10){
    for(int col = 10+((int)(rect.left())/10)*10; col<rect.right(); col+=10)
       painter->drawPoint(col, row);
  }
}

---

Profiling info (compiled with -fno-inline -ggdb -pg) looks like this:

Code:

---

%   cumulative   self              self     total
 time   seconds   seconds    calls  us/call  us/call  name
 33.33      0.02     0.02  1547664     0.01     0.01  QRectF::right() const
 33.33      0.04     0.02                             SpecScene::drawBackground(QPainter*, QRectF const&)
 16.67      0.05     0.01  1527572     0.01     0.01  QPainter::drawPoint(int, int)

---

As you see 33% of the application time is spent in drawBackground and half of it is used to calculate QRectF::right() (which makes sense as it gets calculated in each iteration of the loop) and pretty much is used to actually draw the points.

Code:

---

index % time    self  children    called     name
                                                 <spontaneous>
[1]     83.3    0.02    0.03                 SpecScene::drawBackground(QPainter*, QRectF const&) [1]
                0.02    0.00 1547627/1547664     QRectF::right() const [2]
                0.01    0.00 1527572/1527572     QPainter::drawPoint(int, int) [3]
                0.00    0.00   20635/20651       QRectF::bottom() const [35]
                0.00    0.00   20055/20055       QRectF::left() const [36]
                0.00    0.00     580/580         QRectF::top() const [93]
                0.00    0.00       1/702         QColor::QColor(int, int, int, int) [92]

---

As you see it is actually more time consuming to calculate the rectangle coords than to draw points!

Some facts:
- I use Qt4.2.2 on i686 Linux,
- I didn't use antialiasing,
- I didn't use scaling (so I have an identity matrix when it comes to viewport-window transformations),
- scene size was about 1200x1000,
- I didn't compile Qt with profiling information (so I guess it's not required after all),
- I got detailed info about both mine and Qt methods,
- the result is pretty fast.

I got a pretty good result, but I didn't suffer from viewport-window transformations which might be your case if you scale the view. I didn't use antialiasing for my view, as points are points - they don't suffer from the aliasing effect. I could easily improve the implementation by calculating rectangle coordinates once per drawBackground() which should reduce the execution effort by half.

It is up to you to do the interpretation of the results. My impression is that it is not really drawBackground() which causes the slowdown - maybe it is just called too often by other parts of the system? Maybe you abuse update()?

Quote:

---

Originally Posted by camel

Did you try my last modification? Did it change anything for you?



You do not need to compile anything special (besides DEBUG of course ;-)

Then call


this will produce a file called callgrind.out.PID_OF_PROCESS

then you can call


http://kcachegrind.sourceforge.net/cgi-bin/show.cgi

---

Thanks ! Here is the output as attachment
EDIT: My quota of attachment is almost over. I'll update a bit later.

Quote:

---

Originally Posted by wysota

I have modified my application which uses QGraphicsView framework to draw a grid simmilar to yours and I implemented it in a worst possible way:

Profiling info (compiled with -fno-inline -ggdb -pg) looks like this:

Code:

---

%   cumulative   self              self     total
 time   seconds   seconds    calls  us/call  us/call  name
 33.33      0.02     0.02  1547664     0.01     0.01  QRectF::right() const
 33.33      0.04     0.02                             SpecScene::drawBackground(QPainter*, QRectF const&)
 16.67      0.05     0.01  1527572     0.01     0.01  QPainter::drawPoint(int, int)

---

---

Well, if you disable inlining, no wonder right() is expensive. calling a function 1.5 Million times just takes its time...inlining is there for a reason ;-)

So I would not call this run very exemplary... :-)

just a note:
If you like to profile further into the drawing part,
you need to call:

Quote:

---

valgrind --tool=callgrind --seperate-callers=5 --seperate-recs=10 ./YOUR_APP

---

Otherwise you will get "cycles", which is not very helpfull in this task..

BUT: this takes up much more memory to analyze later...

Hi,

well I just love to play with this... ;-)


Here is another (and much faster) way to write the drawBackground function...with the added niceness that it helps avoiding the color errors ;-)

You might want to check if this survives zooming etc.

Code:

---

void drawBackground(QPainter *painter, const QRectF &rect)
      {
         const int gridSize = 25;
 
        if (backgroundCache.isNull()) {
            backgroundCache = QPixmap(gridSize, gridSize);
            const int middle = gridSize / 2;
            QPainter backgroundPainter(&backgroundCache);
            backgroundPainter.setRenderHints(painter->renderHints());
            backgroundPainter.fillRect(QRect(0, 0, gridSize, gridSize), QBrush(Qt::white));
            backgroundPainter.setPen(backgroundPen);
            backgroundPainter.setBrush(backgroundBrush);
            backgroundPainter.drawLine(0, middle, gridSize, middle);
            backgroundPainter.drawLine(middle, 0, middle, gridSize);
        }
 
         const int realLeft = static_cast<int>(std::floor(rect.left()));
         const int realRight = static_cast<int>(std::ceil(rect.right()));
         const int realTop = static_cast<int>(std::floor(rect.top()));
         const int realBottom = static_cast<int>(std::ceil(rect.bottom()));
 
 
         const int firstLeftGridLine = realLeft - (realLeft % gridSize);
         const int firstTopGridLine = realTop - (realTop % gridSize);
 
 
         QPainterPath background;
         for (int x = firstLeftGridLine; x < realRight; x += gridSize) {
            for (int y = firstTopGridLine; y < realBottom; y += gridSize) {
                painter->drawPixmap(x, y, backgroundCache);
            }
        }
      }

---

Have a nice day :-)

Quote:

---

Originally Posted by camel

just a note:
If you like to profile further into the drawing part,
you need to call:


Otherwise you will get "cycles", which is not very helpfull in this task..

BUT: this takes up much more memory to analyze later...

---

I get the following error

Quote:

---

valgrind: Bad option '--seperate-callers=5'; aborting.

---

I am trying to analyse the code as it is now. I am getting lost here and there but kcachegrind is a cool tool. Needs some time to get used to it. Thanks for helping me till now. I'll continue with this tomorrow.

Quote:

---

Originally Posted by camel

Well, if you disable inlining, no wonder right() is expensive. calling a function 1.5 Million times just takes its time...inlining is there for a reason ;-)

So I would not call this run very exemplary... :-)

---

It means that QRectF::right() takes longer than QPainter::drawPoint() and right() is not a very complex method :)

After getting rid of the right() and bottom() calls inside the loop the result of profiling is as follows:

Code:

---

50.00      0.06     0.06                             SpecScene::drawBackground(QPainter*, QRectF const&)
 25.00      0.09     0.03  1460235     0.02     0.02  QPoint::QPoint(int, int)
 16.67      0.11     0.02  1454915     0.01     0.03  QPainter::drawPoint(int, int)

---

The QPoint constructor is called by QPainter::drawPoint, so we can forget about it. The whole method takes over 90% of the whole application time and gives a total of less than 0.1s. That's not much :)

It is important to see what the bottleneck is instead of shooting blind. No matter how much you optimise QPainter::drawLines() if it's not the bottleneck, you won't get a decent improvement.

In my opinion it would be much simpler and faster to simply apply a backgroundBrush with the grid to the scene and forget about points and lines. You wouldn't get any painter paths then, no floating point operations. The only thing that could slow down the process is the matrix transformation, so try to avoid it.

Quote:

---

Originally Posted by Gopala Krishna

I get the following error
valgrind: Bad option '--seperate-callers=5'; aborting.

---

I think they changed the option names during 3.1 and 3.2 (which I use)

try:
valgrind --tool=callgrind --fn-caller=5 --fn-recursion=10 ./YOUR_APP

Quote:

---

Originally Posted by camel

Hi,

well I just love to play with this... ;-)


Here is another way to write the drawBackground function...with the added niceness that it helps avoiding the color errors ;-)

You might want to check if this survives zooming etc.

Code:

---

void drawBackground(QPainter *painter, const QRectF &rect)
      ...

---

Have a nice day :-)

---

Not bad idea! :) That problem solved !!! Thanks a lot.
Now the next problem is performance ! :(
A good day to you too :)

Quote:

---

Originally Posted by wysota

It is important to see what the bottleneck is instead of shooting blind. No matter how much you optimise QPainter::drawLines() if it's not the bottleneck, you won't get a decent improvement.

---

That is why I look into my KCachgrind and follow the callgraph and sources ;-)

The problem is not drawPoints, as that does not do much.

The problem with drawLines and friends is that it often (with antialiasing) first creates a painterpath from those lines and then paints them. This takes time, not least because of memory allocation.

This is why I said use painterpaths directly.

Optimizing the returnvalue of boundingrect and shape is so easy (and makes the code much nicer in my opinion) so that I would not even consider it a optimization, its just nicer programming. (besides helping during run-time of course ;-)

Quote:

---

Originally Posted by wysota

In my opinion it would be much simpler and faster to simply apply a backgroundBrush with the grid to the scene and forget about points and lines. You wouldn't get any painter paths then, no floating point operations. The only thing that could slow down the process is the matrix transformation, so try to avoid it.

---

I cuncur there, see my last post about the drawbackground function.
To use a background brush would be the logical next step ;-)

Quote:

---

Originally Posted by camel

The problem with drawLines and friends is that it often (with antialiasing) first creates a painterpath from those lines and then paints them. This takes time, not least because of memory allocation.

This is why I said use painterpaths directly.

---

If you disable antialiasing, there is a possibility that painter paths will not be used. And there is no point in having antialiasing enabled for drawing horizontal or vertical lines, as these don't suffer from the aliasing effect. You can safely temporarily disable antialiasing while painting those.

Code:

---

bool isAA = painter->renderHints() & QPainter::Antialiasing;
painter->setRenderHint(QPainter::Antialiasing, false);
painter->drawLines(...);
painter->setRenderHint(QPainter::Antialiasing, isAA);

---

Another possibility of painter paths being active is that there is scaling involved. But while zooming in, the number of lines needed to draw should decrease. While zooming out on the other hand you can draw fewer lines as too many of them will clutter the scene anyway.

You can also store the path in a member variable and then only draw the path on the painter. But I think using a background brush will be much faster, especially with smooth scaling disabled.

Quote:

---

Originally Posted by wysota

In my opinion it would be much simpler and faster to simply apply a backgroundBrush with the grid to the scene and forget about points and lines.

---

Well I tried even this. That is not the problem. First of all the main problem is while moving "many items" together. For example in the above eg the performance goes down if you select all resistors and move - thats about 80 items at a time!!! (20 resistors,40 nodes, 20 text items) . Here to get sufficient performance the area to be updated should be selected appropriately and efficiently so that the scene is updated least number of times.

Further I have a question - is the the bsp indexing causing these performance problems ?

Further I would like to inform that I am not using any matrix transformations. All these are just on regular matrix.

Quote:

---

Originally Posted by wysota

If you disable antialiasing, there is a possibility that painter paths will not be used. And there is no point in having antialiasing enabled for drawing horizontal or vertical lines, as these don't suffer from the aliasing effect. You can safely temporarily disable antialiasing while painting those.

---

The problem being of course that the lines look differenty ;-)
But some people might see that as an advantage in this case.

Quote:

---

Originally Posted by wysota

Another possibility of painter paths being active is that there is scaling involved. But while zooming in, the number of lines needed to draw should decrease. While zooming out on the other hand you can draw fewer lines as too many of them will clutter the scene anyway.

You can also store the path in a member variable and then only draw the path on the painter. But I think using a background brush will be much faster, especially with smooth scaling disabled.

---

In this case the brush is faster. Storing the path in a member variable is not possible in this case, as the lines to draw will change wildly during repaints, and I am not sure how efficient it would be to crop out the rest if you draw all possible lines.

For the other items, I did just that, storing the path. which works out quite well. I might be possible to also use pixmaps here, but that is probably rather a space/time tradeoff... :-)

Quote:

---

Originally Posted by Gopala Krishna

First of all the main problem is while moving "many items" together.

---

So the problem is not within drawBackground....

Quote:

---

Further I have a question - is the the bsp indexing causing these performance problems ?

---

Yes, you should disable the bsp indexing while moving many items at once and enable it again afterwards. Every time you move an item, the tree may need to be rebuilt. Multiply that by the number of items and number of moves you perform...

Quote:

---

Further I would like to inform that I am not using any matrix transformations. All these are just on regular matrix.

---

If you use zooming, then you are using matrix transformations. You are using them anyway as I'm almost 100% sure viewport-window conversion and/or the transformation matrix is used to obtain local coordinate system for items :)

Quote:

---

Originally Posted by camel

The problem being of course that the lines look differenty ;-)

---

Could you explain what you mean by that? Antialiasing shouldn't have any positive effect on horizontal/vertical lines... You can reduce the contrast by using antialiasing on horizontal/vertical lines, but that doesn't improve the line. You can just change the colour you use for drawing and have the same optical effect...

By the way... by a "line" I understand a "thin line" not a "thick filled rectangle". Of course you can still have a simmilar effect to antialiasing by using a pen different than a brush...

Quote:

---

Originally Posted by wysota

So the problem is not within drawBackground....

---

No, it is. I just said when the problem shows up. The main problem is probably not "within" drawBackground but the method which calls it more often than necessary. I mean though we optimize drawBackground() , we need to find out why it is called many times than necessary. I feel this because of experimenting with larger boundingRect gave good performance - i.e lesser drawBackground calls.

Quote:

---

Originally Posted by wysota

Yes, you should disable the bsp indexing while moving many items at once and enable it again afterwards. Every time you move an item, the tree may need to be rebuilt. Multiply that by the number of items and number of moves you perform...

---

I tried by disabling indexing in mousePressEvent() when selectedItems().isEmpty() == false and enabling it again in release event. This didn't help much either :(

Quote:

---

Originally Posted by wysota

If you use zooming, then you are using matrix transformations. You are using them anyway as I'm almost 100% sure viewport-window conversion and/or the transformation matrix is used to obtain local coordinate system for items :)

---

Yeah, i missed this. You are right, but anyway these are unavoidables and are for convenience.

Quote:

---

Originally Posted by wysota

Could you explain what you mean by that? Antialiasing shouldn't have any positive effect on horizontal/vertical lines... You can reduce the contrast by using antialiasing on horizontal/vertical lines, but that doesn't improve the line. You can just change the colour you use for drawing and have the same optical effect...

By the way... by a "line" I understand a "thin line" not a "thick filled rectangle". Of course you can still have a simmilar effect to antialiasing by using a pen different than a brush...

---

I did not say "positive effect", I just said "effect"

Why the effect:
A one-pixel line not antialiased is exactly one pixel wide
A one-pixel line antialiased will have the centre of the line on the border between two pixels. (See here


If you do not use anti-aliasing for all horizontal and vertical lines (such as the resistors in this example) you will have two very different looking lines.

You might like the effect of having different lines or not, but they will look different :-)

Quote:

---

Originally Posted by Gopala Krishna

The main problem is probably not "within" drawBackground but the method which calls it more often than necessary.

---

That's why I mentioned update(). Try reducing the number of items (for example by simplifying the resistor item to consist of only one item), it may improve performance.

Quote:

---

I mean though we optimize drawBackground() , we need to find out why it is called many times than necessary.

---

Funny... I have just taken a look at the profiling info you provided and it seems that it's not mentioned how many time drawBackground() is called. But it might be because of an incorrect compilation of the application (that's why you don't get detailed profiling information for drawBackground()).

Quote:

---

I feel this because of experimenting with larger boundingRect gave good performance - i.e lesser drawBackground calls.

---

Increasing boundingRect() causes less modifications of the BSP tree while moving items.

Quote:

---

I tried by disabling indexing in mousePressEvent() when selectedItems().isEmpty() == false and enabling it again in release event. This didn't help much either :(

---

Try disabling it entirely.

Quote:

---

Originally Posted by camel

I did not say "positive effect", I just said "effect"

---

OK :)

Quote:

---

If you do not use anti-aliasing for all horizontal and vertical lines (such as the resistors in this example) you will have two very different looking lines.

---

I didn't say not to antialias the resistors but the grid. You won't notice any differences between lines then as all grid lines will be drawn in the same way and you can manipulate the contrast (as antialiasing blurs the edges of lines to reduce the contrast) by changing the colour of the pen.

I just wanted to post my last version of the to-be-optimized app.

Little changes I made, were for example:
* making sure that it actually paints correctly (also when the view gets to big, and we start to receive negative coordinates ;-)
* setting QGraphicsScene::NoIndex, with no obvious negative effects
* noticing that it is actually faster (on X11) to draw the pixmap by hand a few times than to use a brush :-/
* setting clipping to the exposed rect while painting the nodes

The top timespenders in the app are basically all related to QRegion, i.e. collision detection and repainting. These can only be optimized...well indirectly...


Anyways, if anybody would like to try out that version, here it is

Code:

---

#include <QtGui>
#include <cmath>
 
 
namespace {
    static inline int gridFloor(const qreal& value)
    {
        if (value < 0) {
            return -1 * static_cast<int>(std::ceil(qAbs(value)));
        } else {
            return static_cast<int>(std::floor(value));
        }
    }
 
    static inline int gridCeil(const qreal &value)
    {
        if (value < 0) {
            return -1 * static_cast<int>(std::floor(qAbs(value)));
        } else {
            return static_cast<int>(std::ceil(value));
        }
    }
 
    static inline int nextSmallerGrid(const int &value, const int &gridSize)
    {
        if (value < 0) {
            const int absValue = qAbs(value);
            return -1 * (absValue + gridSize - (absValue % gridSize));
        } else {
            return value - (value % gridSize);
        }
    }
}
 
class GridScene : public QGraphicsScene
{
   public:
      GridScene(qreal x, qreal y, qreal w, qreal h)
         : QGraphicsScene(x, y, w, h)
    {
        setItemIndexMethod(QGraphicsScene::NoIndex);
    }
 
   protected:
      void drawBackground(QPainter *painter, const QRectF &rect)
      {
          const int gridSize = 25;
 
          if (backgroundCache.isNull()) {
              backgroundCache = QPixmap(gridSize, gridSize);
              const int middle = gridSize / 2;
              {
                  QPainter backgroundPainter(&backgroundCache);
                  backgroundPainter.setRenderHints(painter->renderHints());
                  backgroundPainter.fillRect(QRect(0, 0, gridSize, gridSize), QBrush(Qt::white));
                  backgroundPainter.setPen(QPen(Qt::darkGreen,0));
                  backgroundPainter.setBrush(Qt::NoBrush);
                  backgroundPainter.drawLine(0, middle, gridSize, middle);
                  backgroundPainter.drawLine(middle, 0, middle, gridSize);
              }
          }
 
          const int realLeft = gridFloor(rect.left());
          const int realRight = gridCeil(rect.right());
          const int realTop = gridFloor(rect.top());
          const int realBottom = gridCeil(rect.bottom());
 
 
          const int firstLeftGridLine = nextSmallerGrid(realLeft, gridSize);
          const int firstTopGridLine = nextSmallerGrid(realTop, gridSize);
 
          QRectF cacheRect(0, 0, gridSize, gridSize);
          for (int x = firstLeftGridLine; x < realRight; x += gridSize) {
              for (int y = firstTopGridLine; y < realBottom; y += gridSize) {
                  painter->drawPixmap(QRectF(x, y, gridSize, gridSize),
                                      backgroundCache,
                                      cacheRect);
              }
          }
      }
 
   private:
      QPixmap backgroundCache;
};
 
namespace {
    static inline QPainterPath constructNodeShape(const QRectF& elipseRect)
    {
        QPainterPath path;
        path.addEllipse(elipseRect);
        return path;
    }
}
 
class Node : public QGraphicsItem
{
   public:
      Node(QGraphicsItem *par = 0, QGraphicsScene *sc = 0)
          : QGraphicsItem(par,sc),
            elipseRect(-4.0, -4.0, 8.0, 8.0),
            elipsePath(constructNodeShape(QRectF(-3.0, -3.0, 2.0*3.0, 2.0*3.0))),
            elipseShape(constructNodeShape(elipseRect)),
            nodePen(Qt::darkRed),
            nodeBrush(Qt::NoBrush)
      {
         setFlags(0);
         setAcceptedMouseButtons(0);
      }
 
      void paint(QPainter* p,const QStyleOptionGraphicsItem *o, QWidget *)
      {
         p->setPen(nodePen);
         p->setBrush(nodeBrush);
         p->setOpacity(1.0);
 
         p->setClipRect(o->exposedRect);
         p->drawPath(elipsePath);
      }
 
      QPainterPath shape() const
      {
         return elipseShape;
      }
 
      QRectF boundingRect() const
      {
         return elipseRect;
      }
 
    protected:
      const QRectF elipseRect;
      const QPainterPath elipsePath;
      const QPainterPath elipseShape;
      const QPen nodePen;
      const QBrush nodeBrush;
};
 
 
namespace {
    static inline QPainterPath constructResistorPath()
    {
        QPainterPath resistorPath;
        resistorPath.addRect(QRectF(-18.0, -9.0, 36.0, 18.0));
        resistorPath.moveTo(-27, 0);
        resistorPath.lineTo(-18, 0);
        resistorPath.moveTo(18, 0);
        resistorPath.lineTo(27, 0);
        return resistorPath;
    }
}
 
class Resistor : public QGraphicsItem
{
   public:
      Resistor(QGraphicsItem *par = 0, QGraphicsScene *scene = 0)
          : QGraphicsItem(par,scene),
            resistorPath(constructResistorPath()),
            boundingBox(resistorPath.boundingRect().adjusted(-1, -1, 1, 1)),
            resistorSelectedPen(Qt::darkBlue),
            resistorOpenPen(Qt::darkGray,1),
            resistorBrush(Qt::NoBrush)
      {
         setFlags(ItemIsMovable | ItemIsSelectable | ItemIsFocusable);
         // comment the following  4 lines to see the performance difference
         QGraphicsTextItem *t = new QGraphicsTextItem("R1 = 100k",this,scene);
         t->setPos(0,-35);
         Node * node = new Node(this,scene);
         node->setPos(QPointF(-30,0));
         node = new Node(this,scene);
         node->setPos(QPointF(30,0));
      }
 
      void paint(QPainter *p, const QStyleOptionGraphicsItem *o, QWidget *)
      {
         if(!(o->state & QStyle::State_Open))
            p->setPen(resistorOpenPen);
         if(o->state & QStyle::State_Selected)
            p->setPen(resistorSelectedPen);
 
         p->setClipRect( o->exposedRect );
         p->setBrush(resistorBrush);
         p->drawPath(resistorPath);
      }
 
      QRectF boundingRect() const
      {
        return boundingBox;
      }
 
   private:
      const QPainterPath resistorPath;
      const QRectF boundingBox;
      const QPen resistorSelectedPen;
      const QPen resistorOpenPen;
      const QBrush resistorBrush;
};
 
int main(int argc,char *argv[])
{
   QApplication app(argc,argv);
   GridScene scene(0,0,1024,768);
   for(int j = 2; j < 4; ++j)
      for(int i = 1; i <11; ++i)
      {
         Resistor *r = new Resistor(0,&scene);
         r->setPos(j*100, i * 50);
      }
   QGraphicsView *view = new QGraphicsView(&scene);
   view->setRenderHints(QPainter::Antialiasing | QPainter::SmoothPixmapTransform);
   view->setDragMode(QGraphicsView::RubberBandDrag);
   view->show();
   return app.exec();
}

---

Uiiii....that was fun... ;-)

Quote:

---

Originally Posted by wysota

That's why I mentioned update(). Try reducing the number of items (for example by simplifying the resistor item to consist of only one item), it may improve performance.

---

This won't be possible since I need node to be independent of component.

Quote:

---

Originally Posted by wysota

Funny... I have just taken a look at the profiling info you provided and it seems that it's not mentioned how many time drawBackground() is called. But it might be because of an incorrect compilation of the application (that's why you don't get detailed profiling information for drawBackground()).

---

I'll post the proper one after recompilation today.

Quote:

---

Originally Posted by wysota

Increasing boundingRect() causes less modifications of the BSP tree while moving items. Try disabling it entirely.

---

Could the last post in this thread mean an improvement of performance in Qt4.3 with indexing enabled.

Quote:

---

Originally Posted by camel

I just wanted to post my last version of the to-be-optimized app.

Little changes I made, were for example:
* making sure that it actually paints correctly (also when the view gets to big, and we start to receive negative coordinates ;-)
* setting QGraphicsScene::NoIndex, with no obvious negative effects
* noticing that it is actually faster (on X11) to draw the pixmap by hand a few times than to use a brush :-/
* setting clipping to the exposed rect while painting the nodes

The top timespenders in the app are basically all related to QRegion, i.e. collision detection and repainting. These can only be optimized...well indirectly...


Anyways, if anybody would like to try out that version, here it is

Code:

---

#include <QtGui>
#include <cmath>
 
....

---

Uiiii....that was fun... ;-)

---

Thanks for the cleaner optimized version. :)

Just to fine tune my understanding. Is the indexing helpful only if we use QGraphicsScene::items() in our code? Doesn't the framework use this internally (while detecting event receiving item)thereby reducing speed?

Quote:

---

Originally Posted by Gopala Krishna

This won't be possible since I need node to be independent of component.

---

As far as I remember you had an additional item for displaying some text which you can easily get rid of. But I may be wrong, maybe it was someone else with such an architecture.

Quote:

---

I'll post the proper one after recompilation today.

---

I meant that the compilation was not correct, not the method.

Quote:

---

Could the last post in this thread mean an improvement of performance in Qt4.3 with indexing enabled.

---

Yes. Andreas describes the exact solution you should use.

Quote:

---

Originally Posted by Gopala Krishna

Just to fine tune my understanding. Is the indexing helpful only if we use QGraphicsScene::items() in our code? Doesn't the framework use this internally (while detecting event receiving item)thereby reducing speed?

---

It is helpfull for answering a question "What parts of the canvas does a particular item occupy?" which is strongly related to "What items collide with point X?". So it is for example used to swiftly determine which item was clicked.

Quote:

---

Originally Posted by Gopala Krishna

Just to fine tune my understanding. Is the indexing helpful only if we use QGraphicsScene::items() in our code? Doesn't the framework use this internally (while detecting event receiving item)thereby reducing speed?

---

Yes the framework uses it internally, but it is a tradeoff (as so much in life ;-)

When you use the Index, the collision-detection may be sped up, but every position/size change of every item will cost you.
If you do not use the index, finding an item at a certain location will probably be linear to the number of items in the scene, but you can move/change very cheaply.

If you have many moving things but not so many overall, turning of the index may be a good idea.
If you have many mostly stationary items, the index is very nice indeed.

In the end it depends on experimentation (or very carefull analysis ;-) to find the optimal solution for you.


Since you were keen on moving many items at the same time, it is disabled here. ;-)

I think one could think about it like this (for n: all items, x:moving items)
Looking up items in the index costs log(n)but returns a filtered selection of items z with n >= z >= x
When you have a repaint, you need to find the repainted items, check if they are really included in the repainted area and then repaint them (which we ignore since it does not change with the use of the index ;-), which will cost about O(log(n) + z). Since you are moving you also need to update the index which cost O(log(n)) which means updating for all items costs O(log(n) * x). If we say that painting and updating is done in lockstep, each this step will cost O(x*log(n) + z). (let us hope that the index is sufficiently well thought out so that z is always very near to x and just say the cost is O(x*log(n)))

If you do not use the index, you have to check each item if it is included, then repaint , ie it will cost O(n). When we are moving we do not need to update anything, thus the total cost will be somewhere around O(n)

Now we have the choice O(n) or O(x*log(n)) which is better?
Depends on your purposes, if you have the need to move/change nearly as many items as there are in the scene, the non index version with O(n) is better than the index version with O(n*log(n)).
If the number of changing/moving items is much smaller than the total items, ie n >> x, the index version is clearly better O(log(n) * small number) vs. O(n)



So that concludes my lecture on the run time behavior of the graphics scene. Next week: Mating Rituals of the Norwegian Widget.

[By the way, since half of the time I do not know what I talk about take everything here with a grain of salt ;-) ]

EDIT:
Actually there is another factor in there: The checks for the repaints is done per "dirty" area d, which should (for moving items) be d < 2*x. Why ? The dirty areas may be combined if they are overlapping but you have always one for where the object was, and one for where the object is.

So in the end it would realy be:
O(log(n) * x * d) vs. O(n * d)
This explains for example why it was faster for you when you increased the bounding rect of the resistors. By increasing the bounding rect you created overlapping regions and thus less work had to be done.
Worst case: O(log(n) * n) when overlapping vs. O(log(n) * n^2) when not.

EDIT 2:
As Andreas explains in the post you mentioned, the index will be changed in 4.3 so this analysis is not valid for Qt > 4.2