From 9b4443d555c2eb3b7cec2bcb99b45955c33022da Mon Sep 17 00:00:00 2001 From: Stephen Anthony Date: Mon, 16 Oct 2017 14:39:40 -0230 Subject: [PATCH 1/7] Only automatically enable clang extended warnings in version 5+. --- Makefile | 2 +- configure | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index f80e68eb9..2b0c212c8 100644 --- a/Makefile +++ b/Makefile @@ -53,7 +53,7 @@ CXXFLAGS+= -Wall -Wextra -Wno-unused-parameter -Wno-ignored-qualifiers ifdef HAVE_GCC CXXFLAGS+= -Wno-multichar -Wunused -fno-rtti -Woverloaded-virtual -Wnon-virtual-dtor -std=c++14 endif -ifdef HAVE_CLANG +ifdef CLANG_WARNINGS CXXFLAGS+= -Weverything -Wno-c++17-extensions -Wno-c++98-compat -Wno-c++98-compat-pedantic \ -Wno-double-promotion -Wno-switch-enum -Wno-conversion -Wno-covered-switch-default \ -Wno-inconsistent-missing-destructor-override \ diff --git a/configure b/configure index 648452860..169657f2b 100755 --- a/configure +++ b/configure @@ -414,12 +414,16 @@ if test "$have_clang" = yes; then cxx_version="$cxx_version, bad" cxx_verc_fail=yes fi + + # Only clang >= 5.0 supports extra warnings + if [ $clang_major -ge 5 ]; then + _make_def_CLANG_WARNINGS='CLANG_WARNINGS = 1' + fi fi CXXFLAGS="$CXXFLAGS" _make_def_HAVE_GCC3='HAVE_GCC3 = 1' add_line_to_config_mk 'CXX_UPDATE_DEP_FLAG = -MMD -MF "$(*D)/$(DEPDIR)/$(*F).d" -MQ "$@" -MP' _make_def_HAVE_GCC='HAVE_GCC = 1' - _make_def_HAVE_CLANG='HAVE_CLANG = 1' echo "$cxx_version" elif test "$have_gcc" = yes; then @@ -817,7 +821,7 @@ PROFILE := $_build_profile $_make_def_HAVE_GCC $_make_def_HAVE_GCC3 -$_make_def_HAVE_CLANG +$_make_def_CLANG_WARNINGS INCLUDES += $INCLUDES OBJS += $OBJS From f02a7020b018d41c5e21db2513d7c6420f1069c0 Mon Sep 17 00:00:00 2001 From: Christian Speckner Date: Mon, 16 Oct 2017 23:36:38 +0200 Subject: [PATCH 2/7] Fix typo. --- src/emucore/tia/frame-manager/JitterEmulation.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/emucore/tia/frame-manager/JitterEmulation.cxx b/src/emucore/tia/frame-manager/JitterEmulation.cxx index db6c48372..3026863ff 100644 --- a/src/emucore/tia/frame-manager/JitterEmulation.cxx +++ b/src/emucore/tia/frame-manager/JitterEmulation.cxx @@ -65,7 +65,7 @@ void JitterEmulation::updateJitter(Int32 scanlineDifference) { if (uInt32(abs(scanlineDifference)) < Metrics::minDeltaForJitter) return; - Int32 jitter = std::min(jitter, Metrics::maxJitter); + Int32 jitter = std::min(scanlineDifference, Metrics::maxJitter); jitter = std::max(jitter, -myYStart); if (jitter > 0) jitter += myJitterFactor; From 18895e3683e4df8fbf3438515225f6404b710c25 Mon Sep 17 00:00:00 2001 From: Christian Speckner Date: Mon, 16 Oct 2017 23:37:22 +0200 Subject: [PATCH 3/7] Remove FIXME --- the comparision is legitimate. --- src/emucore/tia/PaddleReader.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/emucore/tia/PaddleReader.cxx b/src/emucore/tia/PaddleReader.cxx index 93d27b6ae..461bd6f77 100644 --- a/src/emucore/tia/PaddleReader.cxx +++ b/src/emucore/tia/PaddleReader.cxx @@ -69,7 +69,7 @@ void PaddleReader::update(double value, double timestamp, ConsoleTiming consoleT setConsoleTiming(consoleTiming); } - if (value != myValue) { // FIXME - warning on 'no-float-equal' + if (value != myValue) { myValue = value; if (myValue < 0) { From 4a73aab1e4f9804084d4a9900783316eed6a3368 Mon Sep 17 00:00:00 2001 From: Christian Speckner Date: Mon, 16 Oct 2017 23:53:41 +0200 Subject: [PATCH 4/7] Fix segfault during ystart autodetection. --- src/emucore/Console.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/emucore/Console.cxx b/src/emucore/Console.cxx index c62666827..033f0b459 100644 --- a/src/emucore/Console.cxx +++ b/src/emucore/Console.cxx @@ -244,7 +244,7 @@ void Console::autodetectYStart() YStartDetector ystartDetector; ystartDetector.setLayout(myDisplayFormat == "PAL" ? FrameLayout::pal : FrameLayout::ntsc); myTIA->setFrameManager(&ystartDetector); - mySystem->reset(); + mySystem->reset(true); for (int i = 0; i < 80; i++) myTIA->update(); From 1338b8d0d05bd28539b239637fb5c8bcb8773714 Mon Sep 17 00:00:00 2001 From: Christian Speckner Date: Tue, 17 Oct 2017 00:25:24 +0200 Subject: [PATCH 5/7] Fix spurious jitter in ROMs with continously changing frame height (ghost!). --- src/common/StateManager.cxx | 2 +- .../tia/frame-manager/JitterEmulation.cxx | 16 ++++++++++++---- .../tia/frame-manager/JitterEmulation.hxx | 4 +++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/common/StateManager.cxx b/src/common/StateManager.cxx index e921567e2..4ce11a951 100644 --- a/src/common/StateManager.cxx +++ b/src/common/StateManager.cxx @@ -28,7 +28,7 @@ #include "StateManager.hxx" -#define STATE_HEADER "05000303state" +#define STATE_HEADER "05000304state" // #define MOVIE_HEADER "03030000movie" // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/emucore/tia/frame-manager/JitterEmulation.cxx b/src/emucore/tia/frame-manager/JitterEmulation.cxx index 3026863ff..438b35831 100644 --- a/src/emucore/tia/frame-manager/JitterEmulation.cxx +++ b/src/emucore/tia/frame-manager/JitterEmulation.cxx @@ -18,9 +18,10 @@ #include "JitterEmulation.hxx" enum Metrics: uInt32 { - framesForStableHeight = 2, - minDeltaForJitter = 3, - maxJitter = 50 + framesForStableHeight = 2, + framesUntilDestabilization = 10, + minDeltaForJitter = 3, + maxJitter = 50 }; // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -32,9 +33,10 @@ JitterEmulation::JitterEmulation() : void JitterEmulation::reset() { myLastFrameScanlines = 0; - myStableFrameFinalLines = 0; + myStableFrameFinalLines = -1; myStableFrames = 0; myStabilizationCounter = 0; + myDestabilizationCounter = 0; myJitter = 0; } @@ -42,17 +44,21 @@ void JitterEmulation::reset() void JitterEmulation::frameComplete(uInt32 scanlineCount) { if (scanlineCount != myStableFrameFinalLines) { + if (myDestabilizationCounter++ > Metrics::framesUntilDestabilization) myStableFrameFinalLines = -1; + if (scanlineCount == myLastFrameScanlines) { if (++myStabilizationCounter >= Metrics::framesForStableHeight) { if (myStableFrameFinalLines > 0) updateJitter(scanlineCount - myStableFrameFinalLines); myStableFrameFinalLines = scanlineCount; + myDestabilizationCounter = 0; } } else myStabilizationCounter = 0; } + else myDestabilizationCounter = 0; myLastFrameScanlines = scanlineCount; @@ -84,6 +90,7 @@ bool JitterEmulation::save(Serializer& out) const out.putInt(myStableFrameFinalLines); out.putInt(myStableFrames); out.putInt(myStabilizationCounter); + out.putInt(myDestabilizationCounter); out.putInt(myJitter); out.putInt(myJitterFactor); out.putInt(myYStart); @@ -108,6 +115,7 @@ bool JitterEmulation::load(Serializer& in) myStableFrameFinalLines = in.getInt(); myStableFrames = in.getInt(); myStabilizationCounter = in.getInt(); + myDestabilizationCounter = in.getInt(); myJitter = in.getInt(); myJitterFactor = in.getInt(); myYStart = in.getInt(); diff --git a/src/emucore/tia/frame-manager/JitterEmulation.hxx b/src/emucore/tia/frame-manager/JitterEmulation.hxx index 6e67eb62c..5c0f47ea3 100644 --- a/src/emucore/tia/frame-manager/JitterEmulation.hxx +++ b/src/emucore/tia/frame-manager/JitterEmulation.hxx @@ -58,12 +58,14 @@ class JitterEmulation: public Serializable { uInt32 myLastFrameScanlines; - uInt32 myStableFrameFinalLines; + Int32 myStableFrameFinalLines; uInt32 myStableFrames; uInt32 myStabilizationCounter; + uInt32 myDestabilizationCounter; + Int32 myJitter; Int32 myJitterFactor; From 424808b94820f6b71f131008adf0524da3df35a9 Mon Sep 17 00:00:00 2001 From: Christian Speckner Date: Tue, 17 Oct 2017 00:26:12 +0200 Subject: [PATCH 6/7] Revised my decision ;) --- src/emucore/tia/frame-manager/FrameManager.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/emucore/tia/frame-manager/FrameManager.cxx b/src/emucore/tia/frame-manager/FrameManager.cxx index 40629f152..6d2b407f8 100644 --- a/src/emucore/tia/frame-manager/FrameManager.cxx +++ b/src/emucore/tia/frame-manager/FrameManager.cxx @@ -162,7 +162,6 @@ void FrameManager::setState(FrameManager::State state) } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -// TODO: kill this with fire once frame manager refactoring is complete void FrameManager::onLayoutChange() { switch (layout()) From ad2d53758f9bfefaf310641eec49378952dc5a5d Mon Sep 17 00:00:00 2001 From: Stephen Anthony Date: Tue, 17 Oct 2017 09:57:19 -0230 Subject: [PATCH 7/7] Fixed final warning in jitter code. --- src/emucore/tia/frame-manager/JitterEmulation.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/emucore/tia/frame-manager/JitterEmulation.cxx b/src/emucore/tia/frame-manager/JitterEmulation.cxx index 438b35831..c815b0701 100644 --- a/src/emucore/tia/frame-manager/JitterEmulation.cxx +++ b/src/emucore/tia/frame-manager/JitterEmulation.cxx @@ -43,7 +43,7 @@ void JitterEmulation::reset() // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void JitterEmulation::frameComplete(uInt32 scanlineCount) { - if (scanlineCount != myStableFrameFinalLines) { + if (Int32(scanlineCount) != myStableFrameFinalLines) { if (myDestabilizationCounter++ > Metrics::framesUntilDestabilization) myStableFrameFinalLines = -1; if (scanlineCount == myLastFrameScanlines) {