jeremyckahn / rekapi

A keyframe animation library for JavaScript
https://jeremyckahn.github.io/rekapi/doc/
MIT License
644 stars 72 forks source link

Remarks about the API #9

Closed sork closed 12 years ago

sork commented 12 years ago

Here are a few thoughts and suggestions:

  1. Because Rekapi was created at first with HTML5 canvas in mind, with other rendering methods added at a later time, I feel that the API is currently a bit too canvas-oriented. All methods beginning with canvas_ in the Kapi object seem to be out of place. I think that these should be relocated to a separate canvas-specific class.

    This line about canvas_clear in the documentation strikes me as a good example of this issue: Erase the canvas. This only does something if Kapi is bound to an HTML 5 canvas This means that this method makes no sense in other contexts.

  2. This leads to me suggest that there should be a canvas extension, just like there is a DOM extension. In my opinion, Rekapi should not assume that canvas is the preferred rendering method since - as you explained in your talk - Rekapi is about animating via keyframes, not rendering.
  3. Therefore, some code should moved from core into this new canvas extension. For instance, I don't think the core Rekapi module should include the current draw function logic. When you think about it, the core is about animating actors, not about drawing. In particular, the drawOrder code looks useless to me if we are using Rekapi for DOM animations or even WebGL/SVG (future extensions!).

To sum it up, I think the Rekapi API and code architecture should follow a more generic approach and not make any assumptions about how the user will want to draw the animation. I'm certain that I've overlooked some things since I've only had a quick look at the code and the API, but I'd be interested in reading your opinion on this.

Related idea: Add a build script accepting different parameters allowing to create a version of Rekapi with your preferred rendering method, i.e. choose which extension(s) you want to use.

jeremyckahn commented 12 years ago

Thanks for bringing this up, you make some really good points. I'll address each:

  1: It's true that Rekapi was originally meant to be canvas specific, and the focus of the project has changed somewhat over time. DOM support is at least as important as canvas support. I agree that it makes sense to replace the canvas concept in Rekapi with something more generic. I think "stage" is a good term – it aligns with the README. 

  2. Also a good point. My main concern is that we might be making things overly abstract. While we shouldn't presuppose that users are making Canvas animations, do we want to go the route of not having a visible default rendering context at all?  This of course could be mitigated by a build script that defines sane default modules like you mentioned. On the other hand, Rekapi allows you to make a plain Object the rendering context. I'm thinking that might be the appropriate default. It doesn't actually render anything, but it doesn't break.

  3. Yup, I agree. I think it makes to start moving the drawing logic out of core and into modules. Not sure the best way to do this yet, but I will add it to the 1.0 roadmap. 

I'm planning to have the bundle files include all of the extensions. That's what will be easiest for users. Advanced users who know what they do and don't need will be able customize their build with the build script (which will need to be rewritten, probably with Node).

sork commented 12 years ago

Thanks for your answer. I'm glad that we agree on most points.

  1. I personally would simply choose the term "context" to refer to contexts. However, I don't think there should be a replacement of the canvas concept in all rendering contexts. I would argue that there is no need for a container/canvas/stage in many situations, for instance when a plain object is used as a context. Also, when using the DOM, I don't actually see the purpose of forcing a stage concept which really only applies to canvas in my opinion.

    (On a related note, about the DOM extension: I think Rekapi should allow you to use actors in different divs, without assuming that they all are children of the same parent. I would suggest to remove the stage concept for the DOM extension entirely.)

  2. I don't think making a canvas extension would make Rekapi too abstract and generic. I like the idea of having no predefined rendering method at all by default.

When thinking about it, I started to realize that the most flexible approach Rekapi could take would be to allow setting a context for each actor. This means that on the same webpage, Rekapi could be used to animate keyframes for actors rendered on a canvas and in the DOM (and even in plain objects [1]) at the same time.

Even though I was not using Rekapi, I recently found myself in two situations already where this could have been useful. In BrowserQuest, I had a multi-canvas (layered on top of each other) rendering. In another application, I had to animate DOM and SVG nodes at the same time. Both of these use cases tend to go against the one-context-for-all approach that Rekapi is currently based on.

So, if each actor had a reference to its context, this would mean that contexts should be instantiated separately from Rekapi. Here is some fictional API code:


var rekapi = new Kapi(),
    canvas_opts = { fps: 50, width: 400, height: 500 },
    canvas = new Kapi.CanvasContext(document.getElementById('my-canvas'), canvas_opts),
    actor = new Kapi.Actor(canvas);

rekapi.addActor(actor);

In this example, we keep the MVC approach that you talked about. Kapi is the controller. Contexts are the views. We can have several of them. Actors are models. In the current architecture, Kapi is both the controller and the view at the same time which is not ideal in my opinion.

Hopefully, some of these ideas can help you build an even better Rekapi. :)

