QMessageBox::warning on secondary thread

[J.Hilk]

@Perdrix said in QMessageBox::warning on secondary thread:

If you could help with that, it would be much appreciated.

depends, how exactly are you invoke that free-standing function in a different thread ?

can you show that code part ?

[J.Hilk]

@Perdrix Actually, std::async doesn't require a free function to invoke.

have you considered creating a temporary parent QObject class to invoke that method ?

would be the easiest I think

[Perdrix]

@J-Hilk

					const auto readTask = [this, pStackingInfo, firstBitmap = m_vBitmaps.cbegin()](const size_t lightTaskNdx, ProgressBase* pProgress) -> std::pair<std::shared_ptr<CMemoryBitmap>, int>
					{
						if (lightTaskNdx >= pStackingInfo->m_pLightTask->m_vBitmaps.size())
							return { {}, -1 };
						const int bitmapNdx = FindBitmapIndex(pStackingInfo->m_pLightTask->m_vBitmaps[lightTaskNdx].filePath.c_str());
						if (bitmapNdx < 0)
							return { {}, -1 };
						const auto& lightframeInfo = m_vBitmaps[bitmapNdx];
						if (lightframeInfo.m_bDisabled)
							return { {}, -1 };

						ZTRACE_RUNTIME("Stack %s", lightframeInfo.filePath.generic_string().c_str());

						std::shared_ptr<CMemoryBitmap> rpBitmap;
						if (::LoadFrame(lightframeInfo.filePath, PICTURETYPE_LIGHTFRAME, pProgress, rpBitmap))
							return { rpBitmap, bitmapNdx };
						else
							return { {}, -1 };
					};

					auto futureForRead = std::async(std::launch::deferred, readTask, 0, m_pProgress); // Load first lightframe synchronously.
					const auto firstBitmap = m_vBitmaps.cbegin();

					for (size_t i = 0; i < pStackingInfo->m_pLightTask->m_vBitmaps.size() && !bStop; ++i)
					{
						auto [pBitmap, bitmapNdx] = futureForRead.get();
						futureForRead = std::async(std::launch::async, readTask, i + 1, nullptr); // Immediately load next lightframe asynchronously (need to set progress pointer to null).

loadFrame() in turn invokes FetchPicture() after doing some stuff.

In FetchPicture() I wrote this:

	if (fs::status(filePath).type() != fs::file_type::regular)
	{
		QString errorMessage{ QCoreApplication::translate(
			"DSS::StackingDlg",
			"%1 does not exist or is not a file").arg(QString::fromStdWString(fileName)) };
#if defined(_CONSOLE)
		std::cerr << errorMessage.toUtf8().constData();
#else
		QMainWindow* mainWindow{ getMainApplicationWindow() };
		ZASSERT(nullptr != mainWindow);
		QMetaObject::invokeMethod(mainWindow,
			SLOT(displayMessageBox(const QString&, QMessageBox::Icon)),
			Q_ARG(QString, errorMessage),
			Q_ARG(QMessageBox::Icon, QMessageBox::Warning));
		
#endif
		return false;
	}

Unfortunately the displayMessageBox() mf of the DeepSkyStacker class never gets driven?

[jsulm]

@Perdrix said in QMessageBox::warning on secondary thread:

displayMessageBox

QMainWindow does not have such a slot, it is your own, right?
I think you need to cast QMainWindow* to your main window class pointer.

[J.Hilk]

@Perdrix ah ok, thats a way to do it :D

since this connect is a cross threads, the arguments need to be copied over. For QString this is trivial and done out of the box for you.

But I think you need to register enum QMessageBox::Icon with the meta system so it knows what it is and can copy it over

https://doc.qt.io/qt-6/qmetatype.html#qRegisterMetaType

Edit:
the Point of @jsulm is also more than valid! You'll need to cast it to your actual class, that has the slot :D

[Christian Ehrlicher]

Another way would be to use a custom QEvent and simply post it to the global event queue. This way you don't need to know who is the receiver. Good for e.g. status messages from anywhere inside your app to show it in a central widget/status bar.

[Perdrix]

@J-Hilk So to register QMessageBox::Icon

Is that as simple as adding this to main()

	//
	// Register the QMessageBox::Icon enum as a meta type
	//
	qRegisterMetaType<QMessageBox::Icon>();

[JonB]

@Christian-Ehrlicher
That is a useful technique, compared to having to create/maintain a QObject. Shame it's not documented somewhere as a useful approach.

[JonB]

@Perdrix
May I mention that you can avoid registration of types if you provided a MainWindow::displayWarningBox() method (which can call displayMessageBox() itself) which did not need a QMessageBox::Icon sent to it. This would allow the caller more of an "abstraction" layer which does not need to know so many Qt/message box details.

[J.Hilk]

@Perdrix AFAIK yes,

maybe you also need
https://doc.qt.io/qt-6/qmetatype.html#Q_DECLARE_METATYPE

I'm never 100% sure with this. The Meta System is always a lot of magic imho :D

[Perdrix]

Final code apart from the registration on QMessageBox::Icon in main().

	if (fs::status(filePath).type() != fs::file_type::regular)
	{
		QString errorMessage{ QCoreApplication::translate(
			"DSS::StackingDlg",
			"%1 does not exist or is not a file").arg(QString::fromStdWString(fileName)) };
#if defined(_CONSOLE)
		std::cerr << errorMessage.toUtf8().constData();
#else
		QMainWindow* mainWindow{ getMainApplicationWindow() };

		DeepSkyStacker* dss = dynamic_cast<DeepSkyStacker*>(mainWindow);
		if (nullptr != dss)
		{
			bool result = QMetaObject::invokeMethod(dss, "displayMessageBox", Qt::QueuedConnection,
				Q_ARG(const QString&, errorMessage),
				Q_ARG(QMessageBox::Icon, QMessageBox::Warning));
		}
		else    // This is here for DeepSkyStackerLive which is not yet Qt 
		{
			AfxMessageBox(errorMessage.toStdWString().c_str(), MB_OK | MB_ICONWARNING);
		}
		
#endif
		return false;
	}

[SimonSchroeder]

Your approach is good (and working). You can still avoid some hassle by using lambdas instead. Here is what we basically do in our software:

QMetaObject::invokeMethod(qApp, [&errorMessage](){ QMessageBox{icon, "DeepSkyStacker", errorMessage}.exec(); });

Please note that your thread will just continue running before the message box might even be shown. Sometimes you might want to wait for a response by the user. For this we need extra synchronization. I've put several use case into a nice little header-only library: https://github.com/SimonSchroeder/QtThreadHelper

You can have a look how you might solve some of the problems.

[Borbixxx]

This post is deleted!