-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"critical error" on startup with software gl renderer #6966
Comments
Commented by: daschuer Hi Ewan, Does Mixxx crash? It looks like Mixxx is configured for experimental GLSL Waveforms. Please change your waveform settings in .mixxx/mixxx.cfg to [Waveform] and report the original value here. This issue should be fixed since Bug #981218. |
Commented by: ewanuno yes , mixxx crashes. |
Commented by: ewanuno i have tried with WaveformType 0 but no change. still the exact same error. since you ask my mixxx.cfg was like this: [Waveform] |
Commented by: ewanuno i have tried with WaveformType 0 but no change. still the exact same error. since you ask my mixxx.cfg was like this: [Waveform] On 26 March 2013 16:33, Daniel Schürmann wrote:
|
Commented by: kain88-de please set the WaveformType to 0. It is still at 6 in your mixxx.cfg If this still crashes could you also create a backtrace as described http://www.mixxx.org/wiki/doku.php/creating_backtraces I'm not sure if this is a bug in nouveau though. This driver should
|
Commented by: daschuer The error messages are initiated from QT itself. I can hit the function with gdb when selecting GLWaveform = WaveformType 6. But not with EmptyWaveform = WaveformType 0 So if Mixxx still crashing with WaveformType 0 please provide the log and a backtrack as requested by Max. |
Commented by: ewanuno sorry for the confusion, i really did set it to WaveformType 0 the config was for daniel who requested the previous config. i should be more specific, it dosent segfault, it freezes while showing me an empty dialog box with "critical error' in its title. here is a back trace:
|
Commented by: kain88-de In your config set show_spinny to zero at every occurence , their should be 2 The main problem here is in noveau (google give a lot of error with your card), but mixxx should also fail more gracefully when we the spinny widgets fail |
Commented by: daschuer Ok, I think I have narrowed it down. The deadlock is, is caused by Mixxx itself, because it tries to display the "Critical Error" Message Box from the Shinny Paint event. We exit Mixxx in any case on after closing the critical error message so the deadlock problem is a minor one. So I will try Max's idea to check http://qt-project.org/doc/qt-4.8/qglwidget.html#isValid before painting. |
Commented by: rryan Nice work! We've had an issue like this before related to the waveforms in On Thu, Apr 4, 2013 at 9:01 AM, Daniel Schürmann <
|
Commented by: rryan Found it -- Bug #517373. This was because our warning handler would show a I agree we should fix that deadlock but like you said it won't actually fix On Thu, Apr 4, 2013 at 10:25 AM, RJ Ryan wrote:
|
Commented by: daschuer The attached patch checks if Wspinny is valid, if not an empty group widget is shown. Please search you mixxx.log for I would like to commit this patch in any case to 1.11 even if it does not fix the problem, because checking if WSinny is valid is a good idea anyhow. OK? |
Commented by: rryan (again, ignoring the deadlock in the messagebox code -- we should still fix that) Hi guys -- I think there really isn't a problem with the invalid QGLWidget here. Even if the QGLWidget is invalid it won't cause problems for us, the paint commands should just fail. The problem is that the Qt source code has qCritical() calls in it for debug output like failing to compile a shader. Qt doesn't expect that a qCritical will result in an exit or abort. This is something we do in Mixxx from src/errordialoghandler.cpp. I propose that instead we remove exit/abort on qCritical from Mixxx and instead make every part of Mixxx that uses qCritical also exit explicitly if it wants to. Currently there are only 3 places that this happens:
I think case #3 should actually be removed in favor of a script console where developers can see the errors in their scripts. If we don't remove the abort/exit on qCritical then some other Qt code that uses qCritical will cause us to crash in the future. |
Commented by: kain88-de did you have something like this in mind? |
Commented by: rryan Actually something a little more involved like this: |
Commented by: kain88-de rryan - LGTM Daniel - I think it would be better to display some text like 'video driver does not support openGL' when the dummy is shown instead of nothing. Otherwise people will think this feature is broken |
Commented by: daschuer We have two aspects:
I have just read: So Qt thinks: I would prefer we do the same: change our MIxxx reportCriticalErrorAndQuit to qFatal. The other problem to me is a major one: With the new ideas issued in the patch, we will not have such a fine bug reports like this in the future. We will only have users reporting some malfunction and then we have to look in the Mixxx log for a critical error but there is no way for the user to make a back trace.
|
Commented by: rryan I committed a fix for the deadlock -- it's a pretty simple one: |
Commented by: rryan I don't think the QGLWidget qCritical would actually cause a crash if it weren't for Mixxx deadlocking or quitting because someone issued a qCritical log. So there's really no crash to fix, just bad behavior on Mixxx's part. (We should confirm this with Ewan once we turn off quitting on qCritical). |
Commented by: rryan I looked at the 3 places we call qFatal in Mixxx and I don't think any of them deserve that we quit -- we could instead handle the error gracefully. RE: Debugging I think the only reason we got a nice backtrace from Ewan was because Mixxx deadlocked. Normally on a qCritical/qFatal we would only see a backtrace leading up to the line the debug was issued from. We already know the line the debug was issued from because we can look where the error string occurs in the codebase. So the information the backtrace can tell us is:
Since the qCritical will still issue an error dialog the user can use GDB to break and get a backtrace when the error dialog is issued. Since the error dialog is modal it preserves the call-stack that led up to the error dialog being displayed. I think it still allows the user to get us the information we need we just need to tell them how to break GDB using Control-C instead of waiting for a crash to occur. |
Commented by: ewanuno ok here a new backtrace using the latest 1.11 bzr and the last few log messages, now i get two critical error dialogs the frst says: 'error linking simple shader "" ' and the seccond says 'error linking blit shader ''' ' when i press ok in one of these dialogs mixx segfaults.
Program received signal SIGSEGV, Segmentation fault.
|
Commented by: rryan Hey Ewan -- this isn't fixed yet so that crash is expected. The problem here is that when Qt writes a qCritical() message they never intended that that could stall execution inside the paint event and go for another round trip through the event loop (via ErrorDialogHandler and a modal QMessageBox) which in this case makes WSpinny's paintEvent re-entrant and multiple painters become active on the spinny. |
Commented by: rryan (it's super useful to have the backtrace to confirm though -- thanks!) |
Commented by: rryan Oh Daniel -- I may have misunderstood you re: debuggability. It's true that qCritical's that Qt issues would no longer result in a message box so we would lose the ability for users to easily get us backtraces when Qt does that. |
Commented by: daschuer
@ewan: please give it a try. If you still have the recursion, please provide the complete mixxx.log file, or at lest the few lines arround WSpinny(). |
Commented by: rryan I added my patch from #15 in lp:mixxx/1.11 r3802. Daniel -- I don't think printing an error message in the skin is the right approach. In past Mixxx versions we have warned the user via a popup message box when their system doesn't support OpenGL and wouldn't have visuals when they run Mixxx. I think that's a better way. When you insert it into the skin it might be invisible due to style sheets. The skinner has to account for the QLabel and style it via the style sheet. Also, the font sizes could mean the text is either too hard to read or too large. The artificial newline you put in the string might work for the skins we have but maybe wouldn't look right on a much larger spinny. (Also it's a new string that won't be translated in time for 1.11.0...) I think a more skin-integrated solution (for 1.12.x) would be for the spinny to have an inactive or disabled pixmap. If OpenGL was not present, we could swap it out for a static QPixmap of the inactive/disabled pixmap. In addition to this, a warning dialog about missing OpenGL support could warn the user in text that some features like waveforms and spinnies would be missing. |
Commented by: daschuer Hi RJ, sorry for committing without a final go.
Now, I would wait for feedback from Ewan. But feel free to revert my commit. This is a corner case anyway, which is issued due to a bad driver setup. Kind regards, Daniel |
Commented by: kain88-de I also like the idea of replacing the wspinny pixmap with a error text/pixmap instead of just a popup message. by the way what is done with the waveforms when qt realises that openGL is not an option, do we fall back to the software renderer? |
Commented by: ewanuno it's fixed! great work everybody. the only waveforms that work for me are filtered-software and hsv (which is no spinnys obviously. thanks! On 6 April 2013 14:49, Max Linke wrote:
|
Commented by: daschuer Hi Ewan, thank you for your feedback! To answer Max's question from #28. Yes, we fall back to Software waveform in case of openGL absence. Kind regards, Daniel |
Commented by: rryan I agree it's not worth arguing about a corner case like this. Thanks for I'm very curious whether an actual crash would occur if we didn't delete On Sun, Apr 7, 2013 at 8:06 AM, Daniel Schürmann <
|
Commented by: ewanuno sure, a patch would be fine too. On 8 April 2013 01:08, RJ Ryan wrote:
|
Commented by: rryan Great -- thanks Ewan. Please apply this patch and let us know what happens / post a backtrace if you get a crash. |
Commented by: ewanuno it works fine with the patch applied. i just noticed though that with and without the patch i get a continuous
and so on until i quit mixxx. On 9 April 2013 05:18, RJ Ryan wrote:
|
Commented by: rryan Good to know -- glad it doesn't crash but it's probably good that we delete Maybe we should make spinny's work more like the waveforms that have a On Tue, Apr 9, 2013 at 11:58 AM, ewan colsell wrote:
|
Commented by: rryan Marking fixed -- thanks Daniel! |
Issue closed with status Fix Released. |
Reported by: ewanuno
Date: 2013-03-26T13:05:21Z
Status: Fix Released
Importance: High
Launchpad Issue: lp1160353
Tags: 1.11
Attachments: spinny_check.patch, error.patch, not_critical.patch, no_invalid_spinny_check.patch
mixxx 1.11 revision 3779 from bzr
i am running mixxx on avlinux 6.0(based on debian squeeze)
using an nvidia geforce fx 5200 (old card) using the nouveau gpl driver, (software renderer)
since i am using the software renderer, i suspect that this is not specific to my graphics card.
i have a laptop with an identical software configuration that does not suffer from this problem which uses an intel graphics card.
i DONT GET THIS PROBLEM WITH THE CLOSED SOURCE NVIDIA DRIVERS,
on startup i get a dialog box with "critical error" in its title but no text or buttons.
The text was updated successfully, but these errors were encountered: