tdf#96083 slide transitions wrong with appearing shapes

When using appearing animations on shapes, some of the slide transitions
incorrectly don't show these shapes. Same for disappearing shapes - both
states are wrong during slide transition time. Fix slide bitmap
generator to take final slide states into account.

Change-Id: Iea0e576009a109c7f44a7a6498b0ee5b2c4791c5
Reviewed-on: https://gerrit.libreoffice.org/20199
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Thorsten Behrens <Thorsten.Behrens@CIB.de>

3 files changed, 79 insertions, 31 deletions

diff --git a/slideshow/source/engine/slide/slideimpl.cxx b/slideshow/source/engine/slide/slideimpl.cxx
index 690977ed5c5d..45c98551b0e6 100644
--- a/slideshow/source/engine/slide/slideimpl.cxx
+++ b/slideshow/source/engine/slide/slideimpl.cxx
@@ -157,6 +157,11 @@ private:
     /// Set all Shapes to their initial attributes for slideshow
     bool applyInitialShapeAttributes( const css::uno::Reference< css::animations::XAnimationNode >& xRootAnimationNode );
 
+    /// Set shapes to attributes corresponding to initial or final state of slide
+    void applyShapeAttributes(
+        const css::uno::Reference< css::animations::XAnimationNode >& xRootAnimationNode,
+        bool bInitial) const;
+
     /// Renders current slide content to bitmap
     SlideBitmapSharedPtr createCurrentSlideBitmap(
         const UnoViewSharedPtr& rView,
@@ -272,8 +277,11 @@ private:
     /// When true, show() was called. Slide hidden oherwise.
     bool                                                mbActive;
 
-    ///When true, enablePaintOverlay was called and mbUserPaintOverlay = true
+    /// When true, enablePaintOverlay was called and mbUserPaintOverlay = true
     bool                                                mbPaintOverlayActive;
+
+    /// When true, final state attributes are already applied to shapes
+    bool                                                mbFinalStateApplied;
 };
 
 
@@ -369,7 +377,8 @@ SlideImpl::SlideImpl( const uno::Reference< drawing::XDrawPage >&           xDra
     mbHaveAnimations( false ),
     mbMainSequenceFound( false ),
     mbActive( false ),
-    mbPaintOverlayActive( false )
+    mbPaintOverlayActive( false ),
+    mbFinalStateApplied( false )
 {
     // clone already existing views for slide bitmaps
     for( const auto& rView : rViewContainer )
@@ -523,8 +532,6 @@ void SlideImpl::hide()
     // vanish from view
     resetCursor();
     mbActive = false;
-
-
 }
 
 basegfx::B2ISize SlideImpl::getSlideSize() const
@@ -680,6 +687,14 @@ SlideBitmapSharedPtr SlideImpl::createCurrentSlideBitmap( const UnoViewSharedPtr
     ENSURE_OR_THROW( mbShowLoaded,
                       "SlideImpl::createCurrentSlideBitmap(): No show loaded" );
 
+    // tdf#96083 ensure end state settings are applied to shapes once when bitmap gets re-rendered
+    // in that state
+    if(!mbFinalStateApplied && FINAL_STATE == meAnimationState && mxRootNode.is())
+    {
+        const_cast< SlideImpl* >(this)->mbFinalStateApplied = true;
+        applyShapeAttributes(mxRootNode, false);
+    }
+
     ::cppcanvas::CanvasSharedPtr pCanvas( rView->getCanvas() );
 
     // create a bitmap of appropriate size
@@ -887,22 +902,12 @@ void SlideImpl::startIntrinsicAnimations()
     mpSubsettableShapeManager->notifyIntrinsicAnimationsEnabled();
 }
 
-bool SlideImpl::applyInitialShapeAttributes(
-    const uno::Reference< animations::XAnimationNode >& xRootAnimationNode )
+void SlideImpl::applyShapeAttributes(
+    const css::uno::Reference< css::animations::XAnimationNode >& xRootAnimationNode,
+    bool bInitial) const
 {
-    if( !implPrefetchShow() )
-        return false;
-
-    if( !xRootAnimationNode.is() )
-    {
-        meAnimationState = INITIAL_STATE;
-
-        return true; // no animations - no attributes to apply -
-                     // succeeded
-    }
-
     uno::Sequence< animations::TargetProperties > aProps(
-        TargetPropertiesCreator::createInitialTargetProperties( xRootAnimationNode ) );
+        TargetPropertiesCreator::createTargetProperties( xRootAnimationNode, bInitial ) );
 
     // apply extracted values to our shapes
     const ::std::size_t nSize( aProps.getLength() );
@@ -994,6 +999,23 @@ bool SlideImpl::applyInitialShapeAttributes(
             }
         }
     }
+}
+
+bool SlideImpl::applyInitialShapeAttributes(
+    const uno::Reference< animations::XAnimationNode >& xRootAnimationNode )
+{
+    if( !implPrefetchShow() )
+        return false;
+
+    if( !xRootAnimationNode.is() )
+    {
+        meAnimationState = INITIAL_STATE;
+
+        return true; // no animations - no attributes to apply -
+                     // succeeded
+    }
+
+    applyShapeAttributes(xRootAnimationNode, true);
 
     meAnimationState = INITIAL_STATE;
 
diff --git a/slideshow/source/engine/slide/targetpropertiescreator.cxx b/slideshow/source/engine/slide/targetpropertiescreator.cxx
index 927ee8fecf7c..8974acdc4b17 100644
--- a/slideshow/source/engine/slide/targetpropertiescreator.cxx
+++ b/slideshow/source/engine/slide/targetpropertiescreator.cxx
@@ -95,19 +95,24 @@ namespace internal
         class NodeFunctor
         {
         public:
-            explicit NodeFunctor( XShapeHash& rShapeHash ) :
-                mrShapeHash( rShapeHash ),
+            explicit NodeFunctor(
+                XShapeHash& rShapeHash,
+                bool bInitial )
+            :   mrShapeHash( rShapeHash ),
                 mxTargetShape(),
-                mnParagraphIndex( -1 )
+                mnParagraphIndex( -1 ),
+                mbInitial( bInitial)
             {
             }
 
             NodeFunctor( XShapeHash&                                rShapeHash,
                          const uno::Reference< drawing::XShape >&   rTargetShape,
-                         sal_Int16                                  nParagraphIndex ) :
+                         sal_Int16                                  nParagraphIndex,
+                         bool                                       bInitial) :
                 mrShapeHash( rShapeHash ),
                 mxTargetShape( rTargetShape ),
-                mnParagraphIndex( nParagraphIndex )
+                mnParagraphIndex( nParagraphIndex ),
+                mbInitial( bInitial )
             {
             }
 
@@ -169,9 +174,11 @@ namespace internal
                         // FALLTHROUGH intended
                     case animations::AnimationNodeType::SEQ:
                     {
+                        /// forward bInitial
                         NodeFunctor aFunctor( mrShapeHash,
                                               xTargetShape,
-                                              nParagraphIndex );
+                                              nParagraphIndex,
+                                              mbInitial );
                         if( !for_each_childNode( xNode, aFunctor ) )
                         {
                             OSL_FAIL( "AnimCore: NodeFunctor::operator(): child node iteration failed, "
@@ -250,8 +257,11 @@ namespace internal
 
                         // check whether we already have an entry for
                         // this target (we only take the first set
-                        // effect for every shape)
-                        if( mrShapeHash.find( aTarget ) != mrShapeHash.end() )
+                        // effect for every shape) - but keep going if
+                        // we're requested the final state (which
+                        // eventually gets overwritten in the
+                        // unordered list, see tdf#96083)
+                        if( mbInitial && mrShapeHash.find( aTarget ) != mrShapeHash.end() )
                             break; // already an entry in existence for given XShape
 
                         // if this is an appear effect, hide shape
@@ -286,6 +296,13 @@ namespace internal
                                 }
                             }
                         }
+
+                        // if initial anim sets shape visible, set it
+                        // to invisible. If we're asked for the final
+                        // state, don't do anything obviously
+                        if(mbInitial)
+                            bVisible = !bVisible;
+
                         // target is set the 'visible' value,
                         // so we should record the opposite value
                         mrShapeHash.insert(
@@ -296,7 +313,7 @@ namespace internal
                                             beans::NamedValue(
                                                 //xAnimateNode->getAttributeName(),
                                                 OUString("visibility"),
-                                                uno::makeAny( !bVisible ) ) ) ) );
+                                                uno::makeAny( bVisible ) ) ) ) );
                     break;
                     }
                 }
@@ -306,19 +323,25 @@ namespace internal
             XShapeHash&                         mrShapeHash;
             uno::Reference< drawing::XShape >   mxTargetShape;
             sal_Int16                           mnParagraphIndex;
+
+            // get initial or filal state
+            bool                                mbInitial;
         };
     }
 
-    uno::Sequence< animations::TargetProperties > SAL_CALL TargetPropertiesCreator::createInitialTargetProperties
+    uno::Sequence< animations::TargetProperties > SAL_CALL TargetPropertiesCreator::createTargetProperties
         (
-            const uno::Reference< animations::XAnimationNode >& xRootNode
+            const uno::Reference< animations::XAnimationNode >& xRootNode,
+            bool bInitial
         ) //throw (uno::RuntimeException, std::exception)
     {
         // scan all nodes for visibility changes, and record first
         // 'visibility=true' for each shape
         XShapeHash aShapeHash( 101 );
 
-        NodeFunctor aFunctor( aShapeHash );
+        NodeFunctor aFunctor(
+            aShapeHash,
+            bInitial );
 
         // TODO(F1): Maybe limit functor application to main sequence
         // alone (CL said something that shape visibility is only
diff --git a/slideshow/source/engine/slide/targetpropertiescreator.hxx b/slideshow/source/engine/slide/targetpropertiescreator.hxx
index 15466a517f9d..24c7fafea63a 100644
--- a/slideshow/source/engine/slide/targetpropertiescreator.hxx
+++ b/slideshow/source/engine/slide/targetpropertiescreator.hxx
@@ -34,7 +34,10 @@ namespace slideshow
     {
         namespace TargetPropertiesCreator
         {
-            uno::Sequence< animations::TargetProperties > SAL_CALL createInitialTargetProperties( const uno::Reference< animations::XAnimationNode >& rootNode );
+            /// Generate shape property list - set bInitial to true for initial slide state
+            uno::Sequence< animations::TargetProperties > SAL_CALL createTargetProperties(
+                const uno::Reference< animations::XAnimationNode >& rootNode,
+                bool bInitial );
         }
 
     } // namespace internal