Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render function called twice while exporting #25

Open
delucis opened this issue Oct 15, 2018 · 7 comments
Open

Render function called twice while exporting #25

delucis opened this issue Oct 15, 2018 · 7 comments

Comments

@delucis
Copy link

delucis commented Oct 15, 2018

I noticed while experimenting with canvas-sketch that when you export an image using cmd + s or an animation using cmd + shift + s, the render function is called twice for each frame.

To reproduce:

// bug-demo.js
const SKETCH = require('canvas-sketch')

const sketch = () => {
  return ({ frame }) => {
    console.log(`render frame ${frame}`)
  }
}

SKETCH(sketch)

Then launch:

canvas-sketch bug-demo.js --open

Open the browser console and then try saving with cmd + s and you should see “render frame 0” logged twice.

Here are the relevant lines from my console (Firefox 62.0.3):

browser console output demonstrating error

“render frame 0” is logged once on page load, then twice more on export/save. When exporting an animation the duplication happens for each frame.

I imagine this has performance implications, but I also tripped up on it because I was using a generator function to get the next value to display for each frame. Each frame was being rendered twice (and saved once), which meant every other value returned from the generator was lost.


P.S. Generally the experience getting started with this has been really great. Thanks for the thoughtful library and clear documentation!

@mattdesl
Copy link
Owner

Thanks for the issue report! This is actually intentional, and not a bug. I'll explain why:

In many sketches the visible size on screen (within your browser) will not match the export size. For example, if you are using { scaleToView: true } it allows you to export huge print sizes without rendering such massive canvases to the browser each frame. In these cases, upon Cmd + S, we have to render the canvas to the full export size, then capture its data URL, and then resize and re-render it back down to its original browser size to avoid an ugly "flash" of large canvas in the browser, and to ensure the user continues to see the scaled canvas in the browser.

Another situation is with { exporting } prop — sometimes you have a sketch where a certain feature should only be used during export (like ignoring debug rendering that you just want during a browser experience). In this case, we render twice: the first render says "this frame is being exported" and the second says "this frame is not being exported, but instead just visualized in the browser." This way there is no flash of content while looking at the browser.

Just like with other declarative frameworks (React, Vue, etc) you should try not to assume too much about when the render() function will be called, as it may also get called in other situations such as on resize. Instead of creating side-effects in your render function, you should try to make it a pure function, such as by operating on the { frame } prop instead of increasing state on each render.

I'd also be happy to explore other solutions, if you let me know a bit more about why you needed to use a generator function. Maybe there is some syntax or documentation I could improve in canvas-sketch.

Cheers!

@delucis
Copy link
Author

delucis commented Oct 27, 2018

Hi! Thanks very much for the detailed response. This makes a lot of sense and I guess is just a drawback of rendering in the browser. (I’m currently working on something inspired by canvas-sketch that renders entirely in Node.)

I imagine this approach also causes issues if you want to repeatedly draw over the previous frame? (Like generative code that overlays on itself constantly.)

Might it be possible to instantiate a second invisible <canvas> and run the rendering for full-size frames there rather than on the displayed <canvas>? If the second “worker” canvas had its own instance of the sketch function, then a generator function created during sketch set-up would be protected by closure. I do totally understand the recommendation to stick with a stateless approach.

Thanks!

@delucis
Copy link
Author

delucis commented Oct 27, 2018

Oh, and regarding documentation: the documentation for this project is absolutely excellent, but yes, I suppose mentioning the mechanics behind frame exporting somewhere would be nice — I hadn’t considered the sizing issues at all.

@mattdesl
Copy link
Owner

Cool that you are remixing it with a node-only version. 😄 Not sure if you saw, but I have also designed canvas-sketch so that it can run inside Node.js, using something like node-canvas (example here), however I haven't tested it in a while and I haven't put a lot of thought into the node-only API/usage yet.

Yes, if you want to repeatedly draw over previous frames this might create issues. I'd like to find a nice solution for those scenarios, though.

The secondary <canvas> idea might work but I worry it would introduce performance problems, will be tricky with WebGL, and would maybe cause confusion in the end-user since it introduces more magic under the hood and more room for them to be surprised.

In general I'd like to clean up some of the logic around the render/frame loop, it generally feels a bit messy/unpolished right now. Maybe there is room for another setting like { incremental: true } (or a better name..) that sets you up for incremental generative rendering, somehow...

@mattdesl
Copy link
Owner

mattdesl commented Oct 27, 2018

Actually, I'm just testing and you can almost achieve a generative result like this, using the tick function instead of the render function. The tick function is designed to be called at the start of each new frame, and only once. Like the sketch and render() function, it is also wrapped with context scaling, so you can render into it.

const canvasSketch = require('canvas-sketch');

const settings = {
  dimensions: [ 2048, 2048 ],
  totalFrames: 100,
  animate: true
};

const sketch = ({ context, width, height }) => {
  return {
    begin () {
      // Initially fill/clear the canvas
      context.fillStyle = 'white';
      context.fillRect(0, 0, width, height);
    },
    tick ({ frame }) {
      // Render each subsequent frame
      console.log('Rendered', frame);
      for (let i = 0; i < 10; i++) {
        context.fillRect(Math.random() * width, Math.random() * height, 10, 10);
      }
    }
  };
};

canvasSketch(sketch, settings);

However, it's still a bit buggy, and this goes back to having to fix the frame/render loop. I've noticed occasionally the last frame doesn't render, and it seems to be due to timing differences.

@delucis
Copy link
Author

delucis commented Oct 27, 2018

Oh, nice, I didn’t notice the section on using canvas-sketch programmatically like that! It looks from the example like it currently works easily for single frames, but getting all frames for a loop would take a little more work.

Does begin replace tick or render for the first frame, or supplement them? So far in my project I just implemented the render function, not any of these other lifecycle possibilities. Some are no longer relevant when rendering straight to video — e.g. the distinction between tick and render — but begin and end do seem potentially helpful.

I don’t know exactly what the performance implications would be of a second canvas. I guess the number of calculations would stay the same because they are already duplicated in the current implementation, but there might be a memory hit. Depending on how fast it is to instantiate or dispose of a <canvas>, the export canvas could always be created only during export and then disposed of so a large canvas isn’t kept in memory. To me, it doesn’t feel like this would be more “magic” than the current implementation, because the resized large canvas is already effectively invisible, but I guess it would cause problems if canvas-sketch was used in parallel with something else that was grabbing the <canvas> element directly. I’ve only been working with 2D contexts (which is also all that node-canvas supports), so I’m not familiar with how WebGL would be impacted.

@mattdesl
Copy link
Owner

Does begin replace tick or render for the first frame, or supplement them? So far in my project I just implemented the render function, not any of these other lifecycle possibilities. Some are no longer relevant when rendering straight to video — e.g. the distinction between tick and render — but begin and end do seem potentially helpful.

begin and end can be used alongside tick and render. These things are still experimental but here is my plan so far:

  • begin is called at the start of playback, and also the beginning of each subsequent loop (so you can, for example, randomize parameters on each loop of an animation)
  • end is called after rendering the final frame, i.e. at the end of each loop (or never called if there is no finite duration to the animation)
  • tick is called before each frame, and before rendering each frame. Allows you to step physics and logic and move your animation forward.
  • render is called after tick in the animation loop, but also after the canvas is resized (i.e. by browser resize) and again just before the export (as the { exporting } prop has changed).

The second canvas is a cool idea but doing it automatically under the hood is just too magical for my taste, and could introduce a lot of new problems. I think its best that { canvas, context } props point to the same element that is visible in the DOM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants