Qt 4.6 GraphicsView items remove problem

[Piskvorkar]

Hi,
I've some strange problem with Qt 4.6.2. I spent long time with trying to fix it, but at the end i only downgrade to Qt 4.5.3, where my application is functional.

Problem is, that when I remove my own item from QGraphicsScene, sometimes it still remains on scene. It's not anymore clickable, but it's realy on scene, no update() or repaint helps. It seems, that problem occures only when I'm adding items using parser (so probably too quickly).

Isn't there any new flag in Qt 4.6? I have had first problem with items didn't send geometry changes, but setFlag(ItemSendsGeometryChanges); was what I need.

But now I realy don't know, where could be problem. When I use Qt 4.5.3, application works properly. Could it be caused by some optimalization changes?

Thanks for any answers.

[Piskvorkar]

I did some measurement a I found this:
I've small example with about 18 graphics items on scene. When I use QGraphicsScene::items().count and it returns 18. I can remove 2 of them a than the same method returns correctly 16. But there is still removed items displayed on scene.

However, if found the way how to update it to display items correctly:
I have to change QGraphicsScene::sceneRect() and than upate scene. So if I want to get properly functionality, i have to call QGraphicsScene::setSceneRect(...) and set sceneRect e.g to scene()->sceneRect().adjusted(1,1,1,1) and then revert it and finally call scene()->update(); and whole scene is then painted correctly.
So I thing if there could be some cached content in scene or in view?

Any ideas?
Thanks.

[wysota]

Please show us the implementation of your custom items. paint() and boundingRect() are the most interesting routines.

[Piskvorkar]

Ok, here is sample of my code:

paint method:

void TransitionArc::paint(QPainter *painter, const QStyleOptionGraphicsItem *, QWidget *)
{
    if (startPoint == endPoint) return;
 
    QColor lineC;   
    Qt::PenStyle lineS;
    float lineWidth;
    float edgeLineWidth_e = editor->edgeLineWidth*TR_LINE_VIEW_COEF; // better looking on scene
 
    if (dimmed){
        lineWidth = edgeLineWidth_e * dimEdgeLineCoef;
        lineS = dimEdgeLineStyle;
        lineC = dimEdgeLineColor;
    } else{
        lineWidth = edgeLineWidth_e * edgeLineWidth;
        lineS = edgeLineStyle;
        lineC = edgeLineColor;
    }
 
    if (checked){
        painter->setPen(QPen(QColor(255,100,100), lineWidth, lineS));
    } 
    else{
        painter->setPen(QPen(lineC, lineWidth, lineS));
    }
 
    painter->setBrush(Qt::NoBrush);
// p is clasls variable QPainterPath, updated in adjust method, which is called when
// changed (position or size) -> this works correctly
    painter->strokePath(p, painter->pen());
 
    // draw arrow
    if (checked)
        painter->setBrush(QColor(255,100,100));
    else
        painter->setBrush(lineC);
 
    painter->drawPolygon(pa); // coumputed in adjust
}

void TransitionArc::paint(QPainter *painter, const QStyleOptionGraphicsItem *, QWidget *)
{
    if (startPoint == endPoint) return;
  
    QColor lineC;   
    Qt::PenStyle lineS;
    float lineWidth;
    float edgeLineWidth_e = editor->edgeLineWidth*TR_LINE_VIEW_COEF; // better looking on scene
  
    if (dimmed){
        lineWidth = edgeLineWidth_e * dimEdgeLineCoef;
        lineS = dimEdgeLineStyle;
        lineC = dimEdgeLineColor;
    } else{
        lineWidth = edgeLineWidth_e * edgeLineWidth;
        lineS = edgeLineStyle;
        lineC = edgeLineColor;
    }
  
    if (checked){
        painter->setPen(QPen(QColor(255,100,100), lineWidth, lineS));
    } 
    else{
        painter->setPen(QPen(lineC, lineWidth, lineS));
    }

    painter->setBrush(Qt::NoBrush);
// p is clasls variable QPainterPath, updated in adjust method, which is called when
// changed (position or size) -> this works correctly
    painter->strokePath(p, painter->pen());

    // draw arrow
    if (checked)
        painter->setBrush(QColor(255,100,100));
    else
        painter->setBrush(lineC);
      
    painter->drawPolygon(pa); // coumputed in adjust
}

