Skip to content

Commit

Permalink
Remove extraneous slotUpdate.
Browse files Browse the repository at this point in the history 
All preference pages get a slotUpdate call when preferences are shown.
  • Loading branch information
@rryan
rryan committed May 12, 2016
1 parent b904c29 commit 7e9bba7
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions src/preferences/dialog/dlgpreferences.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,7 @@ void DlgPreferences::changePage(QTreeWidgetItem* current, QTreeWidgetItem* previ
current = previous;

if (current == m_pSoundButton) {
m_wsound->slotUpdate();
switchToPage(m_wsound);
switchToPage(m_wsound);
} else if (current == m_pLibraryButton) {
switchToPage(m_wlibrary);
} else if (current == m_pControlsButton) {
Expand Down

14 comments on commit 7e9bba7

@sjtoik
Copy link

@sjtoik sjtoik commented on 7e9bba7 May 12, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current master head build fails with error. After scons -c

g++ -o lin64_build/analyzer/analyzerqueue.o -c -std=c++11 -pipe -Wall -Wextra -g -O3 -ffast-math -funroll-loops -fomit-frame-pointer -mtune=generic -pthread -Dx86_64 -DMIXXX_BUILD_DEBUG -D__LINUX__ -D__UNIX__ -DSETTINGS_PATH=".mixxx/" -DSETTINGS_FILE="mixxx.cfg" -DUNIX_SHARE_PATH="/usr/local/share/mixxx" -DUNIX_LIB_PATH="/usr/local/lib/mixxx" -D__PORTAUDIO__ -DQT_SHARED -DQT_TABLET_SUPPORT -DQT_CORE_LIB -DQT_GUI_LIB -DQT_OPENGL_LIB -DQT_XML_LIB -DQT_SVG_LIB -DQT_SQL_LIB -DQT_SCRIPT_LIB -DQT_NETWORK_LIB -DQT_SHARED -D__SNDFILE__ -D__MAD__ -D__HID__ -D__BULK__ -D__VINYLCONTROL__ -D__BROADCAST__ -D__OPUS__ -D__VAMP__ -DHAVE_FFTW3 -D__SQLITE3__ -D__BATTERY__ -Ilin64_build -Isrc -I/usr/include/soundtouch -Ilib/replaygain -Ilib/libebur128-1.1.0/ebur128 -I/usr/include/qt4 -I/usr/include/qt4/QtOpenGL -I/usr/include/qt4/QtXml -I/usr/include/qt4/QtSvg -I/usr/include/qt4/QtSql -I/usr/include/qt4/QtXmlPatterns -I/usr/include/qt4/QtNetwork -I/usr/include/qt4/QtTest -I/usr/include/qt4/QtScriptTools -I/usr/include/qt4/QtGui -I/usr/include/qt4/QtScript -I/usr/include/qt4/QtCore -Ilib/gtest-1.7.0/include -Ilib/fidlib-0.9.10 -I/usr/include/taglib -Ilib/qtscript-bytearray -Ilib/reverb -Ilib/portaudio -I/usr/include/hidapi -I/usr/include/libusb-1.0 -Ilib/xwax -Ilib/scratchlib -I/usr/include/opus -I/usr/include/libupower-glib -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include src/analyzer/analyzerqueue.cpp
In file included from src/util/math.h:16:0,
from src/util/types.h:7,
from src/analyzer/analyzer.h:4,
from src/analyzer/analyzerqueue.h:12,
from src/analyzer/analyzerqueue.cpp:1:
src/util/fpclassify.h:22:20: error: ‘std::util_fpclassify’ has not been declared
#define fpclassify util_fpclassify
^
src/util/fpclassify.h:23:18: error: ‘std::util_isfinite’ has not been declared
#define isfinite util_isfinite
^
src/util/fpclassify.h:20:15: error: ‘std::util_isinf’ has not been declared
#define isinf util_isinf
^
src/util/fpclassify.h:19:15: error: ‘std::util_isnan’ has not been declared
#define isnan util_isnan
^
src/util/fpclassify.h:21:18: error: ‘std::util_isnormal’ has not been declared
#define isnormal util_isnormal
^
scons: *** [lin64_build/analyzer/analyzerqueue.o] Error 1
scons: building terminated because of errors.

@rryan
Copy link
Member Author

@rryan rryan commented on 7e9bba7 May 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjtoik -- shoot, sorry about that. I can build fine on Linux and Mac though. Could you share some more details about your setup? What distro? What version of gcc?

@rryan
Copy link
Member Author

@rryan rryan commented on 7e9bba7 May 12, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daschuer it almost seems as if fpclassify methods are getting inlined -- since it seems like these error messages could only occur if std::isnan in fpclassify.cpp was getting preprocessor replaced to become std::util_isnan. Weird.

@rryan
Copy link
Member Author

@rryan rryan commented on 7e9bba7 May 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjtoik -- what was the last sha you could build successfully at?

@daschuer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think not that there is something inlined, since the function is not there. It looks like there is something weird with the include order. It looks like std::isnan is replaced by the #define isnan util_isnan to
std::util_isnan
It could be interesting to look at the preprocessed C file

gcc -E test.c -o test.i

@sjtoik:

please call

g++ -E -o lin64_build/analyzer/analyzerqueue.i -c -std=c++11 -pipe -Wall -Wextra -g -O3 -ffast-math -funroll-loops -fomit-frame-pointer -mtune=generic -pthread -Dx86_64 -DMIXXX_BUILD_DEBUG -D__LINUX__ -D__UNIX__ -DSETTINGS_PATH=\".mixxx/\" -DSETTINGS_FILE=\"mixxx.cfg\" -DUNIX_SHARE_PATH=\"/usr/local/share/mixxx\" -DUNIX_LIB_PATH=\"/usr/local/lib/mixxx\" -D__PORTAUDIO__ -DQT_SHARED -DQT_TABLET_SUPPORT -DQT_CORE_LIB -DQT_GUI_LIB -DQT_OPENGL_LIB -DQT_XML_LIB -DQT_SVG_LIB -DQT_SQL_LIB -DQT_SCRIPT_LIB -DQT_NETWORK_LIB -DQT_SHARED -D__SNDFILE__ -D__MAD__ -D__HID__ -D__BULK__ -D__VINYLCONTROL__ -D__BROADCAST__ -D__OPUS__ -D__VAMP__ -DHAVE_FFTW3 -D__SQLITE3__ -D__BATTERY__ -Ilin64_build -Isrc -I/usr/include/soundtouch -Ilib/replaygain -Ilib/libebur128-1.1.0/ebur128 -I/usr/include/qt4 -I/usr/include/qt4/QtOpenGL -I/usr/include/qt4/QtXml -I/usr/include/qt4/QtSvg -I/usr/include/qt4/QtSql -I/usr/include/qt4/QtXmlPatterns -I/usr/include/qt4/QtNetwork -I/usr/include/qt4/QtTest -I/usr/include/qt4/QtScriptTools -I/usr/include/qt4/QtGui -I/usr/include/qt4/QtScript -I/usr/include/qt4/QtCore -Ilib/gtest-1.7.0/include -Ilib/fidlib-0.9.10 -I/usr/include/taglib -Ilib/qtscript-bytearray -Ilib/reverb -Ilib/portaudio -I/usr/include/hidapi -I/usr/include/libusb-1.0 -Ilib/xwax -Ilib/scratchlib -I/usr/include/opus -I/usr/include/libupower-glib -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include src/analyzer/analyzerqueue.cpp

and attach the result: lin64_build/analyzer/analyzerqueue.i here.

@daschuer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not have the issue. Here is is mine ouput file:
https://gist.github.com/daschuer/9d85f7f92e44e9888b6b6af0d377e0e0

nothing suspicious we see the compiler inline function that should not be used via the define.
there is nothing std:: related.

isnan containing lines:

extern int isnan (double __value) throw () __attribute__ ((__const__));
...
  constexpr bool
  isnan(float __x)
  { return __builtin_isnan(__x); }

  constexpr bool
  isnan(double __x)
  { return __builtin_isnan(__x); }

  constexpr bool
  isnan(long double __x)
  { return __builtin_isnan(__x); }
...
    isnan(_Tp __x)
    { return false; }
...
int util_isnan(float x);
...
int util_isnan(double x);

@sjtoik
Copy link

@sjtoik sjtoik commented on 7e9bba7 May 26, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rryan Last confirmed build was May 3rd. when you helped with the debugging on IRC. Current head 16a7280 fails with the same error.
I'm rocking Arch Linux with gcc version 6.1.1

@sjtoik
Copy link

@sjtoik sjtoik commented on 7e9bba7 May 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daschuer Here is the output for you
analyzerqueue.txt

@rryan
Copy link
Member Author

@rryan rryan commented on 7e9bba7 May 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting:

#6 "/usr/include/qt4/QtCore/QtCore" 2
#1 "/usr/include/qt4/QtCore/qcoreapplication.h" 1
#7 "/usr/include/qt4/QtCore/QtCore" 2
#1 "/usr/include/qt4/QtCore/qcoreevent.h" 1
#8 "/usr/include/qt4/QtCore/QtCore" 2
#1 "/usr/include/qt4/QtCore/qeventloop.h" 1
#9 "/usr/include/qt4/QtCore/QtCore" 2
#1 "/usr/include/qt4/QtCore/qmath.h" 1
#45 "/usr/include/qt4/QtCore/qmath.h"
#1 "/usr/include/c++/6.1.1/math.h" 1 3
#36 "/usr/include/c++/6.1.1/math.h" 3
#1 "/usr/include/c++/6.1.1/cmath" 1 3
#39 "/usr/include/c++/6.1.1/cmath" 3

#40 "/usr/include/c++/6.1.1/cmath" 3
#37 "/usr/include/c++/6.1.1/math.h" 2 3


#38 "/usr/include/c++/6.1.1/math.h" 3

<snip>

using std::
#63 "/usr/include/c++/6.1.1/math.h"
          util_fpclassify
#63 "/usr/include/c++/6.1.1/math.h" 3
                    ;
using std::
#64 "/usr/include/c++/6.1.1/math.h"
          util_isfinite
#64 "/usr/include/c++/6.1.1/math.h" 3
                  ;
using std::
#65 "/usr/include/c++/6.1.1/math.h"
          util_isinf
#65 "/usr/include/c++/6.1.1/math.h" 3
               ;
using std::
#66 "/usr/include/c++/6.1.1/math.h"
          util_isnan
#66 "/usr/include/c++/6.1.1/math.h" 3
               ;
using std::
#67 "/usr/include/c++/6.1.1/math.h"
          util_isnormal
#67 "/usr/include/c++/6.1.1/math.h" 3
                  ;

@rryan
Copy link
Member Author

@rryan rryan commented on 7e9bba7 May 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we probably have no business defining macros for isnan, isfinite, etc. :)
Maybe we need to find-replace everything across the codebase to add a mixxx_ prefix?

@Pegasus-RPG
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good idea. I've done my fair share of isnan hacks when getting Windows building working. It would be nice to have a definitive way to handle the situation.

@daschuer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Mixxx, we can directly call util_fpclassify. What hurts, are inline functions in third party code, since they inline fpclassify as well and the compiler just throws it out because of our optimization flags.
In the current implementation it depends on the include order if header inline calls are removed or not.
That is a bad situation anyway, but an other issue.
To solve the issue here, we have to put our definition between the /usr/include/c++/6.1.1/math.h include and before the usage of fpclassify and friends.

@sjtoik: please try if it solves the issue to replace by <math.h> in https://github.com/mixxxdj/mixxx/blob/master/src/util/math.h#L12 /usr/include/c++/6.1.1/math.h includes
not sure if this works for all targets though ..

@sjtoik
Copy link

@sjtoik sjtoik commented on 7e9bba7 May 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daschuer Changing that cmath to #include "/usr/include/c++/6.1.1/math.h"
did the trick.

@rryan
Copy link
Member Author

@rryan rryan commented on 7e9bba7 May 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someone on IRC is facing the same issue on Arch.

I filed a bug to track this -- https://bugs.launchpad.net/mixxx/+bug/1586808

Please sign in to comment.