From 90bcaa8fcf23f54fa0876f1edbf071c4867627dd Mon Sep 17 00:00:00 2001 From: Nick Holland Date: Fri, 3 Mar 2017 12:23:06 +0000 Subject: [PATCH] Valgrind fix for uninitialized values in Render::Sampler Valgrind reported conditional jump depends on uninit values in: if( mSampler.mBitField != oldSampler.mBitField ) // Texture::ApplySampler Problem was down to Sampler having a anonymous union: union { unsigned int mBitfield; struct { FilterMode mMinificationFilter : 4; ///< The minify filter FilterMode mMagnificationFilter : 4; ///< The magnify filter WrapMode mSWrapMode : 4; ///< The horizontal wrap mode WrapMode mTWrapMode : 4; ///< The vertical wrap mode WrapMode mRWrapMode : 4; ///< The vertical wrap mode }; }; Fundamentally the size of the bit field struct can be larger than the size of the unsigned int mBitfield, so using mBitfield to compare Samplers isn't reliable. The issue valgrind picked up on is, only 20 bits of the mBitfield are being set in the sampler constructor Sampler() :mMinificationFilter(FilterMode::DEFAULT), mMagnificationFilter(FilterMode::DEFAULT), mSWrapMode(WrapMode::DEFAULT), mTWrapMode(WrapMode::DEFAULT), mRWrapMode(WrapMode::DEFAULT) {} Solution is to remove the union Change-Id: Id66afc05d21377a1363448ef10654cd1ab4bb365 --- dali/internal/render/renderers/render-sampler.h | 32 +++++++++++++---------- dali/internal/render/renderers/render-texture.cpp | 2 +- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/dali/internal/render/renderers/render-sampler.h b/dali/internal/render/renderers/render-sampler.h index 35557ca..c5040ae 100644 --- a/dali/internal/render/renderers/render-sampler.h +++ b/dali/internal/render/renderers/render-sampler.h @@ -50,21 +50,25 @@ struct Sampler ~Sampler() {} - - union + bool operator==(const Sampler& rhs) const { - unsigned int mBitfield; - - struct - { - FilterMode mMinificationFilter : 4; ///< The minify filter - FilterMode mMagnificationFilter : 4; ///< The magnify filter - WrapMode mSWrapMode : 4; ///< The horizontal wrap mode - WrapMode mTWrapMode : 4; ///< The vertical wrap mode - WrapMode mRWrapMode : 4; ///< The vertical wrap mode - }; - - }; + return ( ( mMinificationFilter == rhs.mMinificationFilter ) && + ( mMagnificationFilter == rhs.mMagnificationFilter ) && + ( mSWrapMode == rhs.mSWrapMode ) && + ( mTWrapMode == rhs.mTWrapMode ) && + ( mRWrapMode == rhs.mRWrapMode ) ); + } + + bool operator!=(const Sampler& rhs) const + { + return !(*this == rhs); + } + + FilterMode mMinificationFilter : 4; ///< The minify filter + FilterMode mMagnificationFilter : 4; ///< The magnify filter + WrapMode mSWrapMode : 4; ///< The horizontal wrap mode + WrapMode mTWrapMode : 4; ///< The vertical wrap mode + WrapMode mRWrapMode : 4; ///< The vertical wrap mode }; } // namespace Render diff --git a/dali/internal/render/renderers/render-texture.cpp b/dali/internal/render/renderers/render-texture.cpp index e6e262b..d0d655b 100644 --- a/dali/internal/render/renderers/render-texture.cpp +++ b/dali/internal/render/renderers/render-texture.cpp @@ -826,7 +826,7 @@ void Texture::ApplySampler( Context& context, Render::Sampler* sampler ) Render::Sampler oldSampler = mSampler; mSampler = sampler ? *sampler : Sampler(); - if( mSampler.mBitfield != oldSampler.mBitfield ) + if( mSampler != oldSampler ) { GLint mode = FilterModeToGL( mSampler.mMinificationFilter, DALI_MINIFY_DEFAULT, GL_MINIFY_DEFAULT ); if( mode != FilterModeToGL( oldSampler.mMinificationFilter, DALI_MINIFY_DEFAULT, GL_MINIFY_DEFAULT ) ) -- 2.7.4