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

Error in Fabric 5.0 #43

Open
marcohamersma opened this issue May 18, 2022 · 5 comments
Open

Error in Fabric 5.0 #43

marcohamersma opened this issue May 18, 2022 · 5 comments

Comments

@marcohamersma
Copy link

marcohamersma commented May 18, 2022

I know the readme states that the library is only available for version 3 and 4, but I figured I would make an issue anyway to make sure people know that there is currently a few errors/bugs.

error in _reset

it seems in the _reset method PSBrush calls this._setBrushstyles() (a function it inherits from fabric) with no arguments, but it seems that in version 5 _setBrushStyles expects the context to be passed as the first argument:

  _setBrushStyles: function _setBrushStyles(ctx) {
    ctx.strokeStyle = this.color;
    ctx.lineWidth = this.width;
    ctx.lineCap = this.strokeLineCap;
    ctx.miterLimit = this.strokeMiterLimit;
    ctx.lineJoin = this.strokeLineJoin;
    ctx.setLineDash(this.strokeDashArray || []);
  },

I managed to monkeypatch this fix this in my code by doing the following

const baseBrush = new PSBrush(canvas);
const original = this.baseBrush._setBrushStyles;
  this.baseBrush._setBrushStyles = function _setBrushStyles(ctx) {
  if (!ctx) ctx = this.canvas.contextTop;
  original(ctx);
};

Brush color

When the brush's color is set to a color, the brush will still show up as black while drawing, only switching to the proper color after finishing the stroke

Screen.Recording.2022-05-23.at.18.02.39.mov

Peer dependency

Additionally, it would be good if Fabricjs became a peerDependency, rather than a dependency.

@arcatdmz
Copy link
Member

arcatdmz commented Jun 1, 2022

@marcohamersma Thanks, your suggestion all sounds great and reasonable! Let me look into Fabric 5.0, make appropriate changes, and move Fabric.js to peerDependency. (Also, please feel free to submit a PR as I might not have enough time in coming several weeks.)

@r-debashis
Copy link

I am also facing the same issue with fabric 5+. Would love to use this library for our next release.

@marcohamersma
Copy link
Author

marcohamersma commented Aug 16, 2022

I've created a pull request for these fixes :)

There is however another issue that I'm having with Fabric's new-ish Eraser functionality/mixin. This among other things adds an eraser property to Fabric.Object which can store a Fabric.Eraser object if the brush stroke intersects with the eraser.

The problem occurs when running canvas.loadFromJSON; the PSBrush seems to not restore the plain "object" variation of eraser to a proper Fabric.Eraser. I'm trying to find out where the issue is exactly. My experiments in this codesandbox show that regular brush strokes are not affected by this issue.

So I think the issue is with the PSStroke.fromObject override, but I'm still trying to pinpoint it exactly (when I have some time). If anyone else has some ideas for a fix, that would be super helpful

@Zhuxb-Clouds
Copy link

I've created a pull request for these fixes :)

There is however another issue that I'm having with Fabric's new-ish Eraser functionality/mixin. This among other things adds an eraser property to Fabric.Object which can store a Fabric.Eraser object if the brush stroke intersects with the eraser.

The problem occurs when running canvas.loadFromJSON; the PSBrush seems to not restore the plain "object" variation of eraser to a proper Fabric.Eraser. I'm trying to find out where the issue is exactly. My experiments in this codesandbox show that regular brush strokes are not affected by this issue.

So I think the issue is with the PSStroke.fromObject override, but I'm still trying to pinpoint it exactly (when I have some time). If anyone else has some ideas for a fix, that would be super helpful

The same issue - have you already resolved it or do you have any progress to share?

@marcohamersma
Copy link
Author

I have no progress on this, I refrained from updating Fabric for the time being

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

No branches or pull requests

4 participants