Closed lucasdinonolte closed 2 years ago
Name | Link |
---|---|
Latest commit | 11c0e8f87a69d4455dee8dea3c912f6cf586ec5d |
Latest deploy log | https://app.netlify.com/sites/dsi-logo-maker/deploys/633d70e5873ab70009da562e |
Deploy Preview | https://deploy-preview-152--dsi-logo-maker.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
I think this is really lovely and a very nice upgrade for the design function API. Let's discuss with @fdoflorenzano when we find a time.
@fdoflorenzano I think this is a great observation and something we should discuss on Monday. A good example is p5.js vs. React. I personally think it makes sense to make this change for React, since it's a big mess to make your own draw loop, but for p5.js that already has its own setup
and draw
it makes less sense. Perhaps it's something that's up to each engine? I can see a scenario where the default setting for the react engine is that it works like in this PR, but can be disabled with a setting. I'm in favor of making it really simple to code animations, but giving users the ability to do custom fps
handling if needed.
I think it's more than the difference between p5 and react. Fernando is right, this change would enforce the frame based way of animating. Which is also something I'm concerned (or at least unsure) about.
With this change each frame of an animation is a function of the current frame count. There is no shared state between frames and every calculation has to be made based on frame number if you need it to be deterministic.
While this approach feels natural to me (it's similar to how video editing software works) it isn't the only approach to think about animation. You can also think of them time based (seconds instead of frames) or event based (after x turns of the circle stop the animation). Especially the event approach might feel more natural to people coming to mechanic from a web development background.
In another exploration of the fps throttling I exposed the draw loop to the design function (instead of the frame count) and made it the user's job to use it in their function. I discarded this idea because at the time it felt like just a different way of having useDrawLoop in your function.
But now I'm thinking that maybe we should reconsider this approach as this would allow to also work with time and event based animation mental models as a user can decide what should be setup code and what should be run on every frame iteration.
Looking forward to discussing this on Monday 😊
My main concern with this new feature is to provide a simple API that users can easily pick up. My experience making libraries tell me that the makers of libraries are often concerned about flexibility while users just want ease of use. But maybe this thing of exposing the draw loop is a good one if we can do it in a simple way. Yes, let's discuss on Monday.
Sorry for the delay in reviewing this. I did a pass on Monday and wanted to do another fresh one to make sure I was understanding everything, and with other projects of course that got pushed a bit.
I like how the general structure for the design functions and look like in now (the register of callbacks specially). One little detail that I wanted to mention is that I don't love the return for every frame aspect for design functions, I generally prefer a more explicit API for setting up frames and everything. That and other ideas that I've seen tossed around lately make me feel that design functions will start looking more and more like react components, which to me is to make the API too opinionated.
Where I think there's room for improvement, is with the general design and naming of functions for the main flow of execution in Mechanic core. Part of why I've struggled to make a review is because I'm having a hard time understanding the current structure and which part calls what and so on. There's so many callbacks and functions and not great names (maybe) for some of them that it's easy to get lost. I would be happy to propose or work with you in a more streamlined flow, but I would need a bit of time to think and do it.
I hope all this makes sense. We can talk about it tomorrow!
Thanks for taking the time to review this explorative implementation. I know the names are a bit off and there probably is one or two callbacks too many. So I'd love your help to turn this into clear and understandable code.
Having spent a lot of time exploring this approach and also refactoring parts of DSI-Logo-Maker to this new setup I must say I fully share your concerns. Working with return values instead of explicitly calling frame
or done
looks very cool at first and works very nicely in the simple examples I initially created to implement this new animation API.
Refactoring DSI-Logo-Maker however showed the shortcomings of this approach: You don't just force people into a more React-like mindset as Fernando pointed out, you also lose a lot of flexibility and make things harder. This is not just true for the return based approach, but also for the idea of having the drawloop inside mechanic core. A few examples:
textAndImageCanvas
in DSI-Logo-Maker that even has two output options, based on if an image was loaded or notmemo
feature from another PR to essentially cache random valuesIt could be that these issues arose because DSI-Logo-Maker wasn't build with frame-based animation in mind. Nonetheless, the above mentioned are issues of the approach we explored and end-users could very well encounter them, unless we go the fully opinionated route and somehow force everyone into a frame-based stateless approach—which I would disagree with for the same reasons Fernando mentioned.
What struck me the most is, how much flexibility is potentially lost by trying to make the API a bit more comfortable, because a lot of assumptions have to be made, to make the API changes work. And to be honest, explaining to someone they now have to memoize random values feels more confusing then telling them: "Hey, use frame
when you want to add a frame to your animation and done
when you're ready to export.".
But I don't dislike everything I've explored. Moving the drawLoop away from a custom implementation and into something that is provided to each function as another method on the mechanic
object (in animation-custom
) mode felt really great and like a great simplification of things.
Also knowing that this is fully preconfigured with the correct frameRate and other settings helped reduce and simplify the code in a lot of DSI-Logo-Maker dramatically. The result is a very clear design function, that is clearly separated into the part that does the initial setup and the part that is running in every frame. Now it's up to the user if they want to treat the "every frame" part a s pure function, that only does operations based on the frameCount
argument it gets (which is how I'd do it, when starting from scratch) or if they follow a different mindset of animation and manipulate variables in the global scopes within their frame callback. This feels flexible, because both approaches are possible and you can bring much more of what you already know about coding and put it to good use in your design function.
So I think doing this exploration and trying to understand the pros and cons of the different approaches to animation was very valuable. I increasingly get the feeling that the mechanic callbacks (or dare we call them hooks?) like frame
and done
aren't inconvenient at all, but rather at a level of expressiveness to your code that allows you great levels of flexibility.
After having explored how to remove things from the user's code and make the settings and mechanic core very smart I'd now say: Let's put some code back into the user's code and make mechanic core very simple. I'm proposing we add a third hook to the mechanic
object exposed to a function called drawLoop
. It takes a function that runs on every frame. If a function calls the drawLoop
mechanic-core can know it's animated. If it doesn't mechanic-core will know it's a static function (so this could potentially even remove the animated
or mode
setting).
// Animated Example (Canvas/SVG)
export const handler = async ({ inputs, mechanic }) => {
const { canvas, ctx } = mechanic.getCanvas(inputs.width, inputs.height);
let someVariable = 0;
mechanic.drawLoop((frameCount) => {
// Either implement your drawing code as a pure function here
// or manipulate the parent scope's state if that feels more natural to you
someVariable += 1;
// Exit condition is 100% up to the user, so frame-based or event-based exit
// conditions are equally possible
if (someVariable >= 100) {
mechanic.done(canvas);
} else {
mechanic.frame(canvas);
}
})
};
// Static Example (Canvas/SVG)
export const handler = async ({ inputs, mechanic }) => {
const { canvas, ctx } = mechanic.getCanvas(inputs.width, inputs.height);
const someVariable = 0;
ctx.clear();
// Do some drawing using the value of someVariable here
mechanic.done(canvas);
};
// Animated Example (React)
export const handler = async ({ inputs, mechanic }) => {
// Instead of forcing the custom useEffect setup a predefined hook
// could wrap all of that and just execute on every frame
mechanic.useDrawLoop((frameCount) => {
// Do whatever state updates you want to do on every frame here
if (frameCount >= 100) {
mechanic.done();
} else {
mechanic.frame();
}
});
return <svg></svg>;
};
// Static Example (React)
export const handler = async ({ inputs, mechanic }) => {
useEffect(() => {
mechanic.done();
}, []);
return <svg></svg>;
};
Closing this in favor of a new, clean PR that implements that changes we've agreed upon after using this to explore potential solutions.
This explores the possibilities of adding FPS throttling by introducing a drawLoop util as part of Mechanic core.
What this does
frameRate
setting to design functionsframeRate
is used to by the WebM exporterframeRate
is used to build drawLoop function that is exposed to the function's enginedone
is called from within the handlercanvas-fps
,react-fps
,svg-fps
andp5-fps
Observations
Todo