3Dickulus / FragM

Derived from https://github.com/Syntopia/Fragmentarium/
GNU General Public License v3.0
349 stars 30 forks source link

very long shader error logs cause GUI to block #144

Closed claudeha closed 4 years ago

claudeha commented 4 years ago

Describe the bug I have a complicated multi-include frag thing. It's fragile and slow to compile, but when I make a typo it can happen that the shader compiler error log is 100k lines, which prints to the terminal in a couple of seconds but when Qt tries to display each line one by one scrolling to the bottom each time, it takes forever (I gave up and killed it after about 20mins).

Possible solution

Keep the last lines. The 147 is an arbitrary number. The {scope} is to reduce the lifetime of logListAll so it can be freed quickly. Maybe it would actually be better to keep the first lines, because the first errors can trigger a cascade of many errors. But this is what I tested so far:

diff --git a/Fragmentarium-Source/Fragmentarium/GUI/DisplayWidget.cpp b/Fragmentarium-Source/Fragmentarium/GUI/DisplayWidget.cpp
index 13a2f21..6253eb8 100644
--- a/Fragmentarium-Source/Fragmentarium/GUI/DisplayWidget.cpp
+++ b/Fragmentarium-Source/Fragmentarium/GUI/DisplayWidget.cpp
@@ -571,7 +571,16 @@ QStringList DisplayWidget::shaderAsm(bool w)
 void DisplayWidget::createErrorLineLog ( QString message, QString log, LogLevel priority, bool bS )
 {

-    QStringList logList = log.split ( "\n" );
+    QStringList logList;
+    // keep only last lines, in case the log is really long
+    // adding 100k lines to the GUI one by one is really really slow
+    {
+        QStringList logListAll = log.split ( "\n" );
+        for (int i = 0; i < 147 && logListAll.size() > 0; ++i)
+            logList.push_front(logListAll.takeLast());
+        if (logListAll.size() > 0)
+            logList.push_front(QString("..."));
+    }
     QRegExp num ( "([0-9]+)" );

     int errorCount=0;

Screenshots interrupting FragM in GDB reveals it taking time in harfbuzz (which I think is a text rendering library).

Desktop (please complete the following information):

Additional context working on Magnetbulb, it compiled without errors finally...

3Dickulus commented 4 years ago

output to log window, QListWidget, not finding any documentation on upper limit of items, I think it's limited by memory? so can handle Ks or Ms of items.

HarfBuzz library allows programs to convert a sequence of Unicode input into properly formatted and positioned glyph output. Would selecting a "cheap" non-utf monospace font for the log window prevent the HarfBuzz library from doing fancy render stuff? (could use the font setting for the text editor set in prefs)

note comment // only jump to first error as later errors may be caused by this one so fix it first prior to this code I had planned on showing only the first if compile failed because too many caused crashes @QListWidget::addItem, was something in the Logging class, maybe that wasn't such a bad idea instead of fixing it, perhaps a case of "just because we can doesn't mean we should" show all the lines if many are triggered by the first error and go away when the first one is fixed.

break out of the foreach loop when errorCount > listWidget->DisplayableItems at this point add "..." as the last line.

set listWidget->DisplayableItems in Edit->Preferences ? for a complete log when there is some major debugging to do can just use the log-to-file option or bump up the listWidget->DisplayableItems value?

lol, but of course you are the one to find this bug ;)

3Dickulus commented 4 years ago

Tested: 1 - n lines

diff --git a/Fragmentarium-Source/Fragmentarium/GUI/PreferencesDialog.h b/Fragmentarium-Source/Fragmentarium/GUI/PreferencesDialog.h
index 998099e..8a45275 100644
--- a/Fragmentarium-Source/Fragmentarium/GUI/PreferencesDialog.h
+++ b/Fragmentarium-Source/Fragmentarium/GUI/PreferencesDialog.h
@@ -119,6 +119,7 @@ private slots:
 #endif // USE_OPEN_EXR
         m_ui.stylesheetLineEdit->setText (settings.value ( "editorStylesheet", "font: 9pt Courier;" ).toString() );
         m_ui.useMimetypesCheckBox->setChecked (settings.value ( "useMimetypes", false ).toBool() );
+        m_ui.logLinesSpinBox->setValue (settings.value ( "maxLogLines", 10 ).toInt() );
     }

     void saveSettings()
@@ -152,6 +153,7 @@ private slots:
 #endif // USE_OPEN_EXR
         settings.setValue("editorStylesheet", m_ui.stylesheetLineEdit->text());
         settings.setValue("useMimetypes", m_ui.useMimetypesCheckBox->isChecked() );
+        settings.setValue("maxLogLines", m_ui.logLinesSpinBox->value());
         settings.sync();
     }

diff --git a/Fragmentarium-Source/Fragmentarium/GUI/DisplayWidget.cpp b/Fragmentarium-Source/Fragmentarium/GUI/DisplayWidget.cpp
index 13a2f21..09ec915 100644
--- a/Fragmentarium-Source/Fragmentarium/GUI/DisplayWidget.cpp
+++ b/Fragmentarium-Source/Fragmentarium/GUI/DisplayWidget.cpp
 }

 void DisplayWidget::initializeGL()
@@ -580,6 +582,9 @@ void DisplayWidget::createErrorLineLog ( QString message, QString log, LogLevel

     LOG ( message, priority );

+    QSettings settings;
+    int maxLines = settings.value ( "maxLogLines", 10 ).toInt(); // for log window list widget
+
     foreach ( const QString &str, logList ) {
         QString newStr=str;

@@ -597,6 +602,8 @@ void DisplayWidget::createErrorLineLog ( QString message, QString log, LogLevel
             errorCount++;
         }

+        if(errorCount > maxLines) break;
+        
         // emit a single log widget line for each line in the log
         LOG ( newStr, priority );

also requires logLinesSpinBox added to the Preferences.ui

I have a few other pending commits, if you like this approach then i'll throw it in... during testing I find 5 lines is enough ;)