Viewport Transform and clipping

[inssein]

I was just testing out the latest build of fabric.js with my application, and ran into a small issue.

Bug is caused by PR: #2580

@asturur Why do we inverse the viewport transform before rendering the controls? This pretty much breaks for everyone who has zoomed the canvas.

[asturur]

i ll check again, i was sure i tested even that.
we invert the viewport as it was before, because otherwise the strokewidth of controls would get multiplied by the scaling factor.

i m on code later, i will check it.
did you have a ready fiddle to help me?

[inssein]

@asturur don't have a public build of fabric that has your commit yet, but if you setZoom() on the canvas, it should be fairly obvious.

For example, when I canvas.setZoom(0.25), my controls are four times the size.

[asturur]

http://jsfiddle.net/asturur/7fsw81mp/7/

link to build file

http://www.deltalink.it/andreab/fabric/fabric.js

i cannot find the bug.
Check please, i think you download the library when i put inside the setTransform methods.

[inssein]

@asturur took me a while to figure out, but I re-created the error. The combination of settings that cause it are canvas.controlsAboveOverlay = true and performing a canvas.clipTo().

Fiddle: https://jsfiddle.net/7fsw81mp/8/

[asturur]

Going to check now.
clipTo + canvas viewportTransform has an undefined behaviour in my opinion.
We can still check on original kitchensink.

[asturur]

Ok i see the mistake.
If we are clipping, the viewportTransform is inside the "save + restore" pair of clipping.
So if we draw controls after overlay, and overlay is after clipping restore, the mistake happens.

Should clipping be influenced by canvasZoom? ( i think yes).
if yes we move viewportTransform before clipping and the problem fixes by itself.
if no ve should reinitialize the viewportTransform after restoring the overlay.

Same problem for for overlay: shoudl overlay be influenced by canvas zooming? as of now if we are clipping it is not influencede, if we do not clip it is. ( with or without controlsAboveOverlay).

Thank for spotting this.

[asturur]

Ok effectively we the renderAll changes i messed the order of operation.
pre changes order was:

clip.
draw background
apply viewport
render objects
remove viewport
draw controls *
stop clipping
draw overlay
draw controls *

(*just one occurence)

I m restoring the normal behaviour with a PR.
in the fiddle you posted the library is already fixed and looks good.

[inssein]

thanks @asturur !