patchGnash - The GNU Flash player - Patches: patch #5620, about "display_list" and...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #5620: about "display_list" and "sprite_instance::advance_sprite" implementation

Submitter:  Zou Lunkai <zoulunkai>
Submitted:  Thu 07 Dec 2006 01:12:28 AM UTC
   
 
Category:  core Priority:  5 - Normal
Status:  In Progress Privacy:  Public
Assigned to:  None Open/Closed:  Closed

Discussion locked!

Jump to the original submission

Thu 14 Jun 2007 01:11:37 AM UTC, comment #25: 

OK, close this item. For more reference, see "Timeline control" on wiki.

Zou Lunkai <zoulunkai>
Wed 13 Jun 2007 05:38:36 PM UTC, comment #24: 

Zou, I think this item was obsoleted by the TimelineControl wiki page and all the effort brought there, right ?
Can we close this ?

Sandro Santilli <strk>
Group Member
Sat 10 Feb 2007 01:40:20 PM UTC, comment #23: 

I committed my patch for now. Please give higher priority to bug #19020 as it is a regression.

Sandro Santilli <strk>
Group Member
Sat 10 Feb 2007 12:57:54 PM UTC, comment #22: 

Attached a patch that fixes this specific case and introduces no further failure in current testcases.
Basically I'm now removing from _frame0_chars any static character which is not in current DisplayList.
An equivalent effect is probably obtained by dropping _frame0_chars
as a whole and simply removing only static objects from current DisplayList.
See if you find it useful (simpler then restarting characters).

(file #11956)

Sandro Santilli <strk>
Group Member
Sat 10 Feb 2007 12:21:23 PM UTC, comment #21: 

The character class has a 'restart' method. Maybe that's what we need to simalate a completely new placement (we might call ::construct from ::restart, for example). Want to give this a try ?
Must make sure to call ::restart from sprite_instance method resetting the DisplayList, possibly moving the whole code block
in a specialized private method (could be ::restart itself actually).

Sandro Santilli <strk>
Group Member
Sat 10 Feb 2007 05:46:18 AM UTC, comment #20: 

Done
The updated Place_and_remove_object_insane_test.c contains this test.
It does sth. like this:
step1. place mc_red with '_x' set to 0 at 1st frame of _root;
and add actions 'mc_red._x += 10;' to _root;
add check 'check_equals(mo, " _root.mc_red._x",  "10");'.
step2. remove mc_red;
step3. sth. not important
when restart, the check fails because '_x' has been set to 20.

Zou Lunkai <zoulunkai>
Sat 10 Feb 2007 02:48:46 AM UTC, comment #19: 

build an imaginary testcase like this:
setp1. place a movieClip(mc_red)  at 1st frame of _root;
step2. remove  mc_red at 2nd frame of _root;

then play the imaginary testcase with Gnash:
after playing the 1st frame of this testcase, mc_red is added to sprite_instace::_frame0_chars

after playing the 2nd frame, _frame0_chars does not change;

when restart, mc_red is still in _frame0_chars, then mc_red will not be reconstructed, that's not what I expect. The mc_red should be reconstructed and 'onLoad' event listener should be called again.

hello, strk
I am not an expert in dealing with the DisplayList things. So I'd just like to discuss about this part but not fix the real code. OK, I'd better make some new testcases or improve the orignal one.

Zou Lunkai <zoulunkai>
Sat 10 Feb 2007 01:53:56 AM UTC, comment #18: 

Follow comment #12.

>>Characters instanciated in first frame are special in that >>they are the only ones that are not removed from the >>DisplayList at restart time. All other characters are removed.

yes, I still think so. But we should consider that characters instanciated in first frame could be removed by tags in later frame. They could keep alive only if they were not removed by later tags.

>>These "init_chars" can NOT be recognized by depth (tags in >>subsequent frames can change the depth of an instance) nor by >>id (you can have multiple instances of the same character >>definition).

yes, I still think so.

Zou Lunkai <zoulunkai>
Sun 14 Jan 2007 09:09:17 AM UTC, comment #17: 


>>The failing line is 'flag' expected to be '1'
>>and being '0' in Gnash.

Yes.

Update the insane test agian. Made it more clearer. The original one was attached in a hurry and was ugly organized. Sorry about it.




(file #11738)

Zou Lunkai <zoulunkai>
Fri 12 Jan 2007 02:07:10 PM UTC, comment #16: 

Sorry for the confusion, nothing to do with constant pool,
it was a bug with the check_equals/xcheck_equals macros.
Now fixed. The failing line is 'flag' expected to be '1'
and being '0' in Gnash.

Sandro Santilli <strk>
Group Member
Fri 12 Jan 2007 12:14:44 PM UTC, comment #15: 

The failure seems due to a Constants lookup bug actually.
The code is making a dictionary lookup, but the dictionary
used seems to be the one in Dejagnu.swf rather then
the one in the timeline containing the call.
Guess this is a Function call context problem.

Sandro Santilli <strk>
Group Member
Fri 12 Jan 2007 11:37:43 AM UTC, comment #14: 

Added to the testsuite, and expect the only failure.
Knowing more about it would help (the testcase lacks a general description of the process)

Sandro Santilli <strk>
Group Member
Fri 12 Jan 2007 10:27:27 AM UTC, comment #13: 

Another insane testcase.

Hope bombgame.swf dose not fall into this kind.

And I think current CVS is good enough, though fails on this testcase.


(file #11728)

Zou Lunkai <zoulunkai>
Tue 02 Jan 2007 03:49:36 AM UTC, comment #12: 

Brief summary of a discussion about loop frame advancement between me and Zou:

Characters instanciated in first frame are special in that they are the only ones that are not removed from the DisplayList at restart time. All other characters are removed.

These "init_chars" can NOT be recognized by depth (tags in subsequent frames can change the depth of an instance) nor by id
(you can have multiple instances of the same character definition).

I just committed a patch keeping these "init_chars" by 'character' pointers. The place_and_remove testcase now succeeds.

Next step is trying to break current implementation again :)


Sandro Santilli <strk>
Group Member
Fri 29 Dec 2006 09:47:56 AM UTC, comment #11: 

reopened for Zou request... let's keep this upen until we have enough testcases.

Sandro Santilli <strk>
Group Member
Thu 28 Dec 2006 01:46:45 AM UTC, comment #10: 

Closing this again as the latest patch in sprite_instance::add_display_object and place_object_2::execute fixed the bogus symptom this patch was bogusly trying to fix.

Sandro Santilli <strk>
Group Member
Tue 26 Dec 2006 03:10:11 PM UTC, comment #9: 

commited and updated runner, please check

Sandro Santilli <strk>
Group Member
Mon 25 Dec 2006 05:38:41 AM UTC, comment #8: 

Attaches the source of the test file: loop_test.c
The testrunner need to be updated.


(file #11596)

Zou Lunkai <zoulunkai>
Sun 24 Dec 2006 08:11:07 PM UTC, comment #7: 

I'm reopening this because the committed fix broke elvis
(bug #18549).

Note that the patch for loop_test.swf (how sprite_test.swf
is called in the CVS repository) also fixes the bombgame.swf
movie (last frame is correctly shown), so something must
be good about it... the next step would be checking differences
among the two. BTW, it would be nice to have loop_test.swf
be built from sources using Ming.

Sandro Santilli <strk>
Group Member
Tue 12 Dec 2006 01:14:25 AM UTC, comment #6: 

Added testcase, and verified it would fail w/out your patch.

Sandro Santilli <strk>
Group Member
Fri 08 Dec 2006 02:12:49 PM UTC, comment #5: 

Yes, your patch fixes this. Thanks!
BTW, the best way to provide a patch is by using 'cvs diff'
command.
Your patch is applied.

Also, since you are the author of the testcase, do you feel
like doing a step forward and making your SWF file a self-contained test case as described in gnash manual, under the section "Testing Support - Writing self-contained SWF tests with other compilers" ?

Note that the online manual is not up-to-date, but you should be able to build it from sources (well, of course if you handle to grab the sources...)


Sandro Santilli <strk>
Group Member
Fri 08 Dec 2006 02:01:26 PM UTC, comment #4: 

No, actually I see 2.5 seconds the black circle over the red rectangle, then a very short period of switching, and suddenly the 2.5 seconds of black circle over red again.
I'll check your patch.

Sandro Santilli <strk>
Group Member
Fri 08 Dec 2006 12:49:36 AM UTC, comment #3: 


>> BTW, can we include your testcase in our testsuite ?

Yes, no problem.

>>I already see the depths swap with current Gnash version,
>>are you using gnash as of current HEAD branch ?

No, I am using gnash_release_0.7.2
 
I can also see the depths swap. It might be not a  problem of  depths swapping. It is problem of playback. The  black circle and the red square should be on top of each other with equal time when playback happens.

Macromedia FlashPlayer plays it like this: black circle on top of red square for 2.5 seconds and then red square on top of black circle for 2.5 seconds.  Is that what you see with the HEAD branch?


Zou Lunkai <zoulunkai>
Thu 07 Dec 2006 09:37:52 AM UTC, comment #2: 

BTW, can we include your testcase in our testsuite ?

Sandro Santilli <strk>
Group Member
Thu 07 Dec 2006 09:35:05 AM UTC, comment #1: 

I already see the depths swap with current Gnash version,
are you using gnash as of current HEAD branch ?

Sandro Santilli <strk>
Group Member
Thu 07 Dec 2006 01:12:28 AM UTC, original submission:  

I paste a patch file about the "sprite_instance::advance_sprite(float delta_time)" member function in "sprite_instance.cpp". I think the "execute_frame_tags(m_current_frame)" function should not be re-executed for the first frame when play the flash movie again, because its display_list has been already retrieved.

The attached files are:

advance_sprite.txt:   patch file for the sprite_instance::advance_sprite(float delta_time) member function

sprite_test.swf: the test file that causes some bugs with the original player. Whenever the movie is played again, the black circle and the red square should change the depth and keep the depth untill next time.

sprite_test.fla: the source of the test file.

Zou Lunkai <zoulunkai>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #11956:  sprite.frame0.patch added by strk (2KiB - text/x-patch)
file #11596:  loop_test.c added by zoulunkai (3KiB - text/plain)
file #11467:  advance_sprite.txt added by zoulunkai (2KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by strk (Posted a comment)
  • -email is unavailable- added by zoulunkai (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 24 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-06-14 zoulunkai Open/ClosedOpen Closed
        Discussion LockNone Locked
    2007-02-10 strk Attached File- Added sprite.frame0.patch, #11956
    2007-02-10 zoulunkai Discussion LockLocked None
    2007-02-10 zoulunkai Discussion LockNone Locked
    2007-02-10 zoulunkai Discussion LockLocked None
    2007-02-10 zoulunkai CategoryNone core
        Priority2 5 - Normal
        Discussion LockNone Locked
    2007-01-14 strk Attached File#11469 Removed
    2007-01-14 strk Attached File#11468 Removed
    2007-01-14 zoulunkai Attached File- Added place_and_remove_object_insane_test_update.c, #11738
    2007-01-12 zoulunkai Attached File- Added place_and_remove_object_insane_test.c, #11728
    2006-12-29 strk StatusDone In Progress
        Open/ClosedClosed Open
    2006-12-28 strk Open/ClosedOpen Closed
    2006-12-25 zoulunkai Attached File- Added loop_test.c, #11596
    2006-12-24 strk Open/ClosedClosed Open
    2006-12-12 strk StatusReady For Test Done
        Open/ClosedOpen Closed
    2006-12-08 strk StatusNone Ready For Test
    2006-12-07 zoulunkai Attached File- Added advance_sprite.txt, #11467
        Attached File- Added sprite_test.swf, #11468
        Attached File- Added sprite_test.fla, #11469

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code