[Gnash-dev] Re: gotoAndPlay bug in Gnash (was: Serious performance problem)

[zou lunkai]

> Strk and me had a discussion about display lists and jumping between
> frames.

> We discovered that this problem does not just knock out invalidated
> bounds but is actually a very serious bug in Gnash.

> -- The problem:

> When looping back (for example, gotoAndPlay(_currentframe-1); )
> Gnash resets the sprite's display list and reconstructs it by
> executing all frame tags again until the target frame.

> This causes *everything* in the target frame to become new instances.
> Now, when looping back to a frame that still contains the same sprite
> instance this instance will be re-created, causing all ActionScript
> variables and other stateful information to be lost and actions in the
> first frame of that instance to be executed!

yes, it's not correct to reset display list when jump back. And this
will cause some unnecessary re-creations. The previous implementation
was using "reverse execution". eg. gotoAndPlay(N) (suppose N < current
frame)  will reverse execute frame tags from frame N to current frame.
But the concept of "reverse execution" is also bogus and fails on some
existing testcases.

> With other words: jumping between frames leads to lots of problems.

more specifically, jumping back leads to lots of problems.


> As a side effect set_invalidated() is called without need, but this is
> now a secondary problem.

> Jumping to frame 0 is a special case because it has a dedicated
> _frame0_chars display list. However, this can't be a solution.


> -- The solution we're discussing:

> Generally we would need to get rid of the recreation mechanism.
> goto_frame() should be able to build the correct display list without
> browsing through all frames.

We need to improve the recreation mechanism when jump back, in other
words, only re-create those need to be re-created. eg. characters
removed at current frame but appears at target frame. We need to scan
all frame tags from frame0 to target frame to find out which placement
tags are responsible for handling(ADD, MOVE) this character.

> This way goto_frame() can do a before/after comparison and
> destroy/create sprite instances as needed, including correct
> ActionScript execution and invalidated bounds calculation.

> One way is to keep a DisplayList for *each* frame of a sprite. This
> most probably would eat lots of RAM.

This is a simple way, but costs too much memory.  And the "DisplayList
" might be changed at any time when executing action scripts. That
means you need to maintain lots of DisplayLists for a single sprite.

In sprite_instance::goto_frame()
resetDisplayList() clears all static characters placed from frame1 to
current frame when jump back. Instead of doing that, I think we should
just clear static characters placed from target frame to current
frame. This might solve some problems above.