Prevent renderers from drawing with unbound textures 68/51568/8
authorDavid Steele <david.steele@partner.samsung.com>
Tue, 10 Nov 2015 16:56:08 +0000 (16:56 +0000)
committerDavid Steele <david.steele@samsung.com>
Wed, 9 Mar 2016 15:24:53 +0000 (15:24 +0000)
Changed Texture::Bind to return success/failure instead of Created/NotCreated

If the texture is not created, e.g. a NativeTexture calling out to
NativeImageInterface::GlExtensionCreate() fails for some reason, then
the Bind call will now return false to indicate that it failed.

The renderers now check the result of the Bind call, and do not draw if it is
false. This prevents the screen showing black rectangles for unbound textures.

Change-Id: I8e4c81f43a8b73b05cccdbf7799cf5ef0c8722c4
Signed-off-by: David Steele <david.steele@samsung.com>
automated-tests/src/dali/dali-test-suite-utils/test-native-image.cpp
automated-tests/src/dali/dali-test-suite-utils/test-native-image.h
automated-tests/src/dali/utc-Dali-NativeImage.cpp
dali/internal/render/gl-resources/native-texture.cpp
dali/internal/render/gl-resources/texture-cache.cpp
dali/internal/render/gl-resources/texture-cache.h
dali/internal/render/gl-resources/texture.cpp
dali/internal/render/gl-resources/texture.h
dali/internal/render/renderers/render-renderer.cpp
dali/internal/render/renderers/render-renderer.h

index d8ad2ba..135476e 100644 (file)
@@ -29,7 +29,7 @@ TestNativeImagePointer TestNativeImage::New(int width, int height)
 }
 
 TestNativeImage::TestNativeImage(int width, int height)
-: mWidth(width), mHeight(height), mExtensionCreateCalls(0), mExtensionDestroyCalls(0), mTargetTextureCalls(0)
+: mWidth(width), mHeight(height), mExtensionCreateCalls(0), mExtensionDestroyCalls(0), mTargetTextureCalls(0),createResult(true)
 {
   mExtension = new TestNativeImageExtension();
 }
index 8dc6431..f361280 100644 (file)
@@ -40,7 +40,8 @@ class DALI_IMPORT_API TestNativeImage : public Dali::NativeImageInterface
 public:
   static TestNativeImagePointer New(int width, int height);
 
-  inline virtual bool GlExtensionCreate() { ++mExtensionCreateCalls; return true;};
+  inline void SetGlExtensionCreateResult(bool result){ createResult = result;}
+  inline virtual bool GlExtensionCreate() { ++mExtensionCreateCalls; return createResult;};
   inline virtual void GlExtensionDestroy() { ++mExtensionDestroyCalls; };
   inline virtual GLenum TargetTexture() { ++mTargetTextureCalls; return 1;};
   inline virtual void PrepareTexture() {};
@@ -59,6 +60,8 @@ public:
   int mExtensionCreateCalls;
   int mExtensionDestroyCalls;
   int mTargetTextureCalls;
+
+  bool createResult;
   TestNativeImageExtension* mExtension;
 };
 
index 3dc7568..e44c393 100644 (file)
@@ -234,3 +234,54 @@ int UtcDaliNativeImageGetCustomSamplerTypenameP(void)
   DALI_TEST_EQUALS( nativeImage.GetCustomSamplerTypename(), samplerTypename, TEST_LOCATION );
   END_TEST;
 }
+
+
+
+int UtcDaliNativeImageTestCreationFailure(void)
+{
+  TestApplication application;
+  TestNativeImagePointer nativeImageInterface = TestNativeImage::New( 16, 16 );
+  NativeImage nativeImage = NativeImage::New( *(nativeImageInterface.Get()) );
+
+  tet_printf("Test what happens when GlExtensionCreate is called, and returns false to indicate an error\n");
+
+  nativeImageInterface->SetGlExtensionCreateResult( false );
+
+  ImageActor imageActor = ImageActor::New( nativeImage );
+  imageActor.SetParentOrigin( ParentOrigin::CENTER );
+  Stage::GetCurrent().Add( imageActor );
+
+  TestGlAbstraction& gl = application.GetGlAbstraction();
+  TraceCallStack& textureTrace = gl.GetTextureTrace();
+  textureTrace.Reset();
+  textureTrace.Enable(true);
+
+  TraceCallStack& drawTrace = gl.GetDrawTrace();
+  drawTrace.Reset();
+  drawTrace.Enable(true);
+
+  application.SendNotification();
+  application.Render();
+
+  // Test that nothing was rendered
+  DALI_TEST_EQUALS( nativeImageInterface->mExtensionCreateCalls, 1, TEST_LOCATION );
+  DALI_TEST_EQUALS( nativeImageInterface->mTargetTextureCalls, 0, TEST_LOCATION );
+  DALI_TEST_EQUALS( textureTrace.FindMethod("BindTexture"), false, TEST_LOCATION );
+  DALI_TEST_EQUALS( drawTrace.FindMethod("DrawElements") || drawTrace.FindMethod("DrawArrays"), false, TEST_LOCATION );
+
+  textureTrace.Reset();
+  drawTrace.Reset();
+
+  nativeImageInterface->SetGlExtensionCreateResult( true );
+  imageActor.SetPosition( 0, 0, 1 );
+  application.SendNotification();
+  application.Render();
+
+  // This time around, the bind and draw should occur following the call to nativeImage->GlExtensionCreate.
+  DALI_TEST_EQUALS( nativeImageInterface->mExtensionCreateCalls, 2, TEST_LOCATION );
+  DALI_TEST_EQUALS( nativeImageInterface->mTargetTextureCalls, 1, TEST_LOCATION );
+  DALI_TEST_EQUALS( textureTrace.FindMethod("BindTexture"), true, TEST_LOCATION );
+  DALI_TEST_EQUALS( drawTrace.FindMethod("DrawElements") || drawTrace.FindMethod("DrawArrays"), true, TEST_LOCATION );
+
+  END_TEST;
+}
index f557f97..b268374 100644 (file)
@@ -44,27 +44,30 @@ NativeTexture::NativeTexture(NativeImageInterface* nativeImg, Context& context)
 NativeTexture::~NativeTexture()
 {
   DALI_LOG_INFO (Debug::Filter::gImage, Debug::General, "NativeTexture destroyed\n");
+
   // GlCleanup() should already have been called by TextureCache ensuring the resource is destroyed
   // on the render thread. (And avoiding a potentially problematic virtual call in the destructor)
 }
 
-bool NativeTexture::Bind(GLenum target, TextureUnit textureunit )
+bool NativeTexture::Bind( GLenum target, TextureUnit textureunit )
 {
-  bool created = false;
+  bool result = true;
 
-  if( mId==0 )
+  if( mId == 0 )
   {
-    CreateGlTexture();
-    created = true;
+    result = CreateGlTexture();
   }
 
-  // Bind the texture id
-  mContext.ActiveTexture( textureunit );
-  mContext.Bind2dTexture( mId );
+  if( result )
+  {
+    // Bind the texture id
+    mContext.ActiveTexture( textureunit );
+    mContext.Bind2dTexture(mId);
 
-  mNativeImage->PrepareTexture();
+    mNativeImage->PrepareTexture();
+  }
 
-  return created;
+  return result;
 }
 
 bool NativeTexture::IsFullyOpaque() const
@@ -79,7 +82,7 @@ bool NativeTexture::HasAlphaChannel() const
 
 bool NativeTexture::CreateGlTexture()
 {
-  if ( mId != 0 )
+  if( mId != 0 )
   {
     DALI_LOG_INFO( Debug::Filter::gImage, Debug::General, "GL texture creation duplicate for GL id: %d\n", &mId );
     return true;
index 1cb5f04..856291e 100644 (file)
@@ -293,14 +293,19 @@ void TextureCache::DiscardTexture( ResourceId id )
   }
 }
 
-void TextureCache::BindTexture( Texture *texture, ResourceId id, GLenum target, TextureUnit textureunit )
+bool TextureCache::BindTexture( Texture *texture, ResourceId id, GLenum target, TextureUnit textureunit )
 {
-  bool created = texture->Bind(target, textureunit);
+  unsigned int glTextureId = texture->GetTextureId();
+
+  bool success = texture->Bind(target, textureunit);
+  bool created = ( glTextureId == 0 ) && ( texture->GetTextureId() != 0 );
+
   if( created && texture->UpdateOnCreate() ) // i.e. the pixel data was sent to GL
   {
     ResourceId ppRequest( id );
     mTextureUploadedDispatcher.DispatchTextureUploaded(ppRequest);
   }
+  return success;
 }
 
 Texture* TextureCache::GetTexture(ResourceId id)
index 558adf1..c48a843 100644 (file)
@@ -195,8 +195,10 @@ public:
    * @param[in] id Resource id of texture
    * @param[in] target (e.g. GL_TEXTURE_2D)
    * @param[in] textureunit to use
+   *
+   * @return true if the bind succeeded, false if either the create or bind failed.
    */
-  void BindTexture( Texture* texture, ResourceId id, GLenum target, TextureUnit textureunit );
+  bool BindTexture( Texture* texture, ResourceId id, GLenum target, TextureUnit textureunit );
 
   /**
    * Get the texture associated with the resource ID
index 88d9204..36b5fae 100644 (file)
@@ -167,20 +167,17 @@ bool Texture::Bind(GLenum target, TextureUnit textureunit )
 {
   // This is the only supported type at the moment
   DALI_ASSERT_DEBUG( target == GL_TEXTURE_2D );
-  bool created = false;
+  bool result = true;
 
   if( mId == 0 )
   {
-    if( CreateGlTexture() )
-    {
-      created = true;
-    }
+    result = CreateGlTexture();
   }
 
   // Bind the texture id
   mContext.BindTextureForUnit(textureunit, mId );
 
-  return created;
+  return result;
 }
 
 void Texture::GlContextDestroyed()
index cddf656..1d18db0 100644 (file)
@@ -106,8 +106,8 @@ public:
    *
    * @param target (e.g. GL_TEXTURE_2D)
    * @param textureunit to bind to
-   * @return True if the opengl texture was created, false if there was already a texture
-   * or no texture could be created yet ( e.g. no bitmap data after context loss )
+   *
+   * @return true if the bind succeeded, or false if either the create or bind failed
    */
   virtual bool Bind(GLenum target, TextureUnit textureunit);
 
index cdd8752..99c7999 100644 (file)
@@ -352,54 +352,59 @@ void Renderer::SetUniformFromProperty( BufferIndex bufferIndex, Program& program
   }
 }
 
-void Renderer::BindTextures( SceneGraph::TextureCache& textureCache, Program& program )
+bool Renderer::BindTextures( SceneGraph::TextureCache& textureCache, Program& program )
 {
   int textureUnit = 0;
+  bool result = true;
 
   std::vector<Render::Texture>& textures( mRenderDataProvider->GetTextures() );
-  for( size_t i(0); i<textures.size(); ++i )
+  for( size_t i(0); result && i<textures.size(); ++i )
   {
     ResourceId textureId = textures[i].GetTextureId();
     Internal::Texture* texture = textureCache.GetTexture( textureId );
     if( texture )
     {
-      textureCache.BindTexture( texture, textureId, GL_TEXTURE_2D, (TextureUnit)textureUnit );
+      result = textureCache.BindTexture( texture, textureId, GL_TEXTURE_2D, (TextureUnit)textureUnit );
 
-      Render::Texture& textureMapping = textures[i];
-      // Set sampler uniform location for the texture
-      int32_t uniqueIndex = textureMapping.GetUniformUniqueIndex();
-      if( Render::Texture::NOT_INITIALIZED == uniqueIndex )
+      if( result )
       {
-        uniqueIndex = mUniformNameCache->GetSamplerUniformUniqueIndex( textureMapping.GetUniformName() );
-        textureMapping.SetUniformUniqueIndex( uniqueIndex );
-      }
-      GLint uniformLocation = program.GetSamplerUniformLocation( uniqueIndex, textureMapping.GetUniformName() );
-      if( Program::UNIFORM_UNKNOWN != uniformLocation )
-      {
-        program.SetUniform1i( uniformLocation, textureUnit );
-      }
+        Render::Texture& textureMapping = textures[i];
+        // Set sampler uniform location for the texture
+        int32_t uniqueIndex = textureMapping.GetUniformUniqueIndex();
+        if( Render::Texture::NOT_INITIALIZED == uniqueIndex )
+        {
+          uniqueIndex = mUniformNameCache->GetSamplerUniformUniqueIndex( textureMapping.GetUniformName() );
+          textureMapping.SetUniformUniqueIndex( uniqueIndex );
+        }
+        GLint uniformLocation = program.GetSamplerUniformLocation( uniqueIndex, textureMapping.GetUniformName() );
+        if( Program::UNIFORM_UNKNOWN != uniformLocation )
+        {
+          program.SetUniform1i( uniformLocation, textureUnit );
+        }
 
-      unsigned int samplerBitfield(0);
-      const Render::Sampler* sampler( textureMapping.GetSampler() );
-      if( sampler )
-      {
-        samplerBitfield = ImageSampler::PackBitfield(
-          static_cast< FilterMode::Type >(sampler->GetMinifyFilterMode()),
-          static_cast< FilterMode::Type >(sampler->GetMagnifyFilterMode()),
-          static_cast< WrapMode::Type >(sampler->GetUWrapMode()),
-          static_cast< WrapMode::Type >(sampler->GetVWrapMode())
-          );
-      }
-      else
-      {
-        samplerBitfield = ImageSampler::DEFAULT_BITFIELD;
-      }
+        unsigned int samplerBitfield(0);
+        const Render::Sampler* sampler( textureMapping.GetSampler() );
+        if( sampler )
+        {
+          samplerBitfield = ImageSampler::PackBitfield(
+            static_cast< FilterMode::Type >(sampler->GetMinifyFilterMode()),
+            static_cast< FilterMode::Type >(sampler->GetMagnifyFilterMode()),
+            static_cast< WrapMode::Type >(sampler->GetUWrapMode()),
+            static_cast< WrapMode::Type >(sampler->GetVWrapMode())
+                                                       );
+        }
+        else
+        {
+          samplerBitfield = ImageSampler::DEFAULT_BITFIELD;
+        }
 
-      texture->ApplySampler( (TextureUnit)textureUnit, samplerBitfield );
+        texture->ApplySampler( (TextureUnit)textureUnit, samplerBitfield );
 
-      ++textureUnit;
+        ++textureUnit;
+      }
     }
   }
+  return result;
 }
 
 void Renderer::SetFaceCullingMode( Dali::Renderer::FaceCullingMode mode )
@@ -460,37 +465,38 @@ void Renderer::Render( Context& context,
   // Take the program into use so we can send uniforms to it
   program->Use();
 
-  // set projection and view matrix if program has not yet received them yet this frame
-  SetMatrices( *program, node.GetModelMatrix( bufferIndex ), viewMatrix, projectionMatrix, modelViewMatrix );
-
-  // set color uniform
-  GLint loc = program->GetUniformLocation( Program::UNIFORM_COLOR );
-  if( Program::UNIFORM_UNKNOWN != loc )
+  if( DALI_LIKELY( BindTextures( textureCache, *program ) ) )
   {
-    const Vector4& color = node.GetRenderColor( bufferIndex );
-    if( mPremultipledAlphaEnabled )
-    {
-      program->SetUniform4f( loc, color.r*color.a, color.g*color.a, color.b*color.a, color.a );
-    }
-    else
+    // Only set up and draw if we have textures and they are all valid
+
+    // set projection and view matrix if program has not yet received them yet this frame
+    SetMatrices( *program, node.GetModelMatrix( bufferIndex ), viewMatrix, projectionMatrix, modelViewMatrix );
+
+    // set color uniform
+    GLint loc = program->GetUniformLocation( Program::UNIFORM_COLOR );
+    if( Program::UNIFORM_UNKNOWN != loc )
     {
-      program->SetUniform4f( loc, color.r, color.g, color.b, color.a );
+      const Vector4& color = node.GetRenderColor( bufferIndex );
+      if( mPremultipledAlphaEnabled )
+      {
+        program->SetUniform4f( loc, color.r*color.a, color.g*color.a, color.b*color.a, color.a );
+      }
+      else
+      {
+        program->SetUniform4f( loc, color.r, color.g, color.b, color.a );
+      }
     }
-  }
 
-  //Bind textures
-  BindTextures( textureCache, *program );
+    SetUniforms( bufferIndex, node, *program );
 
-  //Set uniforms
-  SetUniforms( bufferIndex, node, *program );
+    if( mUpdateAttributesLocation || mRenderGeometry->AttributesChanged() )
+    {
+      mRenderGeometry->GetAttributeLocationFromProgram( mAttributesLocation, *program, bufferIndex );
+      mUpdateAttributesLocation = false;
+    }
 
-  if( mUpdateAttributesLocation || mRenderGeometry->AttributesChanged() )
-  {
-    mRenderGeometry->GetAttributeLocationFromProgram( mAttributesLocation, *program, bufferIndex );
-    mUpdateAttributesLocation = false;
+    mRenderGeometry->UploadAndDraw( context, bufferIndex, mAttributesLocation );
   }
-
-  mRenderGeometry->UploadAndDraw( context, bufferIndex, mAttributesLocation );
 }
 
 void Renderer::SetSortAttributes( BufferIndex bufferIndex, SceneGraph::RendererWithSortAttributes& sortAttributes ) const
index 611ee9b..18ccc0c 100644 (file)
@@ -228,8 +228,9 @@ private:
    * Bind the material textures in the samplers and setup the samplers
    * @param[in] textureCache The texture cache
    * @param[in] program The shader program
+   * @return False if create or bind failed, true if success.
    */
-  void BindTextures( SceneGraph::TextureCache& textureCache, Program& program );
+  bool BindTextures( SceneGraph::TextureCache& textureCache, Program& program );
 
 public: