[Gnash-commit] gnash ChangeLog server/sprite_instance.cpp serv...

[Sandro Santilli]

CVSROOT:        /sources/gnash
Module name:    gnash
Changes by:     Sandro Santilli <strk>  07/02/16 11:09:41

Modified files:
        .              : ChangeLog 
        server         : sprite_instance.cpp sprite_instance.h 
        testsuite/misc-ming.all: action_execution_order_test.c 

Log message:
                * server/sprite_instance.{cpp,h}: Fixed (hopefully)
                  the action execution order. Implementation is far
                  from elegant, but gives the idea of how it should work,
                  and actually passed our test for it, w/out breaking
                  anything else.
                * testsuite/misc-ming.all/action_execution_order_test.c:
                  Add an additional test to verify that function definitions
                  are not specially handled as it comes to action execution
                  order. Don't expect failures anymore.

CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&r1=1.2375&r2=1.2376
http://cvs.savannah.gnu.org/viewcvs/gnash/server/sprite_instance.cpp?cvsroot=gnash&r1=1.163&r2=1.164
http://cvs.savannah.gnu.org/viewcvs/gnash/server/sprite_instance.h?cvsroot=gnash&r1=1.67&r2=1.68
http://cvs.savannah.gnu.org/viewcvs/gnash/testsuite/misc-ming.all/action_execution_order_test.c?cvsroot=gnash&r1=1.5&r2=1.6

Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.2375
retrieving revision 1.2376
diff -u -b -r1.2375 -r1.2376
--- ChangeLog   16 Feb 2007 10:36:20 -0000      1.2375
+++ ChangeLog   16 Feb 2007 11:09:41 -0000      1.2376
@@ -1,5 +1,14 @@
 2007-02-16 Sandro Santilli <address@hidden>
 
+       * server/sprite_instance.{cpp,h}: Fixed (hopefully)
+         the action execution order. Implementation is far
+         from elegant, but gives the idea of how it should work,
+         and actually passed our test for it, w/out breaking
+         anything else.
+       * testsuite/misc-ming.all/action_execution_order_test.c:
+         Add an additional test to verify that function definitions
+         are not specially handled as it comes to action execution
+         order. Don't expect failures anymore.
        * server/dlist.{cpp,h}: add clear_except() override taking
          a DisplayList as argument. Add a call_unload argument
          for both clear_except() methods; add empty() method;

Index: server/sprite_instance.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/sprite_instance.cpp,v
retrieving revision 1.163
retrieving revision 1.164
diff -u -b -r1.163 -r1.164
--- server/sprite_instance.cpp  14 Feb 2007 11:08:12 -0000      1.163
+++ server/sprite_instance.cpp  16 Feb 2007 11:09:41 -0000      1.164
@@ -1454,6 +1454,7 @@
        m_mouse_state(UP),
        m_root(r),
        m_display_list(),
+       oldDisplayList(),
        m_action_list(),
        m_goto_frame_action_list(),
        m_play_state(PLAY),
@@ -2392,9 +2393,9 @@
        size_t frame_count = m_def->get_frame_count();
 
        log_msg("sprite '%s' ::advance_sprite is at frame %u/%u "
-               "- onload called: %d",
+               "- onload called: %d - oldDIsplayList has %d elements",
                getTargetPath().c_str(), m_current_frame,
-               frame_count, m_on_event_load_called);
+               frame_count, m_on_event_load_called, oldDisplayList.size());
 #endif
 
        // Update current and next frames.
@@ -2421,6 +2422,8 @@
                // First time execute_frame_tags(0) executed in 
dlist.cpp(child) or movie_def_impl(root)
                if (m_current_frame != (size_t)prev_frame)
                {
+                       // Backup the DisplayList *before* manipulating it !
+                       oldDisplayList = m_display_list;
                        execute_frame_tags(m_current_frame, 
TAG_DLIST|TAG_ACTION);
                }
        }
@@ -2433,14 +2436,47 @@
        }
 #endif
 
+       // Advance DisplayList elements which has not been just-added.
+       // 
+       // These are elements in oldDisplayList cleared of all but elements
+       // still in current DisplayList (the other must have been removed
+       // by RemoveObject tags). I'm not sure we should do this actually,
+       // as maybe we need to execute actions in removed objects *before*
+       // we drop them...
+       //
+       // We do *not* dispatch UNLOAD event on the removed objects here
+       // as we assume the event was dispatched by execution of the
+       // RemoveObject tag itself. I might be wrong though, will come
+       // back to this issue later.
+       //
+       // Note that we work on a *copy* of oldDisplayList as we're going
+       // to need oldDisplayList again later, to extract the list of
+       // newly added characters
+       //
+       DisplayList stillAlive = oldDisplayList;
+       stillAlive.clear_except(m_display_list, false);
+       //log_msg("Advancing %d pre-existing childs of %s", stillAlive.size(), 
getTargetPath().c_str());
+       stillAlive.advance(delta_time);
+       
+       // Now execute actions on this timeline, after actions
+       // in old childs timelines have been executed.
+       //log_msg("Executing actions in %s timeline", getTargetPath().c_str());
        do_actions();
 
-#ifdef GNASH_DEBUG
-       log_msg(" advancing display list (we always do that!)");
-#endif
-
-       // Advance everything in the display list.
-       m_display_list.advance(delta_time);
+       // Finally, execute actions in newly added childs
+       //
+       // These are elements in the current DisplayList, cleared
+       // by all elements in oldDisplayList.
+       //
+       // Of course we do NOT call UNLOAD events here, as
+       // the chars we're clearing have *not* been removed:
+       // we're simply doing internal work here...
+       //
+       DisplayList newlyAdded = m_display_list;
+       //log_msg("%s has %d current childs and %d old childs", 
getTargetPath().c_str(), m_display_list.size(), oldDisplayList.size());
+       newlyAdded.clear(oldDisplayList, false);
+       //log_msg("Advancing %d newly-added (after clearing) childs of %s", 
newlyAdded.size(), getTargetPath().c_str());
+       newlyAdded.advance(delta_time);
 
        // goto_frame_action (for now) need be executed
        // *after* actions in child sprites have
@@ -3417,6 +3453,9 @@
        log_msg("Constructing sprite '%s'", getTargetPath().c_str());
 #endif
 
+       // Backup the DisplayList *before* manipulating it !
+       assert( oldDisplayList.empty() );
+
        on_event(event_id::CONSTRUCT);
        execute_frame_tags(0, TAG_DLIST|TAG_ACTION);    
 

Index: server/sprite_instance.h
===================================================================
RCS file: /sources/gnash/gnash/server/sprite_instance.h,v
retrieving revision 1.67
retrieving revision 1.68
diff -u -b -r1.67 -r1.68
--- server/sprite_instance.h    13 Feb 2007 17:06:28 -0000      1.67
+++ server/sprite_instance.h    16 Feb 2007 11:09:41 -0000      1.68
@@ -17,7 +17,7 @@
 // 
 //
 
-/* $Id: sprite_instance.h,v 1.67 2007/02/13 17:06:28 strk Exp $ */
+/* $Id: sprite_instance.h,v 1.68 2007/02/16 11:09:41 strk Exp $ */
 
 // Stateful live Sprite instance
 
@@ -616,8 +616,16 @@
        // TODO: shouldn't we keep this by intrusive_ptr ?
        movie_instance* m_root;
 
+       /// Current Display List contents.
        DisplayList     m_display_list;
 
+       /// oldDisplayList is a backup of current DisplayList
+       /// (m_display_list) updated at each call to ::advance
+       /// and at first call to ::construct (it's empty in
+       /// this latter case).
+       /// It will be used to control actions execution order.
+       DisplayList     oldDisplayList;
+
        ActionList      m_action_list;
        ActionList      m_goto_frame_action_list;
 

Index: testsuite/misc-ming.all/action_execution_order_test.c
===================================================================
RCS file: 
/sources/gnash/gnash/testsuite/misc-ming.all/action_execution_order_test.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -b -r1.5 -r1.6
--- testsuite/misc-ming.all/action_execution_order_test.c       15 Feb 2007 
09:45:33 -0000      1.5
+++ testsuite/misc-ming.all/action_execution_order_test.c       16 Feb 2007 
11:09:41 -0000      1.6
@@ -64,6 +64,7 @@
 
   dejagnuclip = get_dejagnu_clip((SWFBlock)get_default_font(srcdir), 10, 0, 0, 
800, 600);
   SWFMovie_add(mo, (SWFBlock)dejagnuclip);
+  add_actions(mo, " trace('as in frame0 of root');"); // can't use 'note' 
here, as it's not been defined yet
   SWFMovie_nextFrame(mo);
 
   mc_blu = newSWFMovieClip();
@@ -82,6 +83,7 @@
   SWFDisplayItem_setDepth(it_blu, 3); 
   SWFDisplayItem_setName(it_blu, "mc_blu");
   add_clip_actions(mc_red, " _root.note('as in frame1 of mc_red'); _root.x1 = 
\"as_in_mc_red\"; ");
+  add_clip_actions(mc_red, " func = function() {}; ");
   SWFMovieClip_nextFrame(mc_red); /* 1st frame */
   add_clip_actions(mc_red, " _root.note('as in frame2 of mc_red'); _root.x2 = 
\"as_in_mc_red\"; stop(); ");
   SWFMovieClip_nextFrame(mc_red); /* 2nd frame */
@@ -97,15 +99,17 @@
    * Even if their actions are not expected to be executed yet
    */
   check_equals(mo, "typeOf(_root.mc_red)", "'movieclip'");
+  check_equals(mo, "typeOf(_root.mc_red.func)", "'undefined'");
   check_equals(mo, "typeOf(_root.mc_red.mc_blu)", "'movieclip'");
   SWFMovie_nextFrame(mo); /* 1st frame */
   add_actions(mo, " note('as in frame2 of root'); _root.x2 = \"as_in_root\"; 
");
+  check_equals(mo, "typeOf(_root.mc_red.func)", "'function'");
   SWFMovie_nextFrame(mo); /* 2nd frame */
 
   /* In the frame placing mc_red,  actions in mc_red is executed *after* 
actions in _root */
   check_equals(mo, "_root.x1", "'as_in_mc_blu'");
   /* In subsequent frames, actions in mc_red is executed *before* actions in 
_root */
-  xcheck_equals(mo, "_root.x2", "'as_in_root'");
+  check_equals(mo, "_root.x2", "'as_in_root'");
   add_actions(mo, " _root.totals(); stop(); ");
   SWFMovie_nextFrame(mo); /* 3rd frame */