Image filter bug fix 91/25391/2
authorKingsley Stephens <k.stephens@partner.samsung.com>
Mon, 4 Aug 2014 10:18:41 +0000 (11:18 +0100)
committerKingsley Stephens <k.stephens@partner.samsung.com>
Tue, 5 Aug 2014 13:08:20 +0000 (14:08 +0100)
[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

automated-tests/src/dali-internal/utc-Dali-Internal-Material.cpp
automated-tests/src/dali/utc-Dali-RenderableActor.cpp
dali/internal/common/image-sampler.cpp
dali/internal/common/image-sampler.h
dali/internal/render/gl-resources/texture.cpp
dali/internal/render/gl-resources/texture.h
dali/internal/render/renderers/scene-graph-renderer.cpp
dali/internal/render/shaders/shader.cpp
dali/public-api/actors/sampling.h

index 920dc65..4e3fe95 100644 (file)
@@ -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;
 }
index ee3075b..5eaea36 100644 (file)
@@ -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);
 
   /**************************************************************/
 
index 549f4bf..9c2ccc3 100644 (file)
@@ -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
index 2491b78..2ebd5a4 100644 (file)
@@ -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.
    *
index fb20c30..c0cac8e 100644 (file)
@@ -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;
   }
index 42510c8..1ff8a7e 100644 (file)
@@ -28,6 +28,7 @@
 #include <dali/public-api/images/pixel.h>
 #include <dali/public-api/images/native-image.h>
 #include <dali/public-api/math/rect.h>
+#include <dali/public-api/actors/sampling.h>
 
 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
index 339233d..573aec9 100644 (file)
@@ -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 )
 {
index 2a969de..f60dd14 100644 (file)
@@ -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 );
index f30846c..222ebe9 100644 (file)
@@ -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
 };