Minimize window = images gone

[sooofunky]

Hello,

I am using the paintEvent method to show images on a QFrame. When starting the program, the images are shown correctly most of the times (sometimes they do not appear or in the wrong places). When I minimize the window or it gets overlapped by another, all images disappear. Any ideas what the problem might be?

Thanks!

[wysota]

The problem might be with your code.

[sooofunky]

Thanks for replying. In WeatherFrame.cpp I am doing the drawing - not sure what is wrong.

#include "WeatherFrame.h"
 
WeatherFrame::WeatherFrame(QWidget *parent)
    : QFrame(parent)
{
             mEngine = new GoogleEngine();
	connect(mEngine, SIGNAL(weatherReady()), this, SLOT(handleData()));
	connect(mEngine, SIGNAL(weatherError()), this, SLOT(handleError()));
 
	mEngine->location().city() = "Salzburg";
	mEngine->update("Salzburg");
}
 
WeatherFrame::~WeatherFrame()
{
	delete mEngine;
}
 
void WeatherFrame::handleData()
{
	mCurrentLocation = mEngine->location();
	mCurrentWeather = mEngine->weather();
	mListForecast = mEngine->forecast();
             mReady = true;
	update();
}
 
void WeatherFrame::handleError()
{
	qDebug() << "handleerror";
}
 
void WeatherFrame::paintEvent(QPaintEvent* event)
{
    if (mReady) {
        Q_UNUSED(event);
        QPainter painter(this);
        painter.setRenderHints(QPainter::HighQualityAntialiasing |     QPainter::SmoothPixmapTransform);
 
        // draw current temperature
        painter.setPen(Qt::black);
        painter.setFont(QFont("Arial", 16));
        painter.drawText(QRect(5,2,100,30), Qt::AlignLeft, "Wetter  -");
 
        // draw location's city name
        painter.setFont(QFont("Arial", 16));
        painter.drawText(QRect(100,2,100,30), Qt::AlignLeft, mCurrentLocation.city());
 
        // draw condition pixmap
        painter.drawPixmap(QRect(10,55,50,50),mCurrentWeather.pixmap());
 
        painter.setFont(QFont("Arial", 12));
        painter.drawText(QRect(20,25,60,40), Qt::AlignCenter, "HEUTE");
 
        painter.setFont(QFont("Arial", 14));
        painter.drawText(QRect(20,105,40,40), Qt::AlignCenter,
					 QString::number(FahrenheitToGrad(mCurrentWeather.temperature())));
 
        for (int i = 0; i < mListForecast.count(); i++) {
            Weather weather = mListForecast.at(i);
 
            // draw forecast day
            painter.setFont(QFont("Arial", 12));
            painter.drawText(QRect((i+1)*100+20,25,40,40), Qt::AlignCenter, weather.day().toUpper());
 
            // draw condition pixmap
            painter.drawPixmap(QRect((i+1)*100+20,55,50,50), weather.pixmap());
 
            // draw forecast low and high temperature
            painter.setFont(QFont("Arial", 10));
            painter.drawText(QRect((i+1)*100+20, 95,40,40), Qt::AlignCenter, QString::number(weather.lowTemp()));
            painter.drawText(QRect((i+1)*100+20, 110,40,40), Qt::AlignCenter, QString::number(weather.highTemp()));
 
        }
 
	mReady = false;
    } else {
	QWidget::paintEvent(event);
    }
}
 
int WeatherFrame::FahrenheitToGrad(int f)
{
	int c = (f-32) / 1.8;
	return c;
}

#include "WeatherFrame.h"

WeatherFrame::WeatherFrame(QWidget *parent)
    : QFrame(parent)
{
             mEngine = new GoogleEngine();
	connect(mEngine, SIGNAL(weatherReady()), this, SLOT(handleData()));
	connect(mEngine, SIGNAL(weatherError()), this, SLOT(handleError()));

	mEngine->location().city() = "Salzburg";
	mEngine->update("Salzburg");
}

WeatherFrame::~WeatherFrame()
{
	delete mEngine;
}

void WeatherFrame::handleData()
{
	mCurrentLocation = mEngine->location();
	mCurrentWeather = mEngine->weather();
	mListForecast = mEngine->forecast();
             mReady = true;
	update();
}

void WeatherFrame::handleError()
{
	qDebug() << "handleerror";
}