To copy to clipboard, switch view to plain text mode 

boundingRect metohod is implemented in superclass as virtual

QRectF Transition::boundingRect() const
{
    QPolygonF areaPol = makeClickArea(p);    // p is clasls variable QPainterPath
 
    QRectF rect = areaPol.boundingRect();
 
    if (label)
        rect |= (mapFromItem(label, label->boundingRect())).boundingRect(); // rect over pathRect and labelRect
 
    for (TLabelXList::ConstIterator labelIt = nextLabels.begin();
         labelIt != nextLabels.end();
         ++labelIt)
    {
        rect |= (mapFromItem((*labelIt), (*labelIt)->boundingRect())).boundingRect();
    }        
 
    rect |= pa.boundingRect();
 
    return rect;
}

QRectF Transition::boundingRect() const
{
    QPolygonF areaPol = makeClickArea(p);    // p is clasls variable QPainterPath
    
    QRectF rect = areaPol.boundingRect();
    
    if (label)
        rect |= (mapFromItem(label, label->boundingRect())).boundingRect(); // rect over pathRect and labelRect

    for (TLabelXList::ConstIterator labelIt = nextLabels.begin();
         labelIt != nextLabels.end();
         ++labelIt)
    {
        rect |= (mapFromItem((*labelIt), (*labelIt)->boundingRect())).boundingRect();
    }        

    rect |= pa.boundingRect();

    return rect;
}

To copy to clipboard, switch view to plain text mode 

and finally my shape() method:

QPainterPath Transition::shape() const
{
    QPainterPath path;    
 
    QPolygonF areaPol = makeClickArea(p);        
 
    areaPol = areaPol.united(pa);
 
    if (label != NULL)
        areaPol = areaPol.united(mapFromItem(label, label->boundingRect()));
 
    for (TLabelXList::ConstIterator labelIt = nextLabels.begin();
         labelIt != nextLabels.end();
         ++labelIt)
    {
        areaPol = areaPol.united(mapFromItem((*labelIt), (*labelIt)->boundingRect()));
    }
 
    path.addPolygon(areaPol);    
 
    return path;    
}

QPainterPath Transition::shape() const
{
    QPainterPath path;    

    QPolygonF areaPol = makeClickArea(p);        

    areaPol = areaPol.united(pa);

    if (label != NULL)
        areaPol = areaPol.united(mapFromItem(label, label->boundingRect()));

    for (TLabelXList::ConstIterator labelIt = nextLabels.begin();
         labelIt != nextLabels.end();
         ++labelIt)
    {
        areaPol = areaPol.united(mapFromItem((*labelIt), (*labelIt)->boundingRect()));
    }

    path.addPolygon(areaPol);    

    return path;    
}

To copy to clipboard, switch view to plain text mode 

In attachements are sample images of my application:
- blue and green polygons are only for my testing, blue == shape polygon and green == bounding rect of item -> green is only for TransitionArc
- note: i create my own shape polygon bigger then line/curve only for make clicking on it easier (is it possible to do it simplier?)
- sample.png is state when I loaded automata
- sample_wrong.png is state after I deleted state 2 and move with state 1. Right Arc is still repainted even if I move with state 1 over it. But in Qt 4.5.3 I have no problem.

[wysota]

Are you sure you are always painting only inside the boundingRect() for each and every of your items? Does the boundingRect() ever change (return a different value than earlier)? Do you notify the architecture about such changes?

[Piskvorkar]

Thanks for reply, but I'm affraid that I'm not realy understand.
I'm sure about painting ever inside bounding rect - it has been reason why I have done my testing polygons(green).
But what do you mean with boiundingRect() -> ever change? I thought that boundingRect() is called automaticly from graphicsView and since it is computed from my internal variables, it should be ever alright?
So I thing, that I'm ever change boudningRect and shape when I move or change some item on scene, it should be enough? Or not?

But I think about what you have written and I tried some change. In my adjust method. I had called update() at its end before my change:

void adjust()
{
    // some changed with my path variable
    ...
    update();
}

void adjust()
{
    // some changed with my path variable
    ...
    update();
}

To copy to clipboard, switch view to plain text mode 

and now I have tried this:

void adjust()
{
    prepareGeometryChanges();
    ...
    // some changes on my path variable
    // no update() method called
}

void adjust()
{
    prepareGeometryChanges();
    ...
    // some changes on my path variable
    // no update() method called
}

To copy to clipboard, switch view to plain text mode 

And I'm realy surprised, because it is functional. Could you please try to explain it to me? Is there any important difference? (I thought that prepareGeometryChange only causes update when it's necessary).
What's implementation of prepareGeometryChanges? .. Does it create some local object which do some comparation in its destructor?

[Piskvorkar]

I addition i would like to counterpoint, that when I was using Qt 4.5.3, everything was ok. So there has to be some important behavior change in Qt 4.6. Isn't here some different scene updating? I suppose that now when I call prepareGeometryChange() I will inform environment about change in most cases then before, but why update() doesn't it too?

[wysota]

I'm sure about painting ever inside bounding rect - it has been reason why I have done my testing polygons(green).

I can see the polygons for the arcs only.

But what do you mean with boiundingRect() -> ever change? I thought that boundingRect() is called automaticly from graphicsView and since it is computed from my internal variables, it should be ever alright?

Graphics View won't requery your item for its boundingRect() unless you specifically tell it that it has changed. So if at some point your boundingRect() implementation was to return a different value than it did a second earlier, you are obligated to tell GV about it by calling your item's QGraphicsItem::prepareGeometryChange() method.

And I'm realy surprised, because it is functional. Could you please try to explain it to me?

I think I just did

What's implementation of prepareGeometryChanges? .. Does it create some local object which do some comparation in its destructor?

Qt is Open Source - just dive into the sources and see for yourself.

I addition i would like to counterpoint, that when I was using Qt 4.5.3, everything was ok.

This doesn't mean anything. If you are abusing the architecture by doing some things that were never meant to happen you can't expect it to work in future releases.

So there has to be some important behavior change in Qt 4.6.

Even more than one... That's why the version numbering changed from 4.5.x to 4.6.x you know

[Piskvorkar]

Thanks very much!
But I still don't understend, why update() doesn't notifiy GV about my changes. I used update() before (in my adjust method - only method when I'm doing items changes) and so I thought, that I'm notifying GV about changes. But in Qt 4.6 it's not right more. However in documentation for QGraphicsItem is still written:

Schedules a redraw of the area covered by rect in this item. You can call this function whenever your item needs to be redrawn, such as if it changes appearance or size. ....

If the only thing prepareGeometryChanges() does is calling update if necessarey, why I can't use update() instead? It was the reason I've called update() at the end of my adjust() method, to be sure that it will be called ever - maybe not good for performance ...

But I've just read this for QGraphicsItem::boundingRect():

f you want to change the item's bounding rectangle, you must first call prepareGeometryChange().

So I'm quite confused from it :-). Does update() need any other methods called before itsefl in order to GV realy update item on scene?

It's not so important since it's functionaly for me, but I'm interesting about it.
Once more thanks for replies!

[wysota]

But I still don't understend, why update() doesn't notifiy GV about my changes.

If it did, the scene would have to rebuild all its internal structures for every item every time it repainted itself. update() only says "hey, repaint me".

I used update() before (in my adjust method - only method when I'm doing items changes) and so I thought, that I'm notifying GV about changes.

Only about changes to the looks of the item but not to the size of it.

But in Qt 4.6 it's not right more.

It has never been.

If the only thing prepareGeometryChanges() does is calling update if necessarey

That's not true. The docs say that a redraw is scheduled as part of the method, they don't say it is the only thing that is done there.

why I can't use update() instead?

Laziness is not an excuse, taking even a brief look at the source code of the method may be enlightening.

void QGraphicsItem::prepareGeometryChange()
{
    if (d_ptr->inDestructor)
        return;
    if (d_ptr->scene) {
        d_ptr->scene->d_func()->dirtyGrowingItemsBoundingRect = true;
        d_ptr->geometryChanged = 1;
        d_ptr->paintedViewBoundingRectsNeedRepaint = 1;
        d_ptr->notifyBoundingRectChanged = !d_ptr->inSetPosHelper;
 
        QGraphicsScenePrivate *scenePrivate = d_ptr->scene->d_func();
        scenePrivate->index->prepareBoundingRectChange(this);
        scenePrivate->markDirty(this, QRectF(), /*invalidateChildren=*/true, /*force=*/false,
                                /*ignoreOpacity=*/ false, /*removingItemFromScene=*/ false,
                                /*updateBoundingRect=*/true);
 
        // For compatibility reasons, we have to update the item's old geometry
        // if someone is connected to the changed signal or the scene has no views.
        // Note that this has to be done *after* markDirty to ensure that
        // _q_processDirtyItems is called before _q_emitUpdated.
        if (scenePrivate->isSignalConnected(scenePrivate->changedSignalIndex)
            || scenePrivate->views.isEmpty()) {
            if (d_ptr->hasTranslateOnlySceneTransform()) {
                d_ptr->scene->update(boundingRect().translated(d_ptr->sceneTransform.dx(),
                                                               d_ptr->sceneTransform.dy()));
            } else {
                d_ptr->scene->update(d_ptr->sceneTransform.mapRect(boundingRect()));
            }
        }
    }
 
    d_ptr->markParentDirty(/*updateBoundingRect=*/true);
}

void QGraphicsItem::prepareGeometryChange()
{
    if (d_ptr->inDestructor)
        return;
    if (d_ptr->scene) {
        d_ptr->scene->d_func()->dirtyGrowingItemsBoundingRect = true;
        d_ptr->geometryChanged = 1;
        d_ptr->paintedViewBoundingRectsNeedRepaint = 1;
        d_ptr->notifyBoundingRectChanged = !d_ptr->inSetPosHelper;

        QGraphicsScenePrivate *scenePrivate = d_ptr->scene->d_func();
        scenePrivate->index->prepareBoundingRectChange(this);
        scenePrivate->markDirty(this, QRectF(), /*invalidateChildren=*/true, /*force=*/false,
                                /*ignoreOpacity=*/ false, /*removingItemFromScene=*/ false,
                                /*updateBoundingRect=*/true);

        // For compatibility reasons, we have to update the item's old geometry
        // if someone is connected to the changed signal or the scene has no views.
        // Note that this has to be done *after* markDirty to ensure that
        // _q_processDirtyItems is called before _q_emitUpdated.
        if (scenePrivate->isSignalConnected(scenePrivate->changedSignalIndex)
            || scenePrivate->views.isEmpty()) {
            if (d_ptr->hasTranslateOnlySceneTransform()) {
                d_ptr->scene->update(boundingRect().translated(d_ptr->sceneTransform.dx(),
                                                               d_ptr->sceneTransform.dy()));
            } else {
                d_ptr->scene->update(d_ptr->sceneTransform.mapRect(boundingRect()));
            }
        }
    }

    d_ptr->markParentDirty(/*updateBoundingRect=*/true);
}

To copy to clipboard, switch view to plain text mode 

As you see it does something more than just calling update().

It was the reason I've called update() at the end of my adjust() method, to be sure that it will be called ever - maybe not good for performance ...

That's ok but it's not sufficient when your item changes its size.

Does update() need any other methods called before itsefl in order to GV realy update item on scene?

Please understand, when boundingRect() changes, it is not enough to update the area that used to be covered by the previous boundingRect(). You have to update both the old boundingRect() and the new one, right? So you need to tell GV about such situation so that it can rebuild its internal index and update both areas. Otherwise you'll have artifacts on the view.

[Piskvorkar]

Ok, I think I'm begining to understand :-). Believe that I've realy looked at the prepareGeometryChange code, but I have to confess I was realy lazy to try to apprehend it. It's my unluckiness, that my defective code worked properly under Qt 4.5.3, but now I think it should be ok. Realy thanks much for clarification.
I suppose that what prepareGeometryChange does (among others) is that it set some flags which are affected in next event loop so boundingRect and update and any others methods needed to completly renewing are called.