clone and cloneAsImage creates objects larger than original (retina related)

[jhiswin]

This worked normally in 1.5.0 (unless this is an intended breaking change?).

Both clone() and cloneAsImage() creates a cloned object that is larger than the original.

[asturur]

can you make a fiddle that shows it please?

[jhiswin]

I'm having a hard time simplifying my code. On looking deeper, it seems like SVG paths and SVG images in general is misbehaving, even without cloning.
Originally I had been using minX, minY to get positions before/after cloning, and straight cloning SVG preserved the original sizes, but now it seems that cloning complicated SVG objects gets resized and altered and just handled very differently.

[jhiswin]

It seems like scaling SVG images before adding to patterns and static canvases also behaves differently. I can't seem to get SVG images to resize/scale and then be applied as patterns correctly.

[jhiswin]

@asturur
I made a simplified demo showing cloneAsImage misbehaving. I'm trying to make a demo for clone() misbehaving, but can't get a simplified example yet..
http://jsfiddle.net/ynwduoxc/1/

[jhiswin]

Update:
I've figured out the problem. Retina scaling seems to break things for me.
Is there a best practice so that my code will work even after retina scaling (which I do want)?

[butch2k]

I ended up deactivating retina support as I encountered issues with todataurl not behaving correctly. Now everything works fine. 

    _____________________________

From: jhiswin [email protected]
Sent: jeudi, juin 9, 2016 11:52 PM
Subject: Re: [kangax/fabric.js] clone and cloneAsImage creates objects larger than original (#3041)
To: kangax/fabric.js [email protected]

Update:
I've figured out the problem. Retina scaling seems to break things for me.
Is there a best practice so that my code will work even after retina scaling (which I do want)?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[asturur]

we are improving retina screen support.
meanwhile you can just disable it before calling clone as image and reactivate it again later.

fabric.canvas.enablerRetinaScaling = false; should do the trick.

[butch2k]

Yes that's what I resorted to. 

[jhiswin]

Just an FYI, StaticCanvas and patterns are also affected. Reactivating it causes problems, even when scaling is done, so I had to permanently turn off retina on the StaticCanvas being used as a pattern fill, which for the most part fixed the problems. For everything else I could just rescale (scaleToHeight) after cloning, which maintains the nice retina quality.

[jhiswin]

But, SVG shapes on the main canvas are significantly sharper than before (I just have to remember to scaleToHeight(originalHeight) after cloning), which is quite nice. It'd be great if there was a workaround for sub-Canvases and pattern fills.
If anyone knows of a workaround that doesn't require turning off retina scaling, let me know,

[asturur]

would you so kind to open a jsfiddle showing patterns problems so i can mark this as a bug?

[butch2k]

IIRC: create a rect, copy it as an image position it at the same position as the rect. On standard desktop no issue, both are same position and same size. On a retina the image is half size the rect and not correctly positionned (iirc).
I'm unable to create a fiddle rightnow.

[jhiswin]

@butch2k I posted a fiddle for that already. asturur is talking about another issue related to patterns.
@asturur I'll try to put a fiddle together for you tomorrow. Basically it is: create a pattern from an SVG rendered onto a StaticCanvas, turn off retina, scale it, render the shape with pattern fill and clone an image, etc. Then, turn retina back on and it will be the wrong size. This is even if you turn the retina back on after adding to canvas, which is odd.

Everything will work as expected as long as the StaticCanvas you use for the pattern has retina off at all times.

[butch2k]

@jhiswin Yes i encountered it as well when using a hexagon pattern fill using an svg pattern.

I'm under the feeling that it's related to the way dpr is handled in the __toDataURLWithMultiplier function. on a retina the multiplier is passed to the function being multiplied by the dpr. It's then divided back to it's original value (newZoom) ????

Basically it means you do not retinascale when dataURLing.

To dataURL, multiplying the multiplier by the dpr
toDataURL: function (options) {
(...)
if (this._isRetinaScaling()) {
multiplier *= fabric.devicePixelRatio;
}

if (multiplier !== 1) {
  return this.__toDataURLWithMultiplier(format, quality, cropping, multiplier);
}

And __toDataURLWithMultiplier setting it back to default multiplier:

__toDataURLWithMultiplier: function(format, quality, cropping, multiplier) {
(...)
zoom = this.getZoom(),
newZoom = zoom * multiplier / fabric.devicePixelRatio;

[jhiswin]

@butch2k Ah that makes sense. That explains why turning off retina scaling on a StaticCanvas works around the problem.
I thought at first it was because pattern fill requires the original HTMLImageElement, rather than the scaled fabric Object.

[jhiswin]

@asturur After further testing: turning off retina during cloneAsImage was enough to resolve the pattern issue (I clone multiple times and had turned it off in the wrong place).

Do you still need a fiddle for patterns?

[asturur]

i think that cloneAsImage should be retina disabled unless required by dev with a parameter. Otherwise cloning the same object mutipletime will end in a bigger and bigger object everytime.

[asturur]

@butch2k that newzoom variable is an hack to let the shadow scale properly. when we dataurl we create a new canvas with all the stuff over and i disable the retinaScaling but i enable a zoom that is twice the original. So that