Skip to content

Conversation

@luizzappa
Copy link
Contributor

Motivation

The calculation of the poly's BBOX alters the original height and width values ​​of the object's, causing a different behavior from other fabricjs objects and making the return of the _getNonTrasformedDimensions function incorrect.

Description

It introduces resizingOffset property that stores how much we must add to the width and height of the object to find the correct value for the BBOX and does not change the originals values of width and height.

Changes

In addition of the resizingOffset property, extend transformMatrixKey given that the properties strokeUniform, strokeLineJoin, strokeLineCap, strokeMiterLimit change the width/height of the BBOX, but the original matrix transform cache does not take into account these changes.

This was causing a bug in the controls when changing some property that the cache is not aware of.

image

As the strokeMiterLimit may or may not change the final width/height, I thought it would be better to store the resizingOffset property in the cache key instead of each of the properties individually (storkeLineJoin, StrokeLineCap, strokeUniform, strokeMiterLimit)

  transformMatrixKey(...args: any): string {
    const oldKey = super.transformMatrixKey(...args),
      newPart = this.resizingOffset || '';
    return oldKey + newPart;
  }

@luizzappa luizzappa marked this pull request as draft November 20, 2022 15:53
@ShaMan123
Copy link
Contributor

diff

@ShaMan123
Copy link
Contributor

ShaMan123 commented Nov 20, 2022

If I understand correctly @asturur means we are going to take a closer look at width/height in general. So this effort might be redundant. Hopefully we will discuss it in the upcoming days.

@stale
Copy link

stale bot commented Dec 31, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs 😔. Thank you for your contributions.

@stale stale bot added the stale Issue marked as stale by the stale bot label Dec 31, 2022
@ShaMan123 ShaMan123 removed the stale Issue marked as stale by the stale bot label Jan 1, 2023
@stale
Copy link

stale bot commented Mar 20, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs 😔. Thank you for your contributions.

@ShaMan123
Copy link
Contributor

we will look at this in the context of #8767

@ShaMan123
Copy link
Contributor

@luizzappa would want to try to do polys in #8767 ?

@luizzappa
Copy link
Contributor Author

@luizzappa would want to try to do polys in #8767 ?

@ShaMan123, working on fabric.js internals is always rewarding (and most of the time fun) :)

The only problem is that I don't have a lot of time, so I need a direction of what has to be done and preferably smaller blocks. If the task fits these requirements, you can count on me. Trying to understand everything that is happening in that PR would take some time that I don't have at the moment.

@ShaMan123
Copy link
Contributor

Great
I will ping you when I think it is time
And explain it all

@stale
Copy link

stale bot commented May 21, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs 😔. Thank you for your contributions.

@stale stale bot added the stale Issue marked as stale by the stale bot label May 21, 2023
@stale stale bot closed this Jun 18, 2023
@ShaMan123 ShaMan123 reopened this Jun 19, 2023
@stale stale bot removed the stale Issue marked as stale by the stale bot label Jun 19, 2023
@ShaMan123
Copy link
Contributor

closing in favor of #8374

@ShaMan123 ShaMan123 closed this Jul 19, 2023
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

Successfully merging this pull request may close these issues.

2 participants