fabricjs / fabric.js

Javascript Canvas Library, SVG-to-Canvas (& canvas-to-SVG) Parser
http://fabricjs.com
Other
28.85k stars 3.5k forks source link

renderAll does not use requestAnimationFrame #1979

Closed lukejagodzinski closed 7 years ago

lukejagodzinski commented 9 years ago

I realized that renderAll function does not use requestAnimationFrame to sync with browser. I know that it's used in animations but it should be also used in renderAll. Why? When moving object it's just rendering new position using renderAll and it's like the animation, so it's not efficient when not using requestAnimationFrame. What do you think about it? I could create PR for this.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

asturur commented 9 years ago

@jagi, did you use fabric with metoer js? i saw you wanted to do a package somewhere. I need to load fabricjs in the server side of a meteor APP. no problem using on the client. Can i contact you by email?

maboiteaspam commented 9 years ago

Hi,

I upvote this, Would be great to have this feature. I had to implement that sort of stuff to avoid sur rendering.

this.render = _.throttle(function(){
            // logic....
            fCanvas.renderAll();
        }, 5);

I guess requestAnimationFrame would be more accurate.

Not sure what s the matter with meteor, it happen to me with mutiple dom events interaction. This simple trick helped a lot.

lukejagodzinski commented 9 years ago

@asturur yes I've made Fabric.js package for Meteor but didn't publish it yet. However I'm planning to. Ok no problem, email me (email is in my profile info).

lukejagodzinski commented 9 years ago

Using _.throttle function here is not perfect hack, because mousemove events are rarely triggered more often then every 5 ms, so you would need to increase second parameter of this function to let's say 20-40 ms. But still it's not using synchronization with browser rendering. If browser is rendering webpage once every 16.6 ms (60 fps) then trying to render on canvas in the period between frames is waste of cpu power.

kangax commented 9 years ago

I'm not sure it would help but I'm open to see a comparison. Could you create a demo? rAF is a different concept — renderAll is an on-demand call whereas rAF is a continuous loop. But, like I said, would love to see a comparison.

lukejagodzinski commented 9 years ago

Even when rendering on demand doesn't mean that it shouldn't be in sync with browser rendering frames. Developers should use requestAnimationFrame to speed up not only canvas rendering but also DOM insertion. I will prepare comparison in free time.

lukejagodzinski commented 9 years ago

I was trying to prepare comparison but without success. With requestAnimationFrame I get up to 60 fps (because browser refresh rate is 60 fps) but browser can render my scene even with 120 fps, so at first sight it looks like requestAnimationFrame is slower. But I think it's not possible to present difference in this situation just by looking at fps. The difference can be visible on mobile devices where CPU is less powerful.

Let's cite official specification:

Secondly, setTimeout only updates the screen when it wants to, not when the computer is able to. That means your poor browser has to juggle redrawing the animation whilst redrawing the whole screen, and if your animation frame rate is not in synchronised with the redrawing of your screen, it could take up more processing power. That means higher CPU usage and your computer’s fan kicking in, or draining the battery on your mobile device. Nicolas Zakas does an excellent job explaining the impact timer resolution has on animation in a related article.

and

The other beauty of requestAnimationFrame is that it will group all of your animations into a single browser repaint. This saves CPU cycles and allows your device to live a longer, happier life.

Summing it up, we can say the same about moving object on canvas. When you move objects you render even 120 frames per second when you could just end up on 60 fps and still have smooth animation and save processing power and your battery. However I'm not 100% sure how to show this difference. Probably I should investigate it using Chrome Dev Tools.

kangax commented 9 years ago

@jagi just to clarify — I know about rAF benefts, and I'm all for using it :) I'm just unsure about applicability towards canvas. For DOM animations (when browser needs to repaint), it's useful. But I'm not sure canvas applies. On the other hand, this article suggests using rAF with canvas as well, so there might be some truth to it.

maboiteaspam commented 9 years ago

@jagi, yep you have right. I was certainly lazy to get a decent cross browser implementation and underscore was already loaded : D

Now, i ve made some effort, ahahah, i m working with a render loop such

  var frame;
  this.render = function(){
    window.cancelAnimationFrame(frame);
    frame = window.requestAnimationFrame(function(){
      // do some render logic
      fCanvas.renderAll();
    });
  };

I also had to borrow this from the internet

(function() {
  var lastTime = 0;
  var vendors = ['webkit', 'moz'];
  for(var x = 0; x < vendors.length && !window.requestAnimationFrame; ++x) {
    window.requestAnimationFrame = window[vendors[x]+'RequestAnimationFrame'];
    window.cancelAnimationFrame =
      window[vendors[x]+'CancelAnimationFrame'] || window[vendors[x]+'CancelRequestAnimationFrame'];
  }

  if (!window.requestAnimationFrame)
    window.requestAnimationFrame = function(callback, element) {
      var currTime = new Date().getTime();
      var timeToCall = Math.max(0, 16 - (currTime - lastTime));
      var id = window.setTimeout(function() { callback(currTime + timeToCall); },
        timeToCall);
      lastTime = currTime + timeToCall;
      return id;
    };

  if (!window.cancelAnimationFrame)
    window.cancelAnimationFrame = function(id) {
      clearTimeout(id);
    };
}());

I also keep using _.throttle to avoid sur numerous DOM events. It is possible that the render is not 100% optimum that way, but i don t get any overload anymore. Fair enough in my case.

This said i have not tested it to make sure it work well on major browsers.

BTW, i also implemented a pseudo fps counter. I request another frame to realize the counting. Thus while the app don t render, it keeps counting. And while app render, it does not count more images. Now i barely exceed 35 fps. But i m not sure that method is correct. Just to give you an idea.

lukejagodzinski commented 9 years ago

@kangax rAF was designed at first for working with Canvas. There was similar feature in Flash. However it can speed up any rendering and should be used wherever it's possible.

asturur commented 9 years ago

interesting, if i had time, i would inform myself about this matter. Still how it will work under nodejs? does it?

lukejagodzinski commented 9 years ago

I think it's not going to work in nodejs, for node there have to be different version of render function.

maboiteaspam commented 9 years ago

Hi,

Using 2000 elements on scene, i found this was super good to smooth the rendering process.

