thednp / svg-path-commander

Typescript tools for advanced processing of SVG path data.
https://thednp.github.io/svg-path-commander/
MIT License
228 stars 19 forks source link

Inverted "flipX()" and "flipY()" #13

Closed Fennec-hub closed 2 years ago

Fennec-hub commented 2 years ago

svg-path-commander.js#L252

flipX() and flipY() should be inverted, flipX() is flipping the y coordinates and flipY() the x coordinates ... Not a big deal, but worth mentioning.

Great library by the way, I'm using it to create a small SVG path editor so big thanks.

thednp commented 2 years ago

The logic is correct:

  flipX() {
    this.transform({ rotate: [180, 0, 0] }); // first value corresponds to X
    return this;
  }
  flipX() {
    this.transform({ rotate: [0, 180, 0] }); // second value corresponds to Y
    return this;
  }
thednp commented 2 years ago

Great library by the way, I'm using it to create a small SVG path editor so big thanks.

Link it in.

Fennec-hub commented 2 years ago

If by flipping you mean rotating, then yes the logic is correct, but most people think of flipping as mirroring and you can see this behavior in a lot of graphic libraries like fabric.js, konva.js, three.js ... so the logic would be more of a reverse scale.

 flipX() {
    this.transform({ scale: [-1, 1, 1] }); 
    return this;
 }

 flipY() {
    this.transform({ scale: [1, -1, 1] }); 
    return this;
 }

Link it in.

It's part of an ongoing project, the SVG editor part will look more or less like this demo here ... I'll keep you in touch when the project is done.

Fennec-hub commented 2 years ago

I just noticed that the transform method don't handle all scale axis so in this case just inverting the two methods works fine.

  flipX() {
    this.transform({ rotate: [0, 180, 0] }); // Rotate the Y axes, mirror the X axes
    return this;
  }
  flipY() {
    this.transform({ rotate: [180, 0, 0] }); // Rotate the X axes, mirror the Y axes
    return this;
  }
thednp commented 2 years ago

I just noticed that the transform method don't handle all scale axis so in this case just inverting the two methods works fine.

Yes it does. Perhaps we have a bug?

Fennec-hub commented 2 years ago

Yes it does and it works ... please don't mind my last post.

thednp commented 2 years ago

@Fennec-hub you are right about flipX / flipY, our original source in SVGPath was like that and I changed it at some point.

I remember I was working with font-icons -> SVG icons and it didn't make sense to me I couldn't flip icons upside-down because I was confused on why flipX did not return the expected outcome.

I just checked the native DOMMatrix interface and it does exactly the same as you expected, so...

The future version will fix that.

Fennec-hub commented 2 years ago

Glad to be of help and thanks again for assembling this great library

thednp commented 2 years ago

@Fennec-hub please have a look at the latest master. Have fun