void WeatherFrame::paintEvent(QPaintEvent* event)
{
    if (mReady) {
        Q_UNUSED(event);
        QPainter painter(this);
        painter.setRenderHints(QPainter::HighQualityAntialiasing |     QPainter::SmoothPixmapTransform);

        // draw current temperature
        painter.setPen(Qt::black);
        painter.setFont(QFont("Arial", 16));
        painter.drawText(QRect(5,2,100,30), Qt::AlignLeft, "Wetter  -");

        // draw location's city name
        painter.setFont(QFont("Arial", 16));
        painter.drawText(QRect(100,2,100,30), Qt::AlignLeft, mCurrentLocation.city());

        // draw condition pixmap
        painter.drawPixmap(QRect(10,55,50,50),mCurrentWeather.pixmap());

        painter.setFont(QFont("Arial", 12));
        painter.drawText(QRect(20,25,60,40), Qt::AlignCenter, "HEUTE");

        painter.setFont(QFont("Arial", 14));
        painter.drawText(QRect(20,105,40,40), Qt::AlignCenter,
					 QString::number(FahrenheitToGrad(mCurrentWeather.temperature())));

        for (int i = 0; i < mListForecast.count(); i++) {
            Weather weather = mListForecast.at(i);

            // draw forecast day
            painter.setFont(QFont("Arial", 12));
            painter.drawText(QRect((i+1)*100+20,25,40,40), Qt::AlignCenter, weather.day().toUpper());

            // draw condition pixmap
            painter.drawPixmap(QRect((i+1)*100+20,55,50,50), weather.pixmap());

            // draw forecast low and high temperature
            painter.setFont(QFont("Arial", 10));
            painter.drawText(QRect((i+1)*100+20, 95,40,40), Qt::AlignCenter, QString::number(weather.lowTemp()));
            painter.drawText(QRect((i+1)*100+20, 110,40,40), Qt::AlignCenter, QString::number(weather.highTemp()));

        }

	mReady = false;
    } else {
	QWidget::paintEvent(event);
    }
}

int WeatherFrame::FahrenheitToGrad(int f)
{
	int c = (f-32) / 1.8;
	return c;
}

To copy to clipboard, switch view to plain text mode 

[pan]

Put a break point in your paint event. Check the value of mReady. Or put in some logging.

You will notice how often it repaints and how often your mReady is false. This should help you see why it is not repainted after you have minimized.

[sooofunky]

ouch, somehow I thought it must be a much more severe problem... Thanks!

[Rachol]

It is quite obvious to me what is happenig. You are drawin the widget only once when the data is available and then you draw nothing. However you must draw something when window is being restored.

Much better solution would be do have some QPixmap m_weatherPixmap declared as your private member variable. Then also declare something like updatePixmap, where you draw your weather info into a pixmap;

In handleData you should clear the pixmap: m_weatherPixmap = QPixmap();

in paintEvent:

if(m_weatherPixmap.isEmpty())
{
    if(mReady)
    {
        updatePixmap(m_weatherPixmap);
    }
}
 
QPainter p(this);
p->drawPixmap(0,0,m_weatherPixmap);

if(m_weatherPixmap.isEmpty())
{
    if(mReady)
    {
        updatePixmap(m_weatherPixmap);
    }
}

QPainter p(this);
p->drawPixmap(0,0,m_weatherPixmap);

To copy to clipboard, switch view to plain text mode 

Hope this helps.

[wysota]

Changing any variable value in paintEvent is a sure misdesign.

[Rachol]

Not really, I have never heard of anyone saying that before, in many situations that is the only way you can do something, especially when performance comes into the game.

[wysota]

Example please.

[Rachol]

Example:

You have a QWidget that draws itself based on some multiple properties. This code will be run on a very slow machine. Drawing the widget from the beginning every time is slower than using stored pixmap. Now, if you don't want this pixmap to be redrawn every time you change a property, what do you do?

[nish]

Example:

You have a QWidget that draws itself based on some multiple properties. This code will be run on a very slow machine. Drawing the widget from the beginning every time is slower than using stored pixmap. Now, if you don't want this pixmap to be redrawn every time you change a property, what do you do?

Sorry to pop in, Your sentence is little bit confusing to me. Can you give some sample code and we will see if we can improve upon it?

[Rachol]

I don't see anything confusing in there, it is quite simple. Where is the confusing part? And why should I get a code for imaginary case? I just try to prove that saying that "changing a variable in paintEvent is a misdesign" is totally wrong.

[wysota]

You have a QWidget that draws itself based on some multiple properties. This code will be run on a very slow machine. Drawing the widget from the beginning every time is slower than using stored pixmap. Now, if you don't want this pixmap to be redrawn every time you change a property, what do you do?

Then you render the widget to the pixmap before the paint event is invoked and then only render the pixmap in the event. But I'm not sure why you wouldn't want it the widget to be redrawn when its contents change...

[Rachol]

Because sometimes you can change couple properties and you don't want the pixmap to be rendered twice, or more times before the paintEvent comes.

[wysota]

Why would the pixmap be rendered twice? You can delay your pixmap generating code the same way as paint event is delayed. I agree you can generate the pixmap in the paint event but there are alternatives that have their advantages. Like doing the rendering in an external thread.

[Rachol]

Thanks for agreeing that indeed changing variables in the paintEvent ain't misdesign. Delaying... can't be as optimized as drawing in paintEvent, cause paintEvent is called only and only when necessary. You could of course do it so it is done just before paintEvent is called, but then it is quite much the same thing, but more complicated and unnecessary work is done. Then rendering in a separated thread could be done, but then again it depends on the case. I used to work with a embedded device that couldn't handle well multithreading applications..., so I agree that there are many options it just to choose the option that fits best your case.

[wysota]

Thanks for agreeing that indeed changing variables in the paintEvent ain't misdesign.

I said nothing like that. I said you could do it in paintEvent but I didn't say it's a good design. Unless of course you like having frozen GUI. Paint event should be executed as fast as possible and all unnecessary code should be stripped out of the event handler.

Delaying... can't be as optimized as drawing in paintEvent, cause paintEvent is called only and only when necessary.

I don't see your point. The amount of work to be done is the same -- you need to render to a pixmap and then render the pixmap to the widget. It's the decision when to do it that makes a difference. There is nothing "optimal" in drawing to the pixmap in paint event. If you change a property that affects the looks of the widget then eventually you'll want to display those changes so every update of the pixmap involves triggering a repaint. And every repaint doesn't have to check and update the pixmap because if there was something to update, pixmap updating code would have triggered a repaint.

You could of course do it so it is done just before paintEvent is called, but then it is quite much the same thing

No, it's not the same thing. Especially if your point is that you're working with a slow machine. In that case you want to save every cycle you can.

Then rendering in a separated thread could be done, but then again it depends on the case. I used to work with a embedded device that couldn't handle well multithreading applications...

That's not an argument. If you have a device that's not fit to be used with some solution then you don't use that solution. Extreme cases sometimes require extreme measures. However it doesn't make those measures good solutions. And it doesn't make a good design bad just because there exists a device that involves some special treatment.

I've seen many people do such crazy flag resetting in paint events. It's caused by a false assumption that the painting code will be called only in conditions they specify which is not the case because the code can be called an arbitrary number of times per second. Updating pixmaps in paint events is also such a false assumption. If you start updating pixmaps at this pace in every paint event (assuming some changes indeed trigger those repaints), you'll be creating pixmaps your user will never even have a chance to see simply because the eye is not able to spot differences at such pace. It's enough to update the pixmap one-two times a second (depending on the nature of the data you are displaying) even if you happen to miss 1000 repaints that will render a stale pixmap. Eventually you'll render the right pixmap and it doesn't matter if it happens at repaint 1000 or 1001. What matters is that you'll render the pixmap once which, as you said yourself, is an expensive operation. You don't have such abilities if you update the thread in paint event --- you'll have to do it in every paint event.

[Rachol]

Unless of course you like having frozen GUI

Most ridiculous thing I have read in this thread. So you mean doing heavy operation anywhere else in one threaded app will not freeze the GUI?

If you change a property that affects the looks of the widget then eventually you'll want to display those changes so every update of the pixmap involves triggering a repaint. And every repaint doesn't have to check and update the pixmap because if there was something to update, pixmap updating code would have triggered a repaint.

Again wrong. So in case where 10 of properties change, you will update the pixmap 10 times? Yeah... that sounds like a solution...

That's not an argument. If you have a device that's not fit to be used with some solution then you don't use that solution.

Isn't that exactly the same thing what I have written?

You don't have such abilities if you update the thread in paint event --- you'll have to do it in every paint event.

Do you actually follow what I am writing? Please take a look at the code I have posted already in this thread. Short version:

if(m_weatherPixmap.isEmpty())
{
        updatePixmap(m_weatherPixmap);   
}

if(m_weatherPixmap.isEmpty())
{
        updatePixmap(m_weatherPixmap);   
}

To copy to clipboard, switch view to plain text mode 

I do only an update when it is needed. I am updating something in every paintEvent?

Final word:

I see this conversion becoming unpleasant and I start seeing no point in going with it any forward. We can point out our imaginary situations and keep fighting over stuff, when in most cases we are both right. One thing for me is clear in here with all the respect, you are wrong and you can't say it is a misdesign to set a member variable in paintEvent. If it was so, then I bet that paintEvent would be declared as const method and it isn't. sizeHint() for example is a const method.

And now grande finale:

Take a look at http://doc.qt.nokia.com/latest/paint...gview-cpp.html and its implementation of paintEvent.

[wysota]

Most ridiculous thing I have read in this thread. So you mean doing heavy operation anywhere else in one threaded app will not freeze the GUI?

If it is not trying to redraw at this point then no.

Again wrong. So in case where 10 of properties change, you will update the pixmap 10 times? Yeah... that sounds like a solution...

No, I will update it at most once. Which part of the sentence I have written suggests that each property change will cause a separate update of the pixmap?

Isn't that exactly the same thing what I have written?

Please don't cut out my thoughts in half. A special case doesn't change a bad solution into a good one (we usually call them "workarounds" then).

Do you actually follow what I am writing? Please take a look at the code I have posted already in this thread. Short version:

if(m_weatherPixmap.isEmpty())
{
        updatePixmap(m_weatherPixmap);   
}

if(m_weatherPixmap.isEmpty())
{
        updatePixmap(m_weatherPixmap);   
}

To copy to clipboard, switch view to plain text mode 

I do only an update when it is needed. I am updating something in every paintEvent?

Consider the following logic. Suppose you have a property "prop" that influences the way a widget looks.

void Cls::setProp(... val) {
  if(m_prop == val) return;
  m_prop = val;
  update();
}
// ...
QTimer *t = new QTimer;
connect(t, SIGNAL(timeout()), SLOT(doSomething()));
t->start(10); // 100Hz
// ...
void Cls::doSomething() {
  clsObj->setProp(...);
}

void Cls::setProp(... val) {
  if(m_prop == val) return;
  m_prop = val;
  update();
}
// ...
QTimer *t = new QTimer;
connect(t, SIGNAL(timeout()), SLOT(doSomething()));
t->start(10); // 100Hz
// ...
void Cls::doSomething() {
  clsObj->setProp(...);
}

To copy to clipboard, switch view to plain text mode 

Your pixmap will be updated 100 times a second, the sequence of events will be timeout, paint, timeout, paint, timeout, paint. I take it that you are a super human and will notice all those changes. Most people won't. A much better implementation will schedule a pixmap rebuild instead of calling update() in setProp. And the routine will make sure the pixmap is recreated at most X times a second where X is a sufficiently low value. I'm sure your slow device will benefit more from such solution than updating the pixmap at 100Hz.

I see this conversion becoming unpleasant and I start seeing no point in going with it any forward. We can point out our imaginary situations and keep fighting over stuff, when in most cases we are both right. One thing for me is clear in here with all the respect, you are wrong and you can't say it is a misdesign to set a member variable in paintEvent. If it was so, then I bet that paintEvent would be declared as const method and it isn't. sizeHint() for example is a const method.

It can't be const because you are passing a pointer to "this" to QPainter constructor which would break constness. In other words it wouldn't be possible to paint on a widget from within a const method without breaking constness.

And now grande finale:

Take a look at http://doc.qt.nokia.com/latest/paint...gview-cpp.html and its implementation of paintEvent.

So your argument is that all code in Qt is perfectly designed? It's a pitty it is not so Just take a look at the "threaded fortune cookie" example and ask Bradley Hughes what he thinks of it Or look at one of the last Qt blog entries introducing ideas for Qt5 to see how much of Qt4 is considered "flawed design".

My opinion is that you should act as if paintEvent() was const and so far I haven't heard anything that would convince me otherwise. Your pixmap updating idea is of course fine and it's often used (I often use it myself) but it is not necessary to violate "constness" of paintEvent to use such an approach. Not every simple code is good code. Simple solutions are often the best but not in every case. Please don't take it personally, I find this discussion very interesting and useful. It's one of those times when you have to really stretch your skills instead of just pointing people to Qt docs as it is the case with 90% of issues people come here with.

[Rachol]

I like your solution, but it is with some disadvantages:

it just makes me do extra stuff ( handle updates every X ms ->extra timer, connection, method).
If during the update time, m_prop changes twice, coming back to previous value, I miss that... Updating in paintEvent would do the update as soon as possible after update() is called. In such situation it is about changing the update value to something really small, which brings as back to the same thing, updating pixmap rapidly and then we can still miss the change. On the other hand a hidden widget wouldn't have to update itself, but in your solution it still has to do some extra checks and repainting. Going further it forces you not to do unnecessary updates when widget hidden and also show the correct content of the widget when it becomes visible, most probably without a delay. All this introduces a huge amount of things you have to take care of, that you wouldn't even think of if the logic was in paintEvent.