From 83d460b303c2e92a62c3a5550dfcccc3c62ae90b Mon Sep 17 00:00:00 2001 From: Kingsley Stephens Date: Mon, 4 Aug 2014 11:18:41 +0100 Subject: [PATCH] Image filter bug fix [Problem] There was a bug in the image filtering code that was setting default filtering to nearest mode [Solution] The default will now be linear Change-Id: Ibdf52b0ffbf55cb3d9885efc03c30579fb04228e --- .../dali-internal/utc-Dali-Internal-Material.cpp | 12 ++-- .../src/dali/utc-Dali-RenderableActor.cpp | 67 +++++++++++++++++++-- dali/internal/common/image-sampler.cpp | 58 +++++------------- dali/internal/common/image-sampler.h | 24 -------- dali/internal/render/gl-resources/texture.cpp | 69 ++++++++++++++-------- dali/internal/render/gl-resources/texture.h | 12 ++++ .../render/renderers/scene-graph-renderer.cpp | 2 +- dali/internal/render/shaders/shader.cpp | 2 +- dali/public-api/actors/sampling.h | 3 +- 9 files changed, 143 insertions(+), 106 deletions(-) diff --git a/automated-tests/src/dali-internal/utc-Dali-Internal-Material.cpp b/automated-tests/src/dali-internal/utc-Dali-Internal-Material.cpp index 920dc65..4e3fe95 100644 --- a/automated-tests/src/dali-internal/utc-Dali-Internal-Material.cpp +++ b/automated-tests/src/dali-internal/utc-Dali-Internal-Material.cpp @@ -387,7 +387,7 @@ int UtcDaliMaterialStaging01(void) application.Render(); //Process render Q stores & processes mat renderMaterial->SetUniforms( materialUniforms, testProgram.GetProgram(), Internal::SHADER_DEFAULT ); - renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::DefaultOptions() ); + renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::PackBitfield( FilterMode::DEFAULT, FilterMode::DEFAULT ) ); DALI_TEST_CHECK( boundTextures.GetNumBoundTextures() == 0 ); DALI_TEST_EQUALS( testProgram.GetOpacity(), TEST_PROPS.mOpacity, TEST_LOCATION); @@ -441,7 +441,7 @@ int UtcDaliMaterialStaging02(void) application.Render(); //Process render Q stores & processes mat renderMaterial->SetUniforms( materialUniforms, testProgram.GetProgram(), Internal::SHADER_DEFAULT ); - renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::DefaultOptions() ); + renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::PackBitfield( FilterMode::DEFAULT, FilterMode::DEFAULT ) ); DALI_TEST_CHECK( boundTextures.GetNumBoundTextures() == 0 ); DALI_TEST_EQUALS( testProgram.GetOpacity(), 0.4f, TEST_LOCATION); @@ -484,7 +484,7 @@ int UtcDaliMaterialSetPropsWhilstStaged(void) Internal::SceneGraph::RenderMaterialUniforms materialUniforms; renderMaterial->SetUniforms( materialUniforms, testProgram.GetProgram(), Internal::SHADER_DEFAULT ); - renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::DefaultOptions() ); + renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::PackBitfield( FilterMode::DEFAULT, FilterMode::DEFAULT ) ); DALI_TEST_EQUALS( boundTextures.GetNumBoundTextures(), 0u, TEST_LOCATION ); @@ -526,7 +526,7 @@ int UtcDaliMaterialSetTextureWhilstStaged(void) application.Render(); // Update & Prepare material application.Render(); // Process render Q - renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::DefaultOptions() ); + renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::PackBitfield( FilterMode::DEFAULT, FilterMode::DEFAULT ) ); DALI_TEST_CHECK( boundTextures.CheckFirstTextureBound( GL_TEXTURE0 ) ); END_TEST; } @@ -566,7 +566,7 @@ int UtcDaliMaterialSetUnreadyTextureWhilstStaged(void) application.Render(); // Update & Prepare material application.Render(); // Process render Q - renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::DefaultOptions() ); + renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::PackBitfield( FilterMode::DEFAULT, FilterMode::DEFAULT ) ); DALI_TEST_EQUALS( boundTextures.GetNumBoundTextures(), 0u, TEST_LOCATION ); @@ -578,7 +578,7 @@ int UtcDaliMaterialSetUnreadyTextureWhilstStaged(void) application.Render(); // Process LoadComplete application.SendNotification(); // Process event messages - renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::DefaultOptions() ); + renderMaterial->BindTextures( testProgram.GetProgram(), Internal::ImageSampler::PackBitfield( FilterMode::DEFAULT, FilterMode::DEFAULT ) ); DALI_TEST_CHECK( boundTextures.CheckFirstTextureBound( GL_TEXTURE0 ) ); END_TEST; } diff --git a/automated-tests/src/dali/utc-Dali-RenderableActor.cpp b/automated-tests/src/dali/utc-Dali-RenderableActor.cpp index ee3075b..5eaea36 100644 --- a/automated-tests/src/dali/utc-Dali-RenderableActor.cpp +++ b/automated-tests/src/dali/utc-Dali-RenderableActor.cpp @@ -586,9 +586,8 @@ int UtcDaliRenderableActorSetFilterMode(void) // Verify actor gl state - // The first one should be true as we don't want to use the GL default - // The second one should be false as we want to use the GL default, therefore the TexParameter function should not be called // There are two calls to TexParameteri when the texture is first created + // Texture mag filter is not called as the first time set it uses the system default DALI_TEST_EQUALS( texParameterTrace.CountMethod( "TexParameteri" ), 3, TEST_LOCATION); out.str(""); @@ -597,6 +596,26 @@ int UtcDaliRenderableActorSetFilterMode(void) /**************************************************************/ + // Default/Default + texParameterTrace = application.GetGlAbstraction().GetTexParameterTrace(); + texParameterTrace.Reset(); + texParameterTrace.Enable( true ); + + actor.SetFilterMode( FilterMode::DEFAULT, FilterMode::DEFAULT ); + + // Flush the queue and render once + application.SendNotification(); + application.Render(); + + texParameterTrace.Enable( false ); + + // Verify actor gl state + + // Should not make any calls when settings are the same + DALI_TEST_EQUALS( texParameterTrace.CountMethod( "TexParameteri" ), 0, TEST_LOCATION); + + /**************************************************************/ + // Nearest/Nearest texParameterTrace.Reset(); texParameterTrace.Enable( true ); @@ -661,15 +680,53 @@ int UtcDaliRenderableActorSetFilterMode(void) texParameterTrace.Enable( false ); // Verify actor gl state - DALI_TEST_EQUALS( texParameterTrace.CountMethod( "TexParameteri" ), 2, TEST_LOCATION); + DALI_TEST_EQUALS( texParameterTrace.CountMethod( "TexParameteri" ), 1, TEST_LOCATION); out.str(""); out << GL_TEXTURE_2D << ", " << GL_TEXTURE_MIN_FILTER << ", " << GL_NEAREST; DALI_TEST_EQUALS( texParameterTrace.TestMethodAndParams(0, "TexParameteri", out.str()), true, TEST_LOCATION); + /**************************************************************/ + + // Default/Default + texParameterTrace.Reset(); + texParameterTrace.Enable( true ); + + actor.SetFilterMode( FilterMode::DEFAULT, FilterMode::DEFAULT ); + + // Flush the queue and render once + application.SendNotification(); + application.Render(); + + texParameterTrace.Enable( false ); + + // Verify actor gl state + DALI_TEST_EQUALS( texParameterTrace.CountMethod( "TexParameteri" ), 1, TEST_LOCATION); + out.str(""); - out << GL_TEXTURE_2D << ", " << GL_TEXTURE_MAG_FILTER << ", " << GL_LINEAR; - DALI_TEST_EQUALS( texParameterTrace.TestMethodAndParams(1, "TexParameteri", out.str()), true, TEST_LOCATION); + out << GL_TEXTURE_2D << ", " << GL_TEXTURE_MIN_FILTER << ", " << GL_LINEAR; + DALI_TEST_EQUALS( texParameterTrace.TestMethodAndParams(0, "TexParameteri", out.str()), true, TEST_LOCATION); + + /**************************************************************/ + + // None/None + texParameterTrace.Reset(); + texParameterTrace.Enable( true ); + + actor.SetFilterMode( FilterMode::NONE, FilterMode::NONE ); + + // Flush the queue and render once + application.SendNotification(); + application.Render(); + + texParameterTrace.Enable( false ); + + // Verify actor gl state + DALI_TEST_EQUALS( texParameterTrace.CountMethod( "TexParameteri" ), 1, TEST_LOCATION); + + out.str(""); + out << GL_TEXTURE_2D << ", " << GL_TEXTURE_MIN_FILTER << ", " << GL_NEAREST_MIPMAP_LINEAR; + DALI_TEST_EQUALS( texParameterTrace.TestMethodAndParams(0, "TexParameteri", out.str()), true, TEST_LOCATION); /**************************************************************/ diff --git a/dali/internal/common/image-sampler.cpp b/dali/internal/common/image-sampler.cpp index 549f4bf..9c2ccc3 100644 --- a/dali/internal/common/image-sampler.cpp +++ b/dali/internal/common/image-sampler.cpp @@ -39,17 +39,14 @@ const int MAGNIFY_BIT_SHIFT = 4; const int MASK_MINIFY_FILTER = 0x0000000F; const int MASK_MAGNIFY_FILTER = 0x000000F0; -const unsigned int FILTER_MODE_COUNT = 3; +const unsigned int FILTER_MODE_COUNT = 4; FilterMode::Type FILTER_MODE_OPTIONS[ FILTER_MODE_COUNT ] = - { FilterMode::DEFAULT, + { FilterMode::NONE, + FilterMode::DEFAULT, FilterMode::NEAREST, FilterMode::LINEAR }; -// These are the default sampling options - must match what is in GL -const FilterMode::Type DEFAULT_MINIFY = FilterMode::LINEAR; -const FilterMode::Type DEFAULT_MAGNIFY = FilterMode::LINEAR; - } // namespace /** @@ -63,6 +60,11 @@ void StoreFilterMode( unsigned int& options, FilterMode::Type mode, int bitShift // Start shifting from 1 as 0 is the unassigned state switch ( mode ) { + case FilterMode::NONE: + { + // Nothing to do + break; + } case FilterMode::DEFAULT: { options |= ( 1u << bitShift ); @@ -86,43 +88,17 @@ void StoreFilterMode( unsigned int& options, FilterMode::Type mode, int bitShift * @param[in] options A bitmask of filter values. * @param[in] mask The used to mask unwanted values. * @param[in] bitshift Used to shift to the correct part of options. - * @return The filter mode. + * @return Return the filter mode. */ -bool RetrieveFilterMode( unsigned int options, int mask, int bitShift, FilterMode::Type& filterModeOut ) +FilterMode::Type RetrieveFilterMode( unsigned int options, int mask, int bitShift ) { unsigned int index = options & mask; - if( index != 0 ) - { - index = ( index >> bitShift ) - 1; // Zero based index for array - - DALI_ASSERT_DEBUG( index < FILTER_MODE_COUNT ); - - filterModeOut = FILTER_MODE_OPTIONS[ index ]; - return true; - } - - return false; -} + index = ( index >> bitShift ); // Zero based index for array -unsigned int DefaultOptions() -{ - // Only initialise min filter as mag filter will use the system default - unsigned int bitfield = 0; - StoreFilterMode( bitfield, DEFAULT_MINIFY, MINIFY_BIT_SHIFT ); - return bitfield; -} + DALI_ASSERT_DEBUG( index < FILTER_MODE_COUNT ); -bool IsMinifyAssigned( unsigned int bitfield ) -{ - FilterMode::Type filterModeDummy = FilterMode::NEAREST; - return RetrieveFilterMode( bitfield, MASK_MINIFY_FILTER, MINIFY_BIT_SHIFT, filterModeDummy ); -} - -bool IsMagnifyAssigned( unsigned int bitfield ) -{ - FilterMode::Type filterModeDummy = FilterMode::NEAREST; - return RetrieveFilterMode( bitfield, MASK_MAGNIFY_FILTER, MAGNIFY_BIT_SHIFT, filterModeDummy ); + return FILTER_MODE_OPTIONS[ index ]; } unsigned int PackBitfield( FilterMode::Type minify, FilterMode::Type magnify ) @@ -135,16 +111,12 @@ unsigned int PackBitfield( FilterMode::Type minify, FilterMode::Type magnify ) FilterMode::Type GetMinifyFilterMode( unsigned int bitfield ) { - FilterMode::Type filterMode = FilterMode::NEAREST; - RetrieveFilterMode( bitfield, MASK_MINIFY_FILTER, MINIFY_BIT_SHIFT, filterMode ); - return filterMode; + return RetrieveFilterMode( bitfield, MASK_MINIFY_FILTER, MINIFY_BIT_SHIFT ); } FilterMode::Type GetMagnifyFilterMode( unsigned int bitfield ) { - FilterMode::Type filterMode = FilterMode::NEAREST; - RetrieveFilterMode( bitfield, MASK_MAGNIFY_FILTER, MAGNIFY_BIT_SHIFT, filterMode ); - return filterMode; + return RetrieveFilterMode( bitfield, MASK_MAGNIFY_FILTER, MAGNIFY_BIT_SHIFT ); } } // namespace ImageSampler diff --git a/dali/internal/common/image-sampler.h b/dali/internal/common/image-sampler.h index 2491b78..2ebd5a4 100644 --- a/dali/internal/common/image-sampler.h +++ b/dali/internal/common/image-sampler.h @@ -32,30 +32,6 @@ namespace Internal */ namespace ImageSampler { - - /** - * Determine if the minify component of the bitfield is assigned. - * - * @param[in] bitfield The packed sampler bitfield pattern. - * @return Return if the minify component is assigned or not. - */ - bool IsMinifyAssigned( unsigned int bitfield ); - - /** - * Determine if the magnify component of the bitfield is assigned. - * - * @param[in] bitfield The packed sampler bitfield pattern. - * @return Return if the magnify component is assigned or not. - */ - bool IsMagnifyAssigned( unsigned int bitfield ); - - /** - * @brief Return a sampler bitfield with default settings. - * - * @return Return the default sampler bit pattern. - */ - unsigned int DefaultOptions(); - /** * @brief Pack the filter mode into a bitfield. * diff --git a/dali/internal/render/gl-resources/texture.cpp b/dali/internal/render/gl-resources/texture.cpp index fb20c30..c0cac8e 100644 --- a/dali/internal/render/gl-resources/texture.cpp +++ b/dali/internal/render/gl-resources/texture.cpp @@ -34,13 +34,28 @@ namespace Dali namespace Internal { +namespace +{ + +// These match the GL specification +const GLint SYSTEM_MINIFY_DEFAULT = GL_NEAREST_MIPMAP_LINEAR; +const GLint SYSTEM_MAGNIFY_DEFAULT = GL_LINEAR; + +// These are the Dali defaults +const GLint DALI_MINIFY_DEFAULT = GL_LINEAR; +const GLint DALI_MAGNIFY_DEFAULT = GL_LINEAR; + +} // namespace + /** - * Convert a FilterMode to it's corresponding GL type. + * @brief Convert a FilterMode to it's corresponding GL type. * * @param[in] filterMode The FilterMode type. + * @param[in] defaultfilterMode The filter mode to use if filterMode is DEFAULT. + * @param[in] defaultSystemFilterMode The filter mode to use if filterMode is NONE. * @return Return the equivalent GL type. */ -GLint FilterModeToGL( FilterMode::Type filterMode ) +GLint FilterModeToGL( FilterMode::Type filterMode, GLint defaultfilterMode, GLint defaultSystemFilterMode ) { switch( filterMode ) { @@ -52,9 +67,13 @@ GLint FilterModeToGL( FilterMode::Type filterMode ) { return GL_LINEAR; } + case FilterMode::NONE: + { + return defaultSystemFilterMode; + } case FilterMode::DEFAULT: { - // Do nothing + return defaultfilterMode; } } @@ -241,32 +260,32 @@ void Texture::GetDefaultTextureCoordinates(UvRect& uv) const } +void Texture::ApplyTextureParameter( GLint filterType, FilterMode::Type currentFilterMode, FilterMode::Type newFilterMode, GLint daliDefault, GLint systemDefault ) +{ + GLint newFilterModeGL = FilterModeToGL( newFilterMode, daliDefault, systemDefault ); + GLint currentFilterModeGL = FilterModeToGL( currentFilterMode, daliDefault, systemDefault ); + + if( newFilterModeGL != currentFilterModeGL ) + { + mContext.TexParameteri( GL_TEXTURE_2D, filterType, newFilterModeGL ); + } +} + void Texture::ApplySampler( unsigned int samplerBitfield ) { if( mSamplerBitfield != samplerBitfield ) { - // Only set the tex parameters if they have been set in the sampler bitfield - FilterMode::Type filterMode = ImageSampler::GetMinifyFilterMode( samplerBitfield ); - if( filterMode != FilterMode::DEFAULT ) - { - mContext.TexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, FilterModeToGL( filterMode ) ); - } - else // We don't want to use the GL default - { - // Reset to system default option - mContext.TexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, FilterModeToGL( ImageSampler::GetMinifyFilterMode( ImageSampler::DefaultOptions() ) ) ); - } - - filterMode = ImageSampler::GetMagnifyFilterMode( samplerBitfield ); - if( filterMode != FilterMode::DEFAULT ) - { - mContext.TexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, FilterModeToGL( filterMode ) ); - } - else if( ImageSampler::IsMagnifyAssigned( mSamplerBitfield ) ) // We want to use the GL default - { - // Reset to system default option - mContext.TexParameteri( GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, FilterModeToGL( ImageSampler::GetMagnifyFilterMode( ImageSampler::DefaultOptions() ) ) ); - } + ApplyTextureParameter( GL_TEXTURE_MIN_FILTER, + ImageSampler::GetMinifyFilterMode( mSamplerBitfield ), + ImageSampler::GetMinifyFilterMode( samplerBitfield ), + DALI_MINIFY_DEFAULT, + SYSTEM_MINIFY_DEFAULT ); + + ApplyTextureParameter( GL_TEXTURE_MAG_FILTER, + ImageSampler::GetMagnifyFilterMode( mSamplerBitfield ), + ImageSampler::GetMagnifyFilterMode( samplerBitfield ), + DALI_MAGNIFY_DEFAULT, + SYSTEM_MAGNIFY_DEFAULT ); mSamplerBitfield = samplerBitfield; } diff --git a/dali/internal/render/gl-resources/texture.h b/dali/internal/render/gl-resources/texture.h index 42510c8..1ff8a7e 100644 --- a/dali/internal/render/gl-resources/texture.h +++ b/dali/internal/render/gl-resources/texture.h @@ -28,6 +28,7 @@ #include #include #include +#include namespace Dali { @@ -247,6 +248,17 @@ private: */ void GetDefaultTextureCoordinates(UvRect& uv) const; + /** + * @brief Apply the given texture parameters. + * + * @param[in] filterType Minification or magnification. + * @param[in] currentFilterMode The current filter mode. + * @param[in] newFilterMode The new filter mode. + * @param[in] daliDefault The default dali filter mode for the given filterType. + * @param[in] systemDefault The default system filter mode for the given filterType. + */ + void ApplyTextureParameter( GLint filterType, FilterMode::Type currentFilterMode, FilterMode::Type newFilterMode, GLint daliDefault, GLint systemDefault ); + protected: Context& mContext; ///< The GL Context diff --git a/dali/internal/render/renderers/scene-graph-renderer.cpp b/dali/internal/render/renderers/scene-graph-renderer.cpp index 339233d..573aec9 100644 --- a/dali/internal/render/renderers/scene-graph-renderer.cpp +++ b/dali/internal/render/renderers/scene-graph-renderer.cpp @@ -247,7 +247,7 @@ Renderer::Renderer( RenderDataProvider& dataprovider ) mTextureCache( NULL ), mShader( NULL ), - mSamplerBitfield( ImageSampler::DefaultOptions() ), + mSamplerBitfield( ImageSampler::PackBitfield( FilterMode::DEFAULT, FilterMode::DEFAULT ) ), mUseBlend( false ), mCullFaceMode( CullNone ) { diff --git a/dali/internal/render/shaders/shader.cpp b/dali/internal/render/shaders/shader.cpp index 2a969de..f60dd14 100644 --- a/dali/internal/render/shaders/shader.cpp +++ b/dali/internal/render/shaders/shader.cpp @@ -319,7 +319,7 @@ void Shader::SetUniforms( Context& context, // got effect texture, bind it to texture unit 1 mTexture->Bind( GL_TEXTURE_2D, GL_TEXTURE1 ); // Just apply the default sampling options for now - mTexture->ApplySampler( ImageSampler::DefaultOptions() ); + mTexture->ApplySampler( ImageSampler::PackBitfield( FilterMode::DEFAULT, FilterMode::DEFAULT ) ); // get effect sampler uniform const GLint loc = program.GetUniformLocation( Program::UNIFORM_EFFECT_SAMPLER ); diff --git a/dali/public-api/actors/sampling.h b/dali/public-api/actors/sampling.h index f30846c..222ebe9 100644 --- a/dali/public-api/actors/sampling.h +++ b/dali/public-api/actors/sampling.h @@ -32,7 +32,8 @@ namespace FilterMode */ enum Type { - DEFAULT, + NONE, ///< Use GL system defaults (minification NEAREST_MIPMAP_LINEAR, magnification LINEAR) + DEFAULT, ///< Use dali defaults (minification LINEAR, magnification LINEAR) NEAREST, ///< Filter nearest LINEAR ///< Filter linear }; -- 2.7.4