fabric.fastCanvas = function(_super){
  var __hasProp = {}.hasOwnProperty;
  var __extends = function(child, parent) { for (var key in parent) { if (__hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; };
  return (function(_super) {
    __extends(fastCanvas, _super);
    function fastCanvas() {
      this.frame;
      fastCanvas.__super__.constructor.apply(this, arguments);
    }
    fastCanvas.prototype.renderAll = function() {
      var args = arguments;
      var that = this;
      window.cancelAnimationFrame(this.frame);
      this.frame = window.requestAnimationFrame(function(){
        fastCanvas.__super__.renderAll.apply(that, args);
      });
    };
    return fastCanvas;
  })(_super || fabric.Canvas);
};

//var fastCanvas = fabric.fastCanvas(fabric.StaticCanvas)
//var fastCanvas = fabric.fastCanvas(fabric.Canvas)
var fastCanvas = fabric.fastCanvas(fabric.CanvasWithViewport)
var canvas = new fastCanvas( .... as usual );

Borrowed some code from https://github.com/rstgroup/fabricjs-viewport

gordyr commented 9 years ago

Other than for simply drawing something once, requestAnimationFrame should definitely be used... no question. It was designed for canvas animation first and foremost.

Its aforementioned usage for queuing up DOM manipulation calls into a single browser render loop tick is simply a nice added benefit.

Using requestAnimationFrame would prevent over saturation of the main JS thread. And would indeed smooth out any animation/interaction performed within fabric. Even simply wrapping renderAll inside rAF would have the desired result while keeping the current code and on demand nature of renderAll completely intact.

The point being that the browser would then automatically sync up fabrics draw calls with the browsers internal render loop/refresh rate, never going above 60 times per second and dropping calls when necessary. Meaning that fabric could internally be calling the renderAll function 300 times per second, but the browser will only have to draw it 60 times, giving the browser much more time to do everything else as well as reducing power usage, thread contention etc...etc... Ultimately providing a more stable framerate for far less work.

Right now fabric is doing a lot of extra unnecessary work on fast systems, drawing frames over and over again, multiple times for no reason. On slower systems it will choke and end up out of sync with any mouse/touch input. Using requestAnimationFrame would completely eradicate this. If the browser is doing other things and cannot quite manage to perform renderAll in 16.7ms (the requirement in order to achieve 60fps) frames will simply be dropped instead of the current situation where the rendering locks all input events etc.

Just change the renderAll function to this for now:

renderAll : function(){
       requestAnimationFrame(function(){
              //current renderAll code
      });
}

And for node.js simply skip the skip the usage of rAF.

At some point in the future consider implementing a render loop that starts when properties are changed via the set method and stops itself after say 300ms of inactivity using setTimeout and clearTimeout. Doing so would remove the need to call renderAll after making changes to object properties.

asturur commented 9 years ago

Not that is important to me, but i have a 144hz framerate. The 60fps cap is bound to vsync or is just hardcoded in browsers?

gordyr commented 9 years ago

@asturur

requestAnimationFrame will sync the canvas draw calls up with the browsers internal render loop, whatever that may be. I believe that they all stick to 60fps regardless of vsync but am not entirely sure.

gordyr commented 9 years ago

If anyone is interested adding something along these lines into you application code should quickly monkeypatch fabric without modifying any of the source code:

Warning: this is completely untested, but should do the job.


  fabric.util.object.extend(fabric.Canvas.prototype, {
            renderFrame : function(allOnTop){
               var _this = this;

               if(this.stopRendering){  //if stopRendering is flagged, cancel the animation loop
                    this.rendering = false; 
                    return;
                }

                 requestAnimationFrame(this.renderFrame.bind(this, allOnTop));

                var canvasToDrawOn = this[(allOnTop === true && this.interactive) ? 'contextTop' : 'contextContainer'],
                    activeGroup = this.getActiveGroup();

                if (this.contextTop && this.selection && !this._groupSelector) {
                    this.clearContext(this.contextTop);
                }

                if (!allOnTop) {
                    this.clearContext(canvasToDrawOn);
                }

                this.fire('before:render');

                if (this.clipTo) {
                    fabric.util.clipContext(this, canvasToDrawOn);
                }

                this._renderBackground(canvasToDrawOn);
                this._renderObjects(canvasToDrawOn, activeGroup);
                this._renderActiveGroup(canvasToDrawOn, activeGroup);

                if (this.clipTo) {
                    canvasToDrawOn.restore();
                }

                this._renderOverlay(canvasToDrawOn);

                if (this.controlsAboveOverlay && this.interactive) {
                    this.drawControls(canvasToDrawOn);
                }

                this.fire('after:render');

                this.renderTimeout = setTimeout(function(){
                    _this.stopRendering = true; //allow the loop to run for 300ms before stopping
                }, 300);

            },
            renderAll: function (allOnTop) {
                clearTimeout(this.renderTimeout); //cancel the timeout which artificially stops the render loop
                if(this.rendering){ return this; } //if we are already rendering, then short circuit
                this.rendering = true; 
                this.stopRendering = false;
                //start the render loop
                this.renderFrame(allOnTop);

                return this;
        }
    });

Whenever renderAll is called we check if the loop is already running. If it is, we do nothing. If it's not then we start calling the new renderFrame function over and over, using requestAnimationFrame.

(renderFrame is just direct copy of the old renderAll function with the added requestAnimationFrame stuff as well as the timeout logic added)

At the end of each frame we start a 300ms timeout, which if it gets fired will stop the render loop. And at the beginning of frame, we cancel it out. So whats basically happening is that if renderAll isn't called by fabric for 300ms, the loop stops, further saving resources.

This in my eyes is a good temporary compromise until @kangax decides what should be done. Since it keeps the current behaviour of fabric 100% intact. Though personally I think hooking into the set method to start the loop as I mentioned earlier would be superior, is it means we could all leave out 99% of the renderAll calls out of our application code and go and ahead and simply user set to modify objects allowing fabric to redraw when required.

Though i'm not sure thats the way @kangax wants to go of course.

kangax commented 9 years ago

I would still love to see a simple demo comparing current approach with the new one :)

gordyr commented 9 years ago

@kangax i'll put one together when I get a chance later on. It really is a no brainer if i'm honest. While there will be no noticeable difference on simple scenes and on high end hardware (other than a reduction in CPU consumption and temperature), the difference when rendering something more stressful should be dramatic.

Think of it like playing an FPS shooter like quake or whatever. If the framerate drops to 25fps everything still remains locked to the users input, because all that is happening is that rendering frames are being dropped while all game logic and input code continues to run and update in real time. With the rendering pathway hardwired in as it currently is, each and every fabric render call that has occurred in a single browser tick has to complete before the browser can update object positions/properties. Meaning that as soon as the framerate drops in a stressful scene, rendering lags behind user input.

It basically makes rendering asynchronous. Come to think it, if fabric goes this route it might be worth adding a callback to renderAll so that if people need to fire off a synchronous redraw, they can. This would be useful if needing to update the scene before copying the canvas to a blob for upload or before using toDataUrl as well as for internal use for firing after asynchronous actions like loading images etc.. etc...

gordyr commented 9 years ago

@kangax Okay here's a quick demo...

I've chosen to modify your svg caching demo as it proves very stressfull. Using Chromes built in FPS counter i get about 13fps meaning that each renderAll call is taking much longer than the 16.7ms required to hit a constant 60fps.

http://jsfiddle.net/h8tfkokt

This shows your standard demo with caching disabled. Grab one of the tiger heads and move it around. On my laptop at least (mobile core i7 CPU) all tiger heads stutter dramatically and the movement and object positions stop updating frequently, particularly when first grabbing a tiger head. Also note how far the tiger head lags behind the mouse input and how long it takes for the object controls to appear.

Now try:

http://jsfiddle.net/h8tfkokt/1/

Here we are using requestAnimationFrame . This time when you grab a tiger head and move it around, the other heads should continue spinning, without stopping for long pauses. Also the lag behind the mouse pointer should be greatly reduced and the object controls should appear instantly when selecting a head.

Obviously this is an extreme demo. Meaning that renderAll never completes fast enough at all to achieve lag free motion.

With a more reasonable workload most renderAll calls will be completing in less than 16ms meaning that using requestAnimationFrame will result in a very consistent framerate without huge spikes being caused by the garbage collector or by multiple queued render calls.

Whereas without it you find that for each browser tick we often have to perform renderAll several times before returning control back to the UI and thus the rest of fabric or your application.

Obviously how noticeable the above demo's are will depend on your hardware, but for me the difference is very dramatic, both demos result in about 12-13fps but using requestAnimationFrame we get continual movement/animation, without it, we do not.

If you use the console to monitor how many times renderAll is called over a given 10 second period (while interacting with the tiger heads) you will see that the function is performed about 10 times less using requestAnimationFrame. Also.... if you're still not convinced.... monitor your CPU usage and core temps :P

Interacting with objects in Fabric.js is exactly the kind of thing requestAnimationFrame was designed for. Why not use it? :)