[1] This would be useful when you need to "animate" plain JavaScript objects which don't need to be rendered, like for example: the speed of an object, the gravity of a particle system, etc.

jeremyckahn commented 12 years ago

All well-reasoned points.

  1. You make a strong case for term "context" over "stage." Let's go with that.
  2. Again, I don't love the idea of having Rekapi rely on a soft dependency (I feel it makes the architecture harder to understand for newcomers). But, we'll have a general-use bundle and build script so it shouldn't really be an issue.

I agree that we should decouple actors from the stage. Since it's often much faster to render multiple small canvases instead of one large one, Rekapi needs to support that.

I also agree that Rekapi should support multiple canvases; this is worthy of being a 1.0 requirement. The first step is to define what a good API is, and then build to that.

To be clear, I consider the Kapi Object to be the Model, and the Kapi.Actors are the Views. The playback APIs (play, pause and stop) are the Controllers.

Your API proposal looks flexible enough, but I don't think it's a good idea to make a mandatory Kapi.CanvasContext initialization step. This kind of functionality should be reserved for advanced users with advanced use cases. It's important that Rekapi is as approachable as possible. I think that a better approach is to have a rendering context that is bound to the Kapi instance, as it does now. This context becomes the default renderer for each Actor until they are overridden. Perhaps something like this:


var kapi_context = document.getElementById('kapi-canvas');
var actor_context = document.getElementById('actor-canvas');

var kapi = new Kapi(kapi_context, { width: 400, height: 500 });
var actor = new Kapi.Actor({
    context: actor_context // This parameter would be optional
    ,draw: function () {}
  });

kapi.addActor(actor);

My primary concern is supporting the flexibility that we can agree is necessary, while also keeping the API simple. Thoughts?

Thanks for your suggestions, I think this is really going to help Rekapi be more useful.

sork commented 12 years ago

Apologies for the delay. I wish I could have answered earlier.

First, to be clear, I'm not advocating only for multi canvas support, but for multi-context support. For example, allowing users to animate actors both in the DOM and canvas at the same time. Multi-canvas is only a subset of all the use cases that I have in mind.

I totally agree with you that the API should remain simple for newcomers and I think my earlier code sample was a bit too complex.

I kind of disagree that there is always a need for a context object as a Rekapi constructor argument. In my opinion, what this brings in terms of simplicity for newcomers is traded for rigidity and makes the API looks strange in a multi-context scenario. What you suggested in your code sample would work fine but I find it inconvenient that there is a main context taking precedence over other contexts who need to override it.

Therefore, here is a new API suggestion:

var kapi = new Kapi(),
    canvasActor = new Kapi.CanvasActor({ 
      context: document.getElementById('my-canvas'),
      width: 400,
      height: 500,
      draw: function () {}
    }),
    domActor = new Kapi.DOMActor({
      context: document.getElementById('octocat-img')
    });

kapi.addActor(canvasActor);
kapi.addActor(domActor);

As you can see, I'm sticking to my suggestion of one context per actor for high flexibility. I got rid of my earlier CanvasContext class suggestion because there's no need to over-complicate things. I subclassed the Actor class instead, for all different context types (basically, there would be one for each extension).

As far as I can tell, this does not complicate the API. It eliminates the default rendering context bound to the Kapi instance, in order to solve the multi-context issue, and makes explicit which context each actor will belong to. It also makes the API more consistent because the current Actor does not have a prefix whereas DOMActor does. This paves the way for many other actors such as SVGActor, WebGLActor and so on.

On a side note, I would remove all context-related methods from the Kapi class. To be perfectly clear, all canvas-specific code would go to the CanvasActor class, for instance the canvas_clear method.

Some other remarks:

Please let me know what you think! Congrats for the recent requestAnimationFrame support. :)

jeremyckahn commented 12 years ago

Thanks! These are all really good ideas. I think that if Rekapi could do all that you described, it would make the library far more useful for advanced users. I just have a few concerns.

First, this will be backwards-incompatible with the current API. This isn't a deal breaker, because Rekapi doesn't have many (if any) users that I'm aware of, and we are pre-1.0, so we have some freedom. Rekapi uses SemVer as stated in the README, and we are free to change the API before hitting 1.0.

From what you describe, and I don't disagree, it seems that the Kapi.Actor should act as a completely generic View - it would just receive state values but not render them to any visible context. I like this approach, because we get complete flexibility over what we can do with the data. However, this will completely break all code that uses Kapi.Actor. I think that it makes sense to bite the bullet and break the API. Do you have any thoughts regarding an intermediate step to make this architectural change a little smoother?

Another concern is approachability. As advanced uses cases become more abstract, it gets harder to support basic use in a way that is easy to understand. Personally, I really like how simple it is right now to make a very basic animation. I think it's important to maintain that simplicity. I still think a good compromise is to allow the Kapi constructor to accept default options like canvas width, height and context (as it does now) and allow the Actors to override them as needed. The library should allow any combination of Kapi.Actor subclasses to be added to an animation. Modifying your last example:

var kapi = new Kapi({
      context: document.getElementById('my-canvas'),
      width: 400,
      height: 500
    }),
    canvasActor = new Kapi.CanvasActor({ 
      draw: function () {}
    }),
    domActor = new Kapi.DOMActor(document.getElementById('octocat-img'));

kapi.addActor(canvasActor);
kapi.addActor(domActor);

The above snippet should "just work." kapi's context would be passed down to canvasActor, and domActor would just ignore it. One of my goals for updating the API is to keep it mostly as-is for basic use cases. The following should work as well:

var kapi = new Kapi(),
    domActor = new Kapi.DOMActor(document.getElementById('octocat-img'));

kapi.addActor(domActor);

I agree that requiring a default rendering context for all animations doesn't make sense (such as DOM). What do you think of these two snippets?

As a side note, we need to spec out Rekapi extensions (rendering context, Actor subclass, whatever else) at some point, but that's outside the scope of this discussion.

Regarding your other remarks:

Yes, the canvas_ methods need to be removed from the core and put into a canvas extension. I'm still working out the best way to approach that.

I think we can remove the container node requirement for DOM animations. Now that Shifty/Rekapi fully support CSS transforms, the position of the Actor nodes and their parent is irrelevant. We can remove that once we start on the other API changes.

Once I finished the requestAnimationFrame support I made the default fps 60. Rekapi falls back to setTimeout when some framerate other than 60 is specified. The Kapi constructor should accept a config object for options such as this.

jeremyckahn commented 12 years ago

I started a wiki page to serve as the checklist for the items discussed here. I'm jotting down the high-level requirements resulting from this issue, please add more if you think I missed any.

sork commented 12 years ago

Awesome, I really like what you suggested here and I'm happy we agree about the DOM context and new Actor subclasses.

About the Actor class, which now will be generic, I think it could still be used for "animating" plain JavaScript objects which are not meant to be rendered.

It would be great if it could automatically update the properties of an object context as Rekapi plays the animation.

Example:

var kapi = new Kapi(),
    plainObject = {speed: 0.49, gravity: 1.58},
    actor = new Kapi.Actor(plainObject);

kapi.addActor(actor);

actor.keyframe(0, {
    'speed': 0.8,
    'gravity': 1.2
}).keyframe(1000, {
    'speed': 2.5,
    'gravity': 1.65
});

kapi.play(); // `plainObject` is updated

However, you will probably prefer to put that feature inside another class like ObjectActor and keep Actor as a base class which should never be instantiated directly by the user (as per the new 1.0 requirements wiki page). I'm fine with either approach.

About the API change to actors which would break existing applications, I'd say let's go with it. As you mentioned, SemVer allows you to do this and I think the benefits outweigh the costs since there aren't many users of Rekapi yet (to my knowledge). I don't really have a good suggestion to make the transition smoother.

I completely agree with your two snippets. Such an API seems to have the right mix of simplicity and flexibility. Advanced users can give each actor a context if they need to, and newcomers still have access to the simple way of doing things. I think we have reached a good balance.

On a side note, I think you made a small mistake in the wiki at Replace "stage" term in documentation with "canvas". You surely meant context instead of canvas.

Anyway, thanks for this, I think Rekapi 1.0 is going to be really great and powerful!

Feel free to close this issue if you prefer to have conversations about specific 1.0 features in other issues, because this one had a pretty broad scope.

jeremyckahn commented 12 years ago

Yeah, I don't see why Actor has to be abstract and unusable by itself. Just keyframing unrendered properties can be useful, so I'll build towards that. Setting the initial state as you do in your code sample deviates a little bit too much from the current API, but something like this might fit a little better:

var kapi = new Kapi(),
    state = {speed: 0.49, gravity: 1.58},
    actor = new Kapi.Actor({
      'draw': function () {},
      'initialState': state
    });

kapi.addActor(actor);

actor.keyframe(0, {
    'speed': 0.8,
    'gravity': 1.2
}).keyframe(1000, {
    'speed': 2.5,
    'gravity': 1.65
});

kapi.play(); // `state` is updated

That shouldn't be too hard to build in. I think it makes sense for plain Actors to have a draw function, if only to keep consistent with the subclasses. I don't think an ObjectActor class is necessary, Actor can serve that role. I updated the Roadmap page, and also corrected the error you pointed out.

I'll close this for now, but feel free to ping the issue if you have any followup comments. Thanks for taking the time to bring up all these concerns! I'm really glad to have collaborators on the API, it really helps me figure out what use cases are most important to cater to. My goal is for Rekapi to be the ultimate keyframe tool for JavaScript, and the abstractions you proposed will make the library more useful for many more applications.