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

Setting position in Shape.Rectangle constructor causes incorrect bounds calculations #1686

Closed
nikkwong opened this issue Jul 6, 2019 · 2 comments · Fixed by #1708
Closed
Labels
status: PR proposed an active pull request should fix this issue type: bug

Comments

@nikkwong
Copy link

nikkwong commented Jul 6, 2019

var shape1 = new Shape.Rectangle({
        point: [20, 20],
        size: new paper.Size(100, 100),
        fillColor: 'blue'
});

const shape2 = new Shape.Rectangle({
        fillColor: 'red',
        position:  shape1.position,
        size: new paper.Size(50, 50)
})

console.log(shape1.bounds.area) // 10000
console.log(shape2.bounds.area) // 0

Sketch

@nikkwong
Copy link
Author

nikkwong commented Jul 6, 2019

Somewhat unrelated but also wonder why you can't just copy bounds from one item to the other.

var shape1 = new Shape.Rectangle({
        size: new paper.Size(100, 100),
        fillColor: 'blue'
});

const shape2 = new Shape.Rectangle({
        fillColor: 'red',
        bounds:  shape1.bounds
})

console.log(shape1.bounds.area) // 10000
console.log(shape2.bounds.area) // 0

If bounds is supposed to be readonly, it would be nice to note that somewhere.

sasensi added a commit to sasensi/paper.js that referenced this issue Aug 20, 2019
In some special circumstances, when position was passed in constructor
and when position key was before size key, bounds were wrongly
calculated.
This ensure that when size is set, even the first time, bounds are
properly recalculated.
Closes paperjs#1686
@sasensi
Copy link
Contributor

sasensi commented Aug 20, 2019

@nikkwong, thank you for the report.
The bug was actually linked to the fact that position was set before size, resulting in invalid bounds being cached. #1708 will fix it.

About your second statement, yes, I think that bounds are supposed to be readonly and we are working on improving the readonly states in the documentation along with the typescript documentation improvement.
@lehni, can you confirm that every variant of bounds (bounds, strokeBounds, ...) should be marked as readonly ? If yes, I can make a PR to fix the documentation.

@sasensi sasensi added status: PR proposed an active pull request should fix this issue type: bug labels Aug 20, 2019
lehni pushed a commit that referenced this issue Dec 15, 2019
In some special circumstances, when position was passed in constructor
and when position key was before size key, bounds were wrongly
calculated.
This ensure that when size is set, even the first time, bounds are
properly recalculated.
Closes #1686
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: PR proposed an active pull request should fix this issue type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants