From b3c7f003da75d952f13d8f0781457f22a1056902 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Wed, 11 Jun 2014 20:06:05 +0200 Subject: [PATCH 1/3] BitField: Delete copy assignment to prevent obscure bugs. --- Source/Core/Common/BitField.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Source/Core/Common/BitField.h b/Source/Core/Common/BitField.h index f7a0a0175c..0dda177b59 100644 --- a/Source/Core/Common/BitField.h +++ b/Source/Core/Common/BitField.h @@ -125,6 +125,22 @@ public: // so that we can use this within unions BitField() = default; +#ifndef _WIN32 + // We explicitly delete the copy assigment operator here, because the + // default copy assignment would copy the full storage value, rather than + // just the bits relevant to this particular bit field. + // Ideally, we would just implement the copy assignment to copy only the + // relevant bits, but this requires compiler support for unrestricted + // unions. + // MSVC 2013 has no support for this, hence we disable this code on + // Windows (so that the default copy assignment operator will be used). + // For any C++11 conformant compiler we delete the operator to make sure + // we never use this inappropriate operator to begin with. + // TODO: Implement this operator properly once all target compilers + // support unrestricted unions. + BitField& operator=(const BitField&) = delete; +#endif + __forceinline BitField& operator=(T val) { storage = (storage & ~GetMask()) | ((val << position) & GetMask()); From 78fbf2ecaa8731800dfed8034bbeb1244d68624c Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Wed, 11 Jun 2014 20:34:15 +0200 Subject: [PATCH 2/3] Fix a few warnings caused by using BitField with non-typesafe functions. --- Source/Core/VideoCommon/VertexManagerBase.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/VideoCommon/VertexManagerBase.cpp b/Source/Core/VideoCommon/VertexManagerBase.cpp index 5e44a5aa0e..84db7cffe8 100644 --- a/Source/Core/VideoCommon/VertexManagerBase.cpp +++ b/Source/Core/VideoCommon/VertexManagerBase.cpp @@ -154,7 +154,7 @@ void VertexManager::Flush() #if defined(_DEBUG) || defined(DEBUGFAST) PRIM_LOG("frame%d:\n texgen=%d, numchan=%d, dualtex=%d, ztex=%d, cole=%d, alpe=%d, ze=%d", g_ActiveConfig.iSaveTargetId, xfmem.numTexGen.numTexGens, xfmem.numChan.numColorChans, xfmem.dualTexTrans.enabled, bpmem.ztex2.op, - bpmem.blendmode.colorupdate, bpmem.blendmode.alphaupdate, bpmem.zmode.updateenable); + (int)bpmem.blendmode.colorupdate, (int)bpmem.blendmode.alphaupdate, (int)bpmem.zmode.updateenable); for (unsigned int i = 0; i < xfmem.numChan.numColorChans; ++i) { @@ -175,8 +175,8 @@ void VertexManager::Flush() xfmem.postMtxInfo[i].index, xfmem.postMtxInfo[i].normalize); } - PRIM_LOG("pixel: tev=%d, ind=%d, texgen=%d, dstalpha=%d, alphatest=0x%x", bpmem.genMode.numtevstages+1, bpmem.genMode.numindstages, - bpmem.genMode.numtexgens, (u32)bpmem.dstalpha.enable, (bpmem.alpha_test.hex>>16)&0xff); + PRIM_LOG("pixel: tev=%d, ind=%d, texgen=%d, dstalpha=%d, alphatest=0x%x", (int)bpmem.genMode.numtevstages+1, (int)bpmem.genMode.numindstages, + (int)bpmem.genMode.numtexgens, (u32)bpmem.dstalpha.enable, (bpmem.alpha_test.hex>>16)&0xff); #endif u32 usedtextures = 0; From 3d6f9ef89788dbb64c2a6b4becb84905667b1485 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Wed, 11 Jun 2014 20:35:39 +0200 Subject: [PATCH 3/3] BitField: Add an explicit getter function for retrieving the BitField value. Sometimes (in particular when using non-typesafe functions) it can be convenient to have a getter method rather than performing a potentially lengthy explicit cast. --- Source/Core/Common/BitField.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Source/Core/Common/BitField.h b/Source/Core/Common/BitField.h index 0dda177b59..8ec80de9c0 100644 --- a/Source/Core/Common/BitField.h +++ b/Source/Core/Common/BitField.h @@ -147,7 +147,7 @@ public: return *this; } - __forceinline operator T() const + __forceinline T Value() const { if (std::numeric_limits::is_signed) { @@ -160,6 +160,11 @@ public: } } + __forceinline operator T() const + { + return Value(); + } + private: // StorageType is T for non-enum types and the underlying type of T if // T is an enumeration. Note that T is wrapped within an enable_if in the