Hope this helps!!

lukejagodzinski commented 9 years ago

Great example :) and it's what I was talking about :)

gordyr commented 9 years ago

@jagi thanks.... Hopefully @kangax agrees :)

maboiteaspam commented 9 years ago

@godyr, thanks ! very interesting and experienced feeback ! : )

gordyr commented 9 years ago

Any thoughts @kangax ??? As I said before it really is a no brainer and would be great to not have to patch this in the hacky way I am currently doing.

kangax commented 9 years ago

I have thoughts but not time :) maybe later today, sorry

asturur commented 9 years ago

how do i activate internal frame counter?

maboiteaspam commented 9 years ago

Can someone update the wiki until one of the core maintainer has time to pick this issue ? In the performance section. There are hundred of tickets, i m afraid this good stuff get lost among them.

asturur commented 9 years ago

@maboiteaspam "said the user with word 'spam' in the nickname" :D eheheh i could not resist, no offense of course. I will add a note in the wiki.

@kangax could: "Support rendering queque from browser" like here be good?

kangax commented 9 years ago

@gordyr great demos, thanks. I looked at the CPU usage and — while I'm not seeing as significant improvement in snappiness as you are — rAF version does seem to take slightly less. I imagine on slower machines it's more noticeable. Looking at the timeline in DevTools I'm seeing some improvement as well.

So... could you submit a PR and we'll go from there? :) Thanks!

joepie91 commented 8 years ago

Keep in mind that following rAF will also improve overall rendering quality; synchronizing with the hardware's refresh rate (which, I believe, rAF does by default) will prevent duplicate and skipped frames insofar possible. This makes for less 'stuttering', even if the GPU/CPU power is not being maxed out.

asturur commented 8 years ago

i would like to see how it behaves with my 144hz monitor. 60 does not go well with 144.

asturur commented 7 years ago

This has landed in a simpler way. A new method requestRenderAllhas been add. This method has a flag isRendering that stops new frames to being rendered untill the one requested is completed. renderAll is still what it was, and mouse interactions have been updated to use requestRenderAll