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

[SVG IMPORT] Shapes with transformMatrix have misplaced and/or stretched controls. #1520

Closed
saeschdivara opened this issue Jul 23, 2014 · 57 comments · Fixed by #3969
Closed
Assignees
Labels
Milestone

Comments

@saeschdivara
Copy link

I tried to load an svg which was created by fabric.js (toSVG). Loading was possible but the selection area of the canvas objects are totally wrong (as can be seen in the screenshot)
screen shot 2014-07-23 at 10 13 56

The fabric.js version is 1.4.9

In the fiddle you cannot even select the objects: http://jsfiddle.net/saskyrar/L8cmw/2/

Here is the svg:

<?xml version="1.0" encoding="UTF-8" standalone="no" ?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="800" height="700" style="background-color: rgb(255, 255, 255)" xml:space="preserve">
    <desc>Created with Fabric.js 1.4.5</desc>
    <defs></defs>

    <circle
        cx="0"
        cy="0"
        r="75"
        style="stroke: none; stroke-width: 1; stroke-dasharray: ; stroke-linecap: butt; stroke-linejoin: miter; stroke-miterlimit: 10; fill: blue; opacity: 1;"
        transform="translate(521 203)" id="2b0a303e-857e-9431-e2c9-d67a59759422"/>

    <rect
        x="-150"
        y="-100"
        rx="0"
        ry="0"
        width="300"
        height="200"
        style="stroke: none; stroke-width: 1; stroke-dasharray: ; stroke-linecap: butt; stroke-linejoin: miter; stroke-miterlimit: 10; fill: blue; opacity: 1;"
        transform="translate(185 229)" id="09e6bb6e-8970-57b2-bec4-bcb77787efc5"/>
</svg>

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

@asturur
Copy link
Member

asturur commented Jul 23, 2014

Current toSvg has some problem.
Would you like to try to import svg created with this alpha-never-to-happen-test version ?

http://www.deltalink.it/andreab/fabric/tests.html

you find an orange button that says "test" and allow you to export current canvas to SVG.
Is in italian, sorry, but icons will help you.
I'm testing right now to find solution. More tester the better.

@saeschdivara
Copy link
Author

Hey,

Thank you for the comment first.

I exported the following:

<?xml version="1.0" encoding="UTF-8" standalone="no" ?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="900" height="500" xml:space="preserve"><desc>Created with Fabric.js 1.4.9</desc><defs></defs><g transform="translate(5 23.9)"><text font-family="Helvetica" font-size="36" font-weight="normal" style="stroke: #000000; stroke-width: 1; stroke-dasharray: ; stroke-linecap: butt; stroke-linejoin: miter; stroke-miterlimit: 10; fill: #000000; opacity: 1;" transform="translate(-4.5 35)"></text></g><circle cx="undefined" cy="undefined" r="30" style="stroke: #00FF00; stroke-width: 1; stroke-dasharray: ; stroke-linecap: butt; stroke-linejoin: miter; stroke-miterlimit: 10; fill: #00FF00; opacity: 1;" transform="translate(336.5 197.5) scale(2.64 2.64) "/>
<rect x="-15" y="-15" rx="0" ry="-15" width="30" height="30" style="stroke: #FF0000; stroke-width: 1; stroke-dasharray: ; stroke-linecap: butt; stroke-linejoin: miter; stroke-miterlimit: 10; fill: #FF0000; opacity: 1;" transform="translate(127.75 188.25) scale(3.73 3.73)"/>
</svg>

But The result was the following:
screen shot 2014-07-23 at 13 07 51

@asturur
Copy link
Member

asturur commented Jul 23, 2014

ok ok, i deleted the r of ry by error, and it became y. Now is fixed.
But if you do shapes as before , and then re import, the bouding box is correct or misaligned?

@saeschdivara
Copy link
Author

Still misaligned (the bounding box is for the rect):
screen shot 2014-07-23 at 13 33 40

@saeschdivara
Copy link
Author

It seems that the transformations screw up the bounding box

@asturur
Copy link
Member

asturur commented Jul 23, 2014

I'm checking every aspect of it for the export.
Export finished i hope i can understand what happen going back.
Because if browser render it good it means is a valid svg.
If is a valid svg it has to import fine.

Otherwise something is wrond and we have to understand if it is in the rendering or parsing.

Are you grouping the objects or loading them ungrouped? cause that matter a lot.

I need time :) and a bit of luck.

@saeschdivara
Copy link
Author

I don't group them because I need to be able to move every single object independently.
Well at least we know it does it differently than the browser. This has maybe to do with the viewport?

@saeschdivara
Copy link
Author

By the way, how can I get the whole thing to build?
It seems not to work on my mac.

@saeschdivara
Copy link
Author

It seems that a circle gets height and width from the width & height attribute. Which means if the canvas has a height of 300px and a width of 400px, the circle will have a height of 300px and a width of 400px.

This is why the selection box of my circle is as big as the canvas itself

@asturur
Copy link
Member

asturur commented Jul 24, 2014

Yes but everything is very delicate.
Every class of shape should be analyzed good to be sure that you can:
-Import a svg with a X shape in path-group.
-Import a svg with a X shape as a single shape on canvas
-Create a shape for the canvas ( that is what the library is mostly for if i'm not wrong )
-export what is on a canvas on a SVG
So you should not get stuck on your need ( that happens often to me , i modify code and then i notice i broke other 12 functions ) but try to understand how the fabric.js is supposed to run and look like. I'm not trying to be rude of course, and i'm not even a member of fabric.js so this could be even just my BS.
@kangax is sending us on google-group soon :)

@saeschdivara
Copy link
Author

I fixed the circle height and width problem, the commit you can see on my fork: https://github.com/saeschdivara/fabric.js

@saeschdivara
Copy link
Author

Yes, I can see that but it doesn't seem to me logic that in the creation of an object by an svg element the height should be determined by the height of the canvas itself.
How can I test if I have just broken other functions?

@kangax kangax added the bug label Jul 24, 2014
@saeschdivara
Copy link
Author

I think I have found the problem: getViewportTransform (in object_geometry.mixing.js)
screen shot 2014-07-25 at 09 55 51

It seems that the viewport is not the correct one. This maybe because the transformation is not correctly saved in the object so that the method returns always a no transformation.

I also understand that I have to be very careful not to destroy anything.

@saeschdivara
Copy link
Author

I did a first fix. This means even after an import you can click on the object and select the correct one (This fix can be seen on my fork as the last commit as of today).
I still have to fight with the controls and the border but I think I will get that too.

@asturur
Copy link
Member

asturur commented Jul 25, 2014

For sure
this. _transform(ctx,noTransform)
is missing from the _renderControls method.

And maybe the setCoord or viewportTransform has to handle even the this.transformMatrix.
It looks like that if object has transformMatrix outside the pathgroup it won't work properly.

That's why i always group the svgs.

@kangax is transformMatrix property supported on single shapes ore is not intended to?

@saeschdivara
Copy link
Author

Could you please tell me where would I have to call this. _transform(ctx,noTransform) in _renderControls?

Thanks for the tip about _renderControls. Now I can see the selection area on the objects.
I think I need to do better than only use radius for the height and width for circles but otherwise it looks already really good 👍

@asturur
Copy link
Member

asturur commented Jul 25, 2014

circle initialize method:

this.callSuper('initialize', options);
this.set('radius', options.radius || 0);

switch position of the function as this.

@saeschdivara
Copy link
Author

Could you please send me the changes you made to the export?

@kangax
Copy link
Member

kangax commented Sep 3, 2014

@asturur didn't we fix this one?

@asturur
Copy link
Member

asturur commented Sep 4, 2014

the problem is that importing single shapes from svg fails if shapes have a transform matrix on them.
this was one of the case.
with a couple of other little bugs that get solved by the way.
but we cannot yet load separate shapes with transofrmmatrix. the moving and the controls get screwed.
sometimes it will happen you move the mouse up and the object goes left double spees :s

@Canuto100
Copy link

hi, I have the same problem hopefully can help me ,
I share next data
0. My version fabric --> var fabric = fabric || { version: "1.4.11" };

  1. Load Method -> fabric.loadSVGFromURL(
  2. My Draw Borders is wrong :(
  3. I share the following image:

wrongdrawcontrols

@asturur
Copy link
Member

asturur commented Dec 10, 2014

please share svg, complete load code, and please upgrade to fabric 1.4.13.

@Canuto100
Copy link

hi, switch to version 1.4.13 and the problem continues, I give you the information you ask me,

extract the string

str = "";

$scope.myLoadSvg = function(str) {

fabric.loadSVGFromString(str, function(objects) {
$scope.canvas.add.apply($scope.canvas, objects);
$scope.canvas.renderAll();
});

};

http://demos.iroxit.com/cyes/media/cano.svg

OR

Load with but is grouped around and I need to manipulate separate objects so Therefore I need to ungroup and watch the drawing border well.

fabric.loadSVGFromURL(pathImage, function(objects, options) {

   var shape = fabric.util.groupSVGElements(objects, options);

              canvas.add(shape);
              canvas.renderAll();

  });

thank you !

@asturur
Copy link
Member

asturur commented Dec 10, 2014

So listen, having
<g transform="translate(496.5 381)">
on the shapes will not allowyou to load them separtely and control them.
Only thing you can do is load them grouped OR
for every object do:

obect.transformMatrix = [1,0,0,1,0,0]

This will allow you to have control over them BUT will not give you perfect shape of the SVG.

@Canuto100
Copy link

But everything is set to ( 0,0) :( :( :( Anything I can do?

fabric.loadSVGFromString(str, function(objects) {
angular.forEach(objects, function(o, index){
o.transformMatrix = [1,0,0,1,0,0];
o.setCoords();
$scope.canvas.add(o);
});
$scope.canvas.renderAll();
});

@asturur
Copy link
Member

asturur commented Dec 10, 2014

you can do
object.left = object.transformMatrix[4];
object.top = object.transformMatrix[5];
object.transformMatrix =[1,0,0,1,0,0];
object.setCoords();

but this work just for translate. you can do something till is a matter of scale + rotate + translate. but a generic matrix is a stop for now.

@saeschdivara
Copy link
Author

If you create the svg in the first place, maybe you could use json instead.

@Canuto100
Copy link

works , thank you very much, very helpful,

Greetings from Mexico !!!

@asturur
Copy link
Member

asturur commented Aug 7, 2015

The fix is not finished otherways i would happily release it.
handling the part 4 and 5 ( translation ) of transformMatrix is what is missing.

This was referenced Sep 3, 2015
@pranavgawri
Copy link

Hi, were you able to work on this one? looking forward for fix on this one.

thanks.

@asturur asturur changed the title Shapes with transformMatrix have misplaced and/or stretched controls. [SVG IMPORT] Shapes with transformMatrix have misplaced and/or stretched controls. Oct 8, 2015
@asturur
Copy link
Member

asturur commented Oct 19, 2015

image

look! an ungrouped svg behaving normally

@asturur
Copy link
Member

asturur commented Oct 19, 2015

image

Still some gradient job to do.
This is a long wip, but i wanted to show progress so you do not loose hope :)

@asturur
Copy link
Member

asturur commented Oct 19, 2015

shapes are done.
image

image

text is missing, gradients and patterns are missing.

@pranavgawri
Copy link

Hey Thanks a lot for keeping me updated! :) I appreciate that work is in progress on it.
By any chance, the progress that you have made, is available to pull or in a version?

@asturur
Copy link
Member

asturur commented Oct 19, 2015

Not yet, lot of cleaning is necessary.
Be aware that i will not propose an half version to be fast. When this is done, pathGroup will not be used, ungrouped svg will be different shapes, grouped svg will be a normal group. So this will bring _big backward compatibility problem_ this is a 2.0 feature. I will share code if you want to use in your own way you are free to do so.

@kangax kangax added this to the 2.0.0 milestone Oct 19, 2015
@kangax
Copy link
Member

kangax commented Oct 19, 2015

I marked it for 2.0 (@asturur feel free to do it yourself in the future)

@GeorgeNarcis
Copy link

I' m facing kind of similar problem , but I'm not sure if the root cause is the same.
I am setting various viewport transforms on my fabric canvas instance: translate+scale+rotate.

At some point I am loading shapes to canvas from SVG. If the rotate transform is not set on my canvas viewport, the controls are looking fine. The scale and translate transforms seems to work as expect but when the rotate transformation is added the controls are broken, there seems that some kind of rotation is applied to them and the effect is very strange, they look totally broken.

Am I seeing the same problem as described in this bug or is a new problem (since i don t set transformmatrix on the shape but instead the transform is applied on canvas viewport)? I am using fabric 1.5.0, so any help or possible workaround for my problem would be highly appreciated.

@asturur
Copy link
Member

asturur commented Oct 22, 2015

if you are talking about viewportTransform, rotation is not supported. all viewport transform are just scaling and translate. if you wanna be sure you can open a different issue adding a fiddle with a code sample.

@asturur
Copy link
Member

asturur commented Oct 28, 2015

Currently looking to complex svg with both path and other shapes and GRADIENTS.
Positions of shapes looks correct, also control boxes are.
Gradients or colors look like a bit off sometimes.
So please for whom is interested in this fix contribute some strange svg you may have somewhere on your harddisk if opening a little bounty is not in your possibilties.

@DeDuke
Copy link

DeDuke commented Sep 15, 2016

is it possible to place the selection area via object-options?

@cgatian
Copy link

cgatian commented Mar 3, 2017

I created a simple example of importing text and converting it to an IText. The selection box is incorrect when you attempt to edit. I am assuming this is the same issue?
Thank you, and awesome library.

http://codepen.io/cgatian/pen/yMJLvQ

@krishnaisdinesh
Copy link

Hi,

In lates released 7.9 still loading sg without group scatter in canvas. Not at actual position.
fabric.loadSVGFromURL("file.svg",function(objects,options){ canvas.add.apply(canvas, objects); });
Original svg

screenshot from 2017-04-05 13-27-22

After upload
fireshot capture 078 - my yii application_ - http___localhost_diy_web_logo_edit

I need to change color or edit elements of svg. So cann't group.

@asturur
Copy link
Member

asturur commented Apr 5, 2017 via email

@asturur
Copy link
Member

asturur commented May 30, 2017

For who may be still interested, this thing is finally solved in #3969.
I m really excited about it.

@pranavgawri
Copy link

Thanks!

@asturur
Copy link
Member

asturur commented Jun 1, 2017

i m closing this issue.
Just to be clear, SVG import has been rewritten to absorb the transform matrix and respect controls.
So transform matrix get translated to left,top,scale,skew, etc etc.

Setting the transformMatrix directly yo an object will still give out of box controls.

transformaMatrix will stay as object property for who wants to draw custom things and is not in need of controls, for all other people a new method will be created ( to be named ) that will take as an input the transformMatrix, and mix it with current object properties.

This is already possible using calcTransformMatrix, multiplyTransformMatrices and qrDecompose, but maybe there will be a oneline helper

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

Successfully merging a pull request may close this issue.