Possible bug(s) in PaintManager.repaint? about zebkit

[barmalei]

From one hand paint manager is very carefully overviewed and tested code, since it has big impact to entire zebra functionality. From another hand the first snippet with "return" also look not clear to me. Have to check why it is necessary (or unnecessary).

canvas.$da.width = -1 is indication the paint cycle has been completed and the canvas has no any dirty area. It allows paint manager to avoid unnecessary overhead.

I would appreciate very much any snippet you can provide to illustrate the problem.

PS: nice to see you can follow in source code so deep :)

[zhaizhai]

Thanks for the quick response! I see now that I might not have been implementing my custom component in the intended way. I was trying to do some animation, and the bug I had was that through a chain of events, a call to paint actually triggered synchronously something else which modified the state and asked to repaint again. The second repaint was effectively ignored because of the canvas.$da.width = -1 thing, which short-circuited the next repaint.

In this case I can fix the problem in my code. At first I thought setting canvas.$da.width = -1 was just an unnecessary optimization (since you were already throttling at 20 fps), but now I see that calling repaint during painting is somewhat dangerous practice, since you can get into an infinite loop of repaints (albeit throttled). Setting the width to -1 essentially invalidates any repaints called synchronously within a paint method. So I can see why you might have wanted to do that.

Perhaps instead of ignoring these silently, though, there should be a warning? That would have helped me in this case. I'm thinking that valid uses (if there are any) of calling repaint within a paint method can be implemented by calling repaint asynchronously (or just ignoring the warning).

I will also see if I can construct a snippet to illustrate the first issue with the early return.

[barmalei]

In general calling repaint method from paint manager "paint callback" (set with setTimeout()) is not forbidden. Paint manager "paint callback" does two important things:

• validates canvas and components hierarchy it holds
• paint the canvas UI components hierarchy

At the validation stage validated UI components can call repaint method as many time as they want and need. Actually it is not avoidable, since validation often means applying layout managers what causes UI components resizing and relocating. Called at the validation stage "repaint(...)" method will update dirty area of the rendered canvas. So at the second stage (paint) paint manager will have valid and up-to-date dirty area.

At the second (paint) stage calling repaint method is too late. It also will cause canvas dirty area updating but painting process has already been initiated and the dirty area updates will not take effect.

The rule is the following: don't call repaint method from a component paint(...), update(...), paintOnTop(...) methods, but feel free to call it from any other places.

[zhaizhai]

By "paint method" I did mean one of paint, update, paintOnTop. So my proposal is to do something like adding a warning here:

   // prevent double painting, sometimes 
   // width can be -1 what cause clearRect 
   // clean incorrectly  
   if (canvas.$da.width <= 0) {
       console.warn("repaint ignored because called synchronously within one of paint, update, or paintOnTop!");
       return ;
   }

As for the early return in calculating dirty area, I realized that there are only two scenarios where the canvas will have a dirty area:

1. there is already another paint scheduled, or
2. you are doing something like calling repaint within paint

In the first case, it's fine to return early because a paint will happen soon anyway. In the second case, the repaint is already going to ignored (by the current logic), so it doesn't matter. Still, I feel like the code is a bit confusing as it stands. I'm planning to send you a pull request as a suggestion for clarifying the logic (no pressure to accept it, I understand if you don't want to make changes to this part of the code).