[Bug]: Loading canvas object with path on Itext

radykal commented 1 year ago

CheckList

[X] I agree to follow this project's Code of Conduct
[X] I have read and followed the Contributing Guide
[X] I have read and followed the Issue Tracker Guide
[X] I have searched and referenced existing issues and discussions
[X] I am filing a BUG report.
[X] I have managed to reproduce the bug after upgrading to the latest version
[X] I have created an accurate and minimal reproduction

Version

5.3.0

In What environments are you experiencing the problem?

Firefox, Chrome, Safari

Node Version (if applicable)

None

Link To Reproduction

https://jsfiddle.net/aj5bueot/

Steps To Reproduce

There is a bug when you reload a canvas object that includes an Itext Object with a path parameter. It does work with fabric.Text.

Bug is inside _render method of fabric.Text: path && !path.isNotVisible() && path._render(ctx);

Expected Behavior

No bug in console.

Actual Behavior

Throwing a TypeError.

Error Message & Stack Trace

bric.min.js:1 Uncaught TypeError: e.isNotVisible is not a function
    at i._render (fabric.min.js:1:255402)
    at i.h [as callSuper] (fabric.min.js:1:26128)
    at i._render (fabric.min.js:1:273532)
    at i._render (fabric.min.js:1:26779)
    at i.drawObject (fabric.min.js:1:147682)
    at i.renderCache (fabric.min.js:1:146405)
    at i.render (fabric.min.js:1:146007)
    at i.h [as callSuper] (fabric.min.js:1:26128)
    at i.render (fabric.min.js:1:266710)
    at i.render (fabric.min.js:1:26779)
_

ShaMan123 commented 1 year ago

I am guessing the path doesn't get enlivened which is weird

ShaMan123 commented 1 year ago

fixed in v6 repro

asturur commented 1 year ago

yes this is fixed in v6 but the bug was on 5.3 indeed, this needs to be fixed

shashank-brightness commented 1 year ago

Can you please tell what changes we need to do here. I am trying to make undo redo on canvas but when I do something like first setting path and then trying to move object and then press undo, I am getting this error. I am also getting this with Text object also, and not just only with IText. I am using 5.3.0.

For time being, I was temporarily able to fix by adding isNotVisible function when creating path instance for text's path property. I added always not visible to true as i anyhow don;t want to make it visible.

const textPath = new fabric.Path(pathData, {
    strokeWidth: 1,
    visible: false,
    isNotVisible: function () {
        return true;
    },
});

activeObj?.set({
    path: textPath,
    pathSide: 'center',
    pathStartOffset: 0,
    selectable: true,
});

Jonnas123 commented 1 year ago

Same here, after moving the object and click undo, then the error comes. I try your solution shashank-brightness but the object disapears.

shashank-brightness commented 1 year ago

@Jonnas123 You can fix this by giving path object the protorype of the path when you need to again attach to the object. It disappears in above code as i am always returning true from isNotVisible

Object.setPrototypeOf(textPath, fabric.Path.prototype)

Jonnas123 commented 1 year ago

@shashank-brightness I've checked again. For my case it happens only with IText. No error if I use Text property. Here is a demonstration. The left one is IText, you can see it can be double clicked and placed the cursor inside (not normal behavour) while the right one is Text , which works without this error.

Curve text

If you don't mind telling me how to put Object.setPrototypeOf(textPath, fabric.Path.prototype) into below

function CurveOne() {
    var CurveOneValue = document.getElementById('curveoneval').value;
    var path = new fabric.Path('M236.949,468.245a160.648,160.648,0,0,1,321.3,0');
       //Like this? but not working
        Object.setPrototypeOf(CurveOneValue, fabric.Path.prototype);
    path.set({
        fill: "transparent"
    });
    var CurveOneText = new fabric.IText(CurveOneValue, {
        fontSize: 30,
        fontWeight: "normal",
        textAlign: 'center',
        pathAlign: "center",
        objectCaching: false,
        path: path,
    });
    canvas.add(CurveOneText);
    canvas.centerObject(CurveOneText);
}

sauravkumar3838 commented 8 months ago

@asturur @radykal Can you please tell how to resolve this error in v5.3.0 ? I am implementing undo feature by saving the canvas state.

asturur commented 8 months ago

I think we said there was a bug in 5.x that doesn't allow to enlive that back.

asturur commented 8 months ago

Yes this should be fixed should be also easy to fix

Shivamc489 commented 7 months ago

hey @asturur still didn't got you. How to fix this issues in 5.3, as I can't move to beta currently and I need help in fixing the issue in 5.3 itself. For me it happens when I make a text on path and then import it.

This is how I am generating curved text textObject.set({ path: path });

After that, I export the json.

This is the sample json when imported gives this error:-

{
    "version": "5.3.0",
    "objects": [
        {
            "type": "textbox",
            "version": "5.3.0",
            "originX": "left",
            "originY": "top",
            "left": 344,
            "top": 153,
            "width": 142.01,
            "height": 40.68,
            "fill": "#ad3838",
            "stroke": null,
            "strokeWidth": 1,
            "strokeDashArray": null,
            "strokeLineCap": "butt",
            "strokeDashOffset": 0,
            "strokeLineJoin": "miter",
            "strokeUniform": false,
            "strokeMiterLimit": 4,
            "scaleX": 1,
            "scaleY": 1,
            "angle": 0,
            "flipX": false,
            "flipY": false,
            "opacity": 1,
            "shadow": null,
            "visible": true,
            "backgroundColor": "",
            "fillRule": "nonzero",
            "paintFirst": "fill",
            "globalCompositeOperation": "source-over",
            "skewX": 0,
            "skewY": 0,
            "fontFamily": "Comic Sans MS",
            "fontWeight": "bold",
            "fontSize": 36,
            "text": "Heading",
            "underline": false,
            "overline": false,
            "linethrough": false,
            "textAlign": "center",
            "fontStyle": "normal",
            "lineHeight": 1.16,
            "textBackgroundColor": "",
            "charSpacing": 0,
            "styles": [],
            "direction": "ltr",
            "path": {
                "type": "Path",
                "version": "5.3.0",
                "originX": "left",
                "originY": "top",
                "left": -0.5,
                "top": -0.5,
                "width": 500,
                "height": 0,
                "fill": "",
                "stroke": "",
                "strokeWidth": 1,
                "strokeDashArray": null,
                "strokeLineCap": "butt",
                "strokeDashOffset": 0,
                "strokeLineJoin": "miter",
                "strokeUniform": false,
                "strokeMiterLimit": 4,
                "scaleX": 1,
                "scaleY": 1,
                "angle": 0,
                "flipX": false,
                "flipY": false,
                "opacity": 1,
                "shadow": null,
                "visible": true,
                "backgroundColor": "",
                "fillRule": "nonzero",
                "paintFirst": "fill",
                "globalCompositeOperation": "source-over",
                "skewX": 0,
                "skewY": 0,
                "path": [
                    [
                        "M",
                        0,
                        145.5
                    ],
                    [
                        "Q",
                        250,
                        -145.5,
                        500,
                        145.5
                    ]
                ]
            },
            "pathStartOffset": 0,
            "pathSide": "left",
            "pathAlign": "baseline",
            "minWidth": 20,
            "splitByGrapheme": false,
            "optional": {}
        },
        {
            "id": "textCurvePath",
            "type": "Path",
            "version": "5.3.0",
            "originX": "left",
            "originY": "top",
            "left": -0.5,
            "top": -0.5,
            "width": 500,
            "height": 0,
            "fill": "",
            "stroke": "",
            "strokeWidth": 1,
            "strokeDashArray": null,
            "strokeLineCap": "butt",
            "strokeDashOffset": 0,
            "strokeLineJoin": "miter",
            "strokeUniform": false,
            "strokeMiterLimit": 4,
            "scaleX": 1,
            "scaleY": 1,
            "angle": 0,
            "flipX": false,
            "flipY": false,
            "opacity": 1,
            "shadow": null,
            "visible": true,
            "backgroundColor": "",
            "fillRule": "nonzero",
            "paintFirst": "fill",
            "globalCompositeOperation": "source-over",
            "skewX": 0,
            "skewY": 0,
            "path": [
                [
                    "M",
                    0,
                    145.5
                ],
                [
                    "Q",
                    250,
                    -145.5,
                    500,
                    145.5
                ]
            ]
        }
    ],
    "background": "#fff"
}

asturur commented 7 months ago

i ll try to get back to you monday i didn't have time to handle it

Shivamc489 commented 7 months ago

i ll try to get back to you monday i didn't have time to handle it

sure sure, no worries, take your time. Enjoy!!

asturur commented 7 months ago

So the reason why it doesn't load correctly is that we added the restore code to Text and not IText or Textbox and i think the reason is because interactivity at this point is still broken. ( i plan to fix that in 6.x anyway ) I can copy paste the restore code in the other 2 classes and push it out as 5.4.0 if it really helps you, i could have time tonight

Shivamc489 commented 7 months ago

Yeah, that would be so helpful. As, there are some breaking changes in 6.x, it would be best if it comes in 5.4.0. Thanks a lot 🙏

asturur commented 7 months ago

5.4.0 will be released just to punch in a bunch of broken things we noticed while working on v6 All next work will be v6 onward.

Il giorno mer 1 mag 2024 alle ore 04:45 Shivam Chauhan < @.***> ha scritto:

Also, @asturur https://github.com/asturur 1 question, will 5.4.0 be realeased as beta or LTS version. I am new to open source and don't know how this works or what rules you use in this community. If you can guide me to a ruleset (if we have one) would be helpful. Might encourage me to do some contributions also in future.

— Reply to this email directly, view it on GitHub https://github.com/fabricjs/fabric.js/issues/8865#issuecomment-2087895321, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJDQQFKGFC4WOWD56FTDQ3ZABJMHAVCNFSM6AAAAAAXM64QVSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBXHA4TKMZSGE . You are receiving this because you were mentioned.Message ID: @.***>

thijsbouwes commented 6 months ago

@asturur on V6 (6.0.0-rc.1) I have the same issue when exporting/importing canvas that contains text on a path jsfiddle.

When do you expect 5.4 to be released?

asturur commented 5 months ago

Hey please check if this build fix this issue: https://github.com/fabricjs/fabric.js/pull/9965 If i don't get feedback i won't publish it.

thijsbouwes commented 5 months ago

@asturur thanks for the fix, exporting/importing canvas that contains text on a path is working now 👍

Shivamc489 commented 5 months ago

Hey @asturur thanks, this works like a charm 🫡

fabricjs / fabric.js