Two different methods of using QBrush with QPainter - why is only one working?

Hi everybody,

I just managed to complete a long untouched work on a widget I want to use in one of my projects. It is a "circular progress bar", simply drawn with QPainter::drawPie().

Attachment 9037

During development, I have learned that I cannot assign a pointer to a QBrush to QPainter::setBrush() and then change some values of the brush to use different painting styles (well, actually I can - but in this case, all the area of the ellipse is painted black instead of only the pie itself). Instead, I have to create an all-new QBrush everytime I want to change the drawing style.

Here is some code to explain the problem:

Code:

---

//[... somewhere in the widget's header ...]
 
    QPen *_foreGround;
    QBrush *_backGround;
 
//[... somewhere in the widget's initialization process ...]
 
    _foreGround = new QPen();
    _foreGround->setCosmetic(true);
    _foreGround->setWidth(3);
    _backGround = new QBrush();
    _backGround->setColor(QColor(qRgba(0,0,0,0)));
 
//[...]
 
void QtProgressPie::paintEvent(QPaintEvent *e)
{
    // recalculate the value according to the conditions of QPainter::drawPie
    // step 1: subtract _minimum from _value (so the calculation base is always 0)
    // step 2: calculate the percentage of value (value/max * 100)
    // step 3: translate it to circular conditions (value/max * [100 * 3.6 * 16]) [...] = _FULLCIRCLE
    // step 4: take it times -1 (QPainter::drawPie works counter-clockwise with positive values)
 
    int _painterValue = ((_value - _minimum) / double(_maximum - _minimum) * _FULLCIRCLE) * -1;
 
    QPainter p;
    p.begin(this);  // Begin drawing on the widget
 
    // make sure that the widget appears always as a circle, not as an ellipse
    int _squaredSize = qMin(this->width(), this->height());
    p.setViewport((this->width() - _squaredSize) / 2,(this->height() - _squaredSize) / 2,
                  _squaredSize, _squaredSize);
    // subtract the width of the pen ("the border width") from the widget's rect() to get the drawable area
    int _borderWidth = _foreGround->width();
    QRect drawableArea(0 + _borderWidth, 0 + _borderWidth,
                       this->width() - _borderWidth * 2, this->height() - _borderWidth * 2);
 
 
    p.setRenderHint(QPainter::HighQualityAntialiasing, true);
    p.setPen(*_foreGround);
    //p.setBrush(*_backGround); <--- DOESN'T WORK
 
    // draw surrounding circle
    // _backGround->setStyle(Qt::NoBrush); <--- DOESN'T WORK
    p.setBrush(QBrush(QColor(qRgba(0,0,0,0)),Qt::NoBrush)); // <--- WORKS!
    p.drawEllipse(drawableArea);
 
 
    // draw pie if value < 0
    if (_painterValue < 0)
    {
        //_backGround->setStyle(Qt::SolidPattern); <--- DOESN'T WORK
        p.setBrush(QBrush(QColor(qRgba(0,0,0,0)),Qt::SolidPattern)); // <--- WORKS!
        p.drawPie(drawableArea, _OFFSET, _painterValue);
        // _OFFSET = 1/4 of _FULLCIRCLE (transition from 3 o'clock (drawPie's base) to 12 o'clock)
    }
 
    p.end();    // Drawing finished

---

So, my question is: why can't I use the "more elegant" solution with the pointer variable?

Greetings,
Markus

EDIT: picture added, futher explanation of what goes wrong when using a pointer.

This line:

Code:

---

//p.setBrush(*_backGround); <--- DOESN'T WORK

---

absolutely does work; that is a fundamental feature of C++ and nothing to do with Qt. Actually, all of the lines you marked work, they just do not do what you expect.

You need to understand that QPainter::setBrush() takes a copy of the brush, and setPen() a copy of the QPen, you pass it. You can change the QBrush pointed to by your pointer after a call to QPainter::setBrush() but this will have no effect on the painter because the QBrush referenced through your pointer is not the same QBrush instance as the one used by QPainter. The same goes for QPen. QBrush and QPen are value objects and are designed to be be efficiently copied, there's really nothing to be gained from heap allocating them.

Quote:

---

Originally Posted by ChrisW67

This line:

Code:

---

//p.setBrush(*_backGround); <--- DOESN'T WORK

---

absolutely does work; that is a fundamental feature of C++ and nothing to do with Qt. Actually, all of the lines you marked work, they just do not do what you expect.

---

Yeah, i know (this is what i tried to explain in my EDIT before ;-)).

Quote:

---

You need to understand that QPainter::setBrush() takes a copy of the brush, and setPen() a copy of the QPen, you pass it. You can change the QBrush pointed to by your pointer after a call to QPainter::setBrush() but this will have no effect on the painter because the QBrush referenced through your pointer is not the same QBrush instance as the one used by QPainter. The same goes for QPen. QBrush and QPen are value objects and are designed to be be efficiently copied, there's really nothing to be gained from heap allocating them.

---

Thanks, that explains it. After reading your answer, I once again checked the Qt manual and recognized the const ref to QBrush... ;-)
So, either I have to recall p.setBrush(*_backGround) after messing around with _backGround or just leave anything as it is right now. However, I think, the first solution doesn't make any sense at all as QBrush is copied anyways - so in fact, keeping a pointer to an extra QBrush is in fact a greater "memory pollution" than just creating a new QBrush on the fly when it's needed, am I correct?

Thanks for your help!
Markus

You can create new QBrush or QPen instances on the fly (as local variables) or you can keep copies as member variables that you adjust before using them in setBrush()/setPen() calls. Your call really.

To add to that -- if you keep the brush as a member variable, then setting the brush on the painter with setBrush() does not increase memory use -- you have two "handles" to the brush but in fact only one instance of brush data. This works as long as you don't modify any of the "handles". If you do then a copy takes place so that each brush has its own instance of data. But since you modify the brush to apply the changes right away, you end up with a single instance of data again as the old data is deleted by QBrush. Same works for QPen, of course.

Thank you both for your valuable answers!

Greetings,
Markus

Hello again!

As I'm advancing with my little widget, i just got another problem, which is also related with widget painting, so i decided to append my new problem right here in order to not flood the forum. :rolleyes:

I have subclassed my ProgressPie widget to create a timer-controlled progress pie. You give it a range and it counts down or up its limit values. I did this by implementing QObject::startTimer() and the timerEvent() method.

I simply wanted to use my setValue() method from the parent widget which already includes a request to redraw itself after a value change:

Code:

---

repaint(this->rect());

---

But when I call the function from the timerEvent() subclassed widged, nothing happens - unless i resize the window, which will trigger the resizeEvent (which is also implemented in the parent's code and which contains just the same line of code).

When i put the line directly into my timerEvent() method of the subclass, the widget will repaint itself properly every time the timerEvent occurs.
So - why's that?

This is the whole code of both functions:

The setValue() method of the parent class:

Code:

---

void QtProgressPie::setValue(const int &val)
{
    // Sets the given value which is checked against its bounds
    if (val != _value)
    {
        _value = qBound(_minimum, val, _maximum);
        repaint(this->rect());
    }
}

---

The timerEvent() of the subclass:

Code:

---

void QtTimedProgressPie::timerEvent(QTimerEvent *e)
{
    // Modifies the current value of the timed ProgressPie.
    // By calling setValue of QtProgressPie, the widget will also be approprietely redrawn. // <--- No, it will not! :-(
    // Also, if the minimum or maximum value is arrived, the timerEvent will kill the timer
    // and emit the finished()-signal.
 
    if (_countBackwards)    // First case: counting towards _minimum
    {
        this->setValue(--_value);   // dec and set value
        if (_value == _minimum)     // check if value is now equal to the minimum
        {                           // if yes, kill timer and emit signal
            _timerIsRunning = false;
            killTimer(e->timerId());
            emit finished();
        }
    }
    else                    // Second case: counting towards _maximum
    {
        this->setValue(++_value);   // inc and set value
        if (_value == _maximum)     // check if value is now equal to the maximum
        {                           // if yes, kill timer and emit signal
            _timerIsRunning = false;
            killTimer(e->timerId());
            emit finished();
        }
    }
    repaint(this->rect()); // <--- this seems to be needed, but why?
}

---

EDIT: typo

First of all use update() instead of repaint(). Second of all you don't have to pass rect() into the call as by default the widget updates its whole rect. Third of all the call that you mark as needed is not needed.

Thanks for your advice. I adapted the code according to your remarks, but I still need the call inside the timerEvent() method, otherwise the widget won't get updated.


EDIT: i finally got it. Thie problem was not the update() call but the way I called the setValue()-method.

I tried to call setValue() with ++_value or --_value. This of course manipulates _value directly before calling the method with the same new _value.

But my setValue() method begins with a check if the given integer parameter is different to the actual _value (because I want to emit a valueChanged() signal from that method). Of course, this will never be true, because of the direct manipulation of _value with ++ and --.

Anyway, thanks for you help!