Re: [Fenfire-dev] PEG animation_api--mudyc: Animation Layer API

[Tuomas Lukka]

On Wed, Oct 15, 2003 at 01:08:29PM +0300, Matti Katila wrote:
> :Scope:    Minor

I think the scope is rather major...

> We have a very powerful system for drawing on the screen and animating.
> In larger programs, where it's quite easy to create bugs,
> the current API is misused more often. To make it easier to
> debug the animation and events, I propose that we make a new layer for 
> calling the animation routines.
> 
> Instead of calling ::
> 
>     AbstractUpdateManager.chg()
> 
> directly, let us have a new API which is told the caller of ``chg``,
> for debugging::
> 
>     anim.animate("MouseEventHdl");
> 
> where ``anim`` is an instance of ``AnimationAPI``.

Issue: Is the string parameter needed? If you want debugging to show where
it was called, you can use a Throwable and print the stack trace
(without actually throwing an exception).

> 
> Issues
> ======
> 
> ISSUE: How can we make sure that no other object calls the public method 
> AbstractUpdateManager.chg()?
> 
>    RESOLVED: Use the source, Luke. Yes, we have source and we can 
>    use the command ``grep`` to search trough source tree. After all, 
>    this is easy to convert as a simple test.

I disagree. If it should never be called, there should not be a method.
> ISSUE: If we don't want people to see AbstractUpdateManager
> should we hide some of it's methods to package private?
> 
>     Small demos might still need these methods.
>     Otoh, safer is better. One possibility is to 
>     add this API within the demo framework but then it would be
>     very hard to call from the java side.

So what is the resolution?

I don't understand the last sentence at all.

I had raised some issues previously but I don't see them here. Where
did they go?

You should not delete issues, they are vital to keep in place so that
answers will be remembered, and especially the *REASONS* for those answers.

> Changes
> =======
> 
> Create a new animation layer which encapsulates 
> AbstractUpdateManager method calls, i.e, chg and setNoAnimation.
> The new layer will be the only one that takes care of reusing VobScenes.
> **No other objects are allowed to call the above named UpdateManager 
> methods in the source tree, except small demos.**
> 
> For safety of event handling, possibility to check if 
> VobScene has changed is added into the animation layer.

I don't understand how that makes things safer. Please explain.

> Let's define the following interface: ::
> 
>     package org.nongnu.libvob.view;
>     import org.nongnu.libvob.VobScene;
> 
>     /** An interface for providing common tool set for animation 
>      * and animation debugging information.
>      * This interface encapsulates the low-level animation interface
>      * such as AbstractUpdateManagers' chg and setNoAnimation methods. 

If we'll remove AbstractUpdateManager.chg, then this is the *main* place
for it - you can't refer to other places. You have to explain here
how this will work..


>      * <p>
>      * This interface sets strict policy for several routines.
>      *
>      *   1) There must not be other place to get previous/last 
>      *      VobScene. If a VobScene is saved in other place than 
>      *      here, it could prevent the GC to clean old VobScenes.
>      *      By using only the correct 'previous' VobScene
>      *      program can not get the famous 'invalid coorsys' bug.

>      *   2) There must not be objects that call low-level animation 
>      *      interface to change animation state. This could prevent
>      *      proper animation, e.g., by setting no animation even when
>      *      animation should be done.

ISSUE: Does there really need to exist a "lower-level animation interface"?




>         /** Animate to next VobScene by creating a new VobScene.
>          */
>         void animate(String caller);

How does this routine know how to do that?

Or does it actually *not* do what you say in the javadoc it does but
rather sets a flag or something?

See above about the issue on callers.

>         /** Switch to next VobScene by creating a new VobScene.
>          * The switch is fast and no animation is seen.
>          */
>         void switchVS(String caller);

Same question as above.

>         /** Rerender the current VobScene. 
>          * Changes in next frame are seen if coordinate system
>          * parameters are set. Rerendering the current VobScene 
>          * is much faster than creating a new VobScene, 
>          * so e.g., any drag actions should be implemented to use
>          * this method. 
>        * <p>
>          * Even though new coordinate systems can be created, the current 
>          * coorder implementation uses finite range of coordinate systems. 
>        * And creating too many new coordinate systems leads to
>          * undefined behauviour.
>        * @see VobCoorder
>          */
>         void rerender(String caller);

The second paragraph should be something like "(Implementation note: ".

>       /** Get the current visible vobscene. 
>          * When programming, you create vobscenes
>          * for future usually, so this returns the obivious previous 
>          * vobscene, e.g., to set coordinate system parameters.
>          */
>         VobScene getCurrentVS();

??? The second sentence makes no sense.

>       /** Returns true if VobScene has changed from previous
>          * <code> animate</code> or <code>switchVS</code> method.
>          * In some situations when handling the events the programmer 
>          * needs to know whether the VobScene is new or the still the old one.
>        */
>       boolean hasVSchanged();

Could you give an example? I don't understand when I'd use this.

        Tuomas