Calling QGraphicsItem::update() does not paint

SuperCiuper

Hi,

I wanted to make my QGraphicsItem(s) paint only when it update() is called (from class that is observes). To do that I tried a lot of different approaches and ultimately failed but I can't understand how this is possible.
Class diagram is something like this:

Idea is that some external container will trigger MovingObject::update() -> view::PointPainter()::paint() -> this.update()
Then scene should be informed of update() request and event machine should handle QEvent::Paint. Why it does not work tho? Prints to console after 4 times calling MovingObject::update() on my object, as you can see there is not actual painting:

 junction paint 
 PointPainter paint 
 junction paint 
 PointPainter paint 
 junction paint 
 PointPainter paint 
 junction paint 
 PointPainter paint

Here is my github, it's WIP but to trigger this you can just click "Add Road" button, it should repaint Junction (inherits MovingObject): https://github.com/SuperCiuper/TrafficSimulation/commit/7210f546966ca4e010b7a109270186796d92cb74

Here is some code (only important parts):

MovingObject:

namespace trafficsimulation::model
{

class MovingObject
{
public:
    virtual ~MovingObject();
    
    void setPainter(std::unique_ptr<interface::PointPainter> painter);
    void update();

protected:
    MovingObject();

private:
    std::unique_ptr<interface::PointPainter> painter_;
};

MovingObject::MovingObject()
    : painter_{nullptr}
{
}

MovingObject::~MovingObject() = default;

void MovingObject::setPainter(std::unique_ptr<interface::PointPainter> painter)
{
    painter_ = std::move(painter);
}

void MovingObject::update()
{
    if(painter_ == nullptr)
    {
        return;
    }
    painter_->paint();
}

} // trafficsimulation::model

interface::PointPainter:

namespace trafficsimulation::interface
{

class PointPainter
{
public:
    virtual ~PointPainter() = default;

    virtual void paint() = 0;

protected:
    PointPainter() = default;
};

} // trafficsimulation::interface

view::PointPainter:

namespace trafficsimulation::view
{

class PointPainter : public interface::PointPainter, public QGraphicsItem
{
public:
    virtual ~PointPainter();

    void paint() override;

    QRectF boundingRect() const override;

protected:
    PointPainter();

    common::Point point_;
    bool highlight_;
};

PointPainter::PointPainter()
    : point_{common::Point{-10, -10}}
    , highlight_{false}
{
}

PointPainter::~PointPainter() = default;

void PointPainter::paint()
{
    std::cout << " PointPainter paint " << std::endl;
    update();
}

QRectF PointPainter::boundingRect() const
{
    return QRect(0, 0, 1300, 820);
}

} // trafficsimulation::view

interface::JunctionPainter:

namespace trafficsimulation::view
{
constexpr auto JUNCTIONDIAMETER = uint32_t{36};

class JunctionPainter : public PointPainter
{
public:
    JunctionPainter();
    ~JunctionPainter();

    void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget) override;
};

JunctionPainter::JunctionPainter()
    : PointPainter{}
{
}

JunctionPainter::~JunctionPainter() = default;

void JunctionPainter::paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget)
{
    std::cout << " JunctionPainter paint " << std::endl;

    painter->setPen(QPen{Qt::blue, 2, Qt::SolidLine});
    painter->setBrush(QBrush{Qt::green});
    painter->drawEllipse(point_.x - JUNCTIONDIAMETER/2, point_.y - JUNCTIONDIAMETER/2,
        JUNCTIONDIAMETER, JUNCTIONDIAMETER);
}

} // trafficsimulation::view

MainWindow:

namespace trafficsimulation
{

constexpr auto SCENEWIDTH = uint32_t{1300};
constexpr auto SCENEHEIGHT = uint32_t{820};

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow{parent}
    , ui_{new Ui::MainWindow}
    , scene_{new QGraphicsScene(this)}
{
    ui_->setupUi(this);
    ui_->graphicsView->setScene(scene_);
    ui_->graphicsView->installEventFilter(new GraphicsViewFilter{});

    scene_->setSceneRect(0, 0, SCENEWIDTH, SCENEHEIGHT);
    scene_->installEventFilter(this);
}

} // trafficsimulation

SuperCiuper

Small update, use this git commit https://github.com/SuperCiuper/TrafficSimulation/commit/3f1de667b536f0798c7eb0f5cc2f7c3cf1a71887

Also I forgot to mention that it works if I pass QGraphicsScene* scene and in view::PointPainter::paint() use scene->update() it works but repaints all scene which I do not want but will do as Qt should optimize it in some way when more objects will start calling for scene->update()

wrosecrans

@SuperCiuper In general, redrawing the whole scene is the expected behavior here. What problem are you actually trying to solve?

If there are many quick calls to update(), they will generally be batched into one actual paintEvent.

SuperCiuper

@wrosecrans I don't want to pass pointer to the scene to QGraphicsItem. From QGraphicsItem 6.5 documentation:

void QGraphicsItem::update(const QRectF &rect = QRectF())
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.
This function does not cause an immediate paint; instead it schedules a paint request that is processed by QGraphicsView after control reaches the event loop. The item will only be redrawn if it is visible in any associated view.
As a side effect of the item being repainted, other items that overlap the area rect may also be repainted.

Even more so that it's impossible to pass actual scene to QGraphicsItem, my class that inherits (view::PointPainter) has to keep that pointer. Why is that?

Asperamanca

@SuperCiuper
Why do you want to do it this way? What aspect of the default repainting behavior of QGraphicsScene doesn't work well for you?

SuperCiuper

@Asperamanca I'm just asking why put a function that should cause repaint of an object when it does not work? Why keep QGraphicsItem::update(const QRectF &rect = QRectF()) and not marking it in any shape or form that it should not be called from QGraphicsItem? Why is documentation incorrect/misleading then?

I'll repeat myself, for me there is not big difference in calling scene()->update() in PointPainter::paint(). I just want to know why it is that way: bug or feature?

Asperamanca

@SuperCiuper update just schedules a repaint. And it may optimize the repaint away, e.g. if the item is outside of any visible area.
Did you run the event loop after calling update four times? Have you made sure your item is visible, all it's parent items are visible and the item is actually located in an area covered by at least QGraphicsView?

JonB

@SuperCiuper
Nobody has said that method "does not work". Nobody has said it should not be called "from QGraphicsItem". It's a member method, so you have to have an instance of a QGraphicsItem to call it on, whether that be from the outside world or from within a QGraphicsItem subclass. I don't see anything "misleading" in its documentation, it does exactly what it says. Namely, it marks the rectangle of the QGraphicsItem, or a rectangle you choose to specify which might be smaller or larger, as "dirty" and in need of repaint. When the event loop is next reached Qt will assess all the areas which are dirty and decide how best to redraw them all.

Even more so that it's impossible to pass actual scene to QGraphicsItem

Don't know what this means. I don't see why you need to pass the scene to call this function, and if you want to access that there is always QGraphicsScene *QGraphicsItem::scene() const anyway.

SuperCiuper

@Asperamanca Item is visible, it's parent is visible, just before those prints it was repainted because WindowActivate event triggered a repaint. The object is literally in the middle of my application so and in the middle of QGraphicsView and QGraphicsScene

@JonB Yes my bad, I did not found *QGraphicsScene QGraphicsItem::scene() const but it's only part of my question.
As you said:

Nobody has said that method "does not work". Nobody has said it should not be called "from QGraphicsItem".

I said that because it does not repaint my item. PointPainter uses exactly the same QRectf as scene so it should match but it does not. I don't know how event loop cannot be reach as application is still responsive, I can e.g. open modal using my other buttons, also I do nothing with event loop when calling scene()->update() yet this one works just fine. Please explain to me why do you think it should work because as of now it does not

JonB

@SuperCiuper said in Calling QGraphicsItem::update() does not paint:

because as of now it does not

It does. If you want to produce a minimal example where you say it does not we will look at it.

BTW, the job of QGraphicsItem::paint(), direct or overridden, is to paint the item into the painter, like your JunctionPainter::paint() appears to do. I'm not sure what your void PointPainter::paint() which just calls update(); is supposed to achieve/behave like.

SuperCiuper

@JonB Minimal project as you requested github
I have to say that this works :D But then I managed to make it not work again, ViewportUpdateMode set to NoViewportUpdate

QGraphicsView will never update its viewport when the scene changes; the user is expected to control all updates. This mode disables all (potentially slow) item visibility testing in QGraphicsView, and is suitable for scenes that either require a fixed frame rate, or where the viewport is otherwise updated externally.

That surprised me because descriptions says that the user is expected to control all updates, so I thought that calling a QGraphicsItem::update() will propagate this to a scene() but I guess not. Well this thread can be closed then, hope someone in the future will not waste 2 days trying to fix it