Clipping draw order fix 25/135025/3
authorTom Robinson <tom.robinson@samsung.com>
Tue, 20 Jun 2017 10:50:25 +0000 (11:50 +0100)
committerAdeel Kazmi <adeel.kazmi@samsung.com>
Wed, 21 Jun 2017 11:02:19 +0000 (12:02 +0100)
Change-Id: I845d2f140048abd3de5fb6c5f399e53f9fca7ff3

automated-tests/src/dali/utc-Dali-Actor.cpp
dali/internal/render/common/render-algorithms.cpp
dali/internal/update/manager/render-instruction-processor.cpp

index 34ef21c..0fa0347 100644 (file)
@@ -4218,6 +4218,98 @@ int UtcDaliActorPropertyClippingNestedChildren(void)
   END_TEST;
 }
 
+int UtcDaliActorPropertyClippingActorDrawOrder(void)
+{
+  // This test checks that a hierarchy of actors are drawn in the correct order when clipping is enabled.
+  tet_infoline( "Testing Actor::Property::CLIPPING_MODE draw order" );
+  TestApplication application;
+  TestGlAbstraction& glAbstraction = application.GetGlAbstraction();
+  TraceCallStack& enabledDisableTrace = glAbstraction.GetEnableDisableTrace();
+
+  /* We create a small tree of actors as follows:
+
+                           A
+                          / \
+     Clipping enabled -> B   D
+                         |   |
+                         C   E
+
+     The correct draw order is "ABCDE" (the same as if clipping was not enabled).
+  */
+  Actor actors[5];
+  for( int i = 0; i < 5; ++i )
+  {
+    BufferImage image = BufferImage::New( 16u, 16u );
+    Actor actor = CreateRenderableActor( image );
+
+    // Setup dimensions and position so actor is not skipped by culling.
+    actor.SetResizePolicy( ResizePolicy::FIXED, Dimension::ALL_DIMENSIONS );
+    actor.SetSize( 16.0f, 16.0f );
+
+    if( i == 0 )
+    {
+      actor.SetParentOrigin( ParentOrigin::CENTER );
+    }
+    else
+    {
+      float b = i > 2 ? 1.0f : -1.0f;
+      actor.SetParentOrigin( Vector3( 0.5 + ( 0.2f * b ), 0.8f, 0.8f ) );
+    }
+
+    actors[i] = actor;
+  }
+
+  // Enable clipping on the actor at the top of the left branch.
+  actors[1].SetProperty( Actor::Property::CLIPPING_MODE, ClippingMode::CLIP_CHILDREN );
+
+  // Build the scene graph.
+  Stage::GetCurrent().Add( actors[0] );
+
+  // Left branch:
+  actors[0].Add( actors[1] );
+  actors[1].Add( actors[2] );
+
+  // Right branch:
+  actors[0].Add( actors[3] );
+  actors[3].Add( actors[4] );
+
+  // Gather the call trace.
+  enabledDisableTrace.Reset();
+  enabledDisableTrace.Enable( true );
+  application.SendNotification();
+  application.Render();
+  enabledDisableTrace.Enable( false );
+
+  /* Check stencil is enabled and disabled again (as right-hand branch of tree is drawn).
+
+     Note: Correct enable call trace:    StackTrace: Index:0, Function:Enable, ParamList:3042 StackTrace: Index:1, Function:Enable, ParamList:2960 StackTrace: Index:2, Function:Disable, ParamList:2960
+           Incorrect enable call trace:  StackTrace: Index:0, Function:Enable, ParamList:3042 StackTrace: Index:1, Function:Enable, ParamList:2960
+  */
+  size_t startIndex = 0u;
+  DALI_TEST_CHECK( enabledDisableTrace.FindMethodAndParamsFromStartIndex( "Enable",  "3042", startIndex ) );
+  DALI_TEST_CHECK( enabledDisableTrace.FindMethodAndParamsFromStartIndex( "Enable",  "2960", startIndex ) ); // 2960 is GL_STENCIL_TEST
+  DALI_TEST_CHECK( enabledDisableTrace.FindMethodAndParamsFromStartIndex( "Disable", "2960", startIndex ) );
+
+  // Swap the clipping actor from top of left branch to top of right branch.
+  actors[1].SetProperty( Actor::Property::CLIPPING_MODE, ClippingMode::DISABLED );
+  actors[3].SetProperty( Actor::Property::CLIPPING_MODE, ClippingMode::CLIP_CHILDREN );
+
+  // Gather the call trace.
+  enabledDisableTrace.Reset();
+  enabledDisableTrace.Enable( true );
+  application.SendNotification();
+  application.Render();
+  enabledDisableTrace.Enable( false );
+
+  // Check stencil is enabled but NOT disabled again (as right-hand branch of tree is drawn).
+  // This proves the draw order has remained the same.
+  startIndex = 0u;
+  DALI_TEST_CHECK( enabledDisableTrace.FindMethodAndParamsFromStartIndex(  "Enable",  "2960", startIndex ) );
+  DALI_TEST_CHECK( !enabledDisableTrace.FindMethodAndParamsFromStartIndex( "Disable", "2960", startIndex ) );
+
+  END_TEST;
+}
+
 int UtcDaliActorPropertyClippingActorWithRendererOverride(void)
 {
   // This test checks that an actor with clipping will be ignored if overridden by the Renderer properties.
index 660a9a6..0c50ce4 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2017 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -123,12 +123,13 @@ inline void SetupClipping( const RenderItem& item, Context& context, uint32_t& l
       context.StencilMask( 0xff );
       context.Clear( GL_STENCIL_BUFFER_BIT, Context::CHECK_CACHED_VALUES );
     }
-    else if( ( currentStencilDepth < lastStencilDepth ) || ( clippingId != lastClippingId ) )
+    else if( ( currentStencilDepth < lastStencilDepth ) ||
+           ( ( currentStencilDepth == lastStencilDepth ) && ( clippingId > lastClippingId ) ) )
     {
       // The above if() statement tests if we need to clear some (not all) stencil bit-planes.
       // We need to do this if either of the following are true:
-      //   1) We traverse up the scene-graph to a previous stencil
-      //   2) We are at the same stencil depth but the clipping Id has changed.
+      //   1) We traverse up the scene-graph to a previous stencil depth
+      //   2) We are at the same stencil depth but the clipping Id has increased.
       //
       // This calculation takes the new depth to move to, and creates an inverse-mask of that number of consecutive bits.
       // This has the effect of clearing everything except the bit-planes up to (and including) our current depth.
index 3cc4027..edb03f0 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2017 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -59,8 +59,8 @@ namespace
  * @param[in] rhs Right hand side item
  * @return True if left item is greater than right
  */
-bool PartialCompareItems( const RenderInstructionProcessor::SortAttributes& lhs,
-                          const RenderInstructionProcessor::SortAttributes& rhs )
+inline bool PartialCompareItems( const RenderInstructionProcessor::SortAttributes& lhs,
+                                 const RenderInstructionProcessor::SortAttributes& rhs )
 {
   if( lhs.shader == rhs.shader )
   {
@@ -86,30 +86,12 @@ bool CompareItems( const RenderInstructionProcessor::SortAttributes& lhs, const
   // encapsulates the same data (e.g. the middle-order bits of the ptrs).
   if( lhs.renderItem->mDepthIndex == rhs.renderItem->mDepthIndex )
   {
-    return PartialCompareItems(lhs, rhs);
+    return PartialCompareItems( lhs, rhs );
   }
   return lhs.renderItem->mDepthIndex < rhs.renderItem->mDepthIndex;
 }
 
 /**
- * Function which sorts render items by clipping hierarchy, then depth index and instance
- * ptrs of shader/geometry/material.
- * @param[in] lhs Left hand side item
- * @param[in] rhs Right hand side item
- * @return True if left item is greater than right
- */
-bool CompareItemsWithClipping( const RenderInstructionProcessor::SortAttributes& lhs, const RenderInstructionProcessor::SortAttributes& rhs )
-{
-  // Items must be sorted in order of clipping first, otherwise incorrect clipping regions could be used.
-  if( lhs.renderItem->mNode->mClippingSortModifier == rhs.renderItem->mNode->mClippingSortModifier )
-  {
-    return CompareItems( lhs, rhs );
-  }
-
-  return lhs.renderItem->mNode->mClippingSortModifier < rhs.renderItem->mNode->mClippingSortModifier;
-}
-
-/**
  * Function which sorts the render items by Z function, then
  * by instance ptrs of shader / geometry / material.
  * @param[in] lhs Left hand side item
@@ -118,7 +100,7 @@ bool CompareItemsWithClipping( const RenderInstructionProcessor::SortAttributes&
  */
 bool CompareItems3D( const RenderInstructionProcessor::SortAttributes& lhs, const RenderInstructionProcessor::SortAttributes& rhs )
 {
-  bool lhsIsOpaque = lhs.renderItem->mIsOpaque;
+  const bool lhsIsOpaque = lhs.renderItem->mIsOpaque;
   if( lhsIsOpaque == rhs.renderItem->mIsOpaque )
   {
     if( lhsIsOpaque )
@@ -307,10 +289,9 @@ RenderInstructionProcessor::RenderInstructionProcessor()
 : mSortingHelper()
 {
   // Set up a container of comparators for fast run-time selection.
-  mSortComparitors.Reserve( 4u );
+  mSortComparitors.Reserve( 3u );
 
   mSortComparitors.PushBack( CompareItems );
-  mSortComparitors.PushBack( CompareItemsWithClipping );
   mSortComparitors.PushBack( CompareItems3D );
   mSortComparitors.PushBack( CompareItems3DWithClipping );
 }
@@ -378,12 +359,11 @@ inline void RenderInstructionProcessor::SortRenderItems( BufferIndex bufferIndex
     }
   }
 
-  // Here we detemine which comparitor (of the 4) to use.
-  // The comparitors work like a bitmask.
-  //   1 << 0  is added to select a clipping comparitor.
-  //   1 << 1  is added for 3D comparitors.
-  const unsigned int comparitorIndex = ( respectClippingOrder                         ? ( 1u << 0u ) : 0u ) |
-                                       ( layer.GetBehavior() == Dali::Layer::LAYER_3D ? ( 1u << 1u ) : 0u );
+  // Here we determine which comparitor (of the 3) to use.
+  //   0 is LAYER_UI
+  //   1 is LAYER_3D
+  //   2 is LAYER_3D + Clipping
+  const unsigned int comparitorIndex = layer.GetBehavior() == Dali::Layer::LAYER_3D ? respectClippingOrder ? 2u : 1u : 0u;
 
   std::stable_sort( mSortingHelper.begin(), mSortingHelper.end(), mSortComparitors[ comparitorIndex ] );