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

getBoundingBox improvements #121

Merged
merged 13 commits into from Oct 19, 2012
Merged

getBoundingBox improvements #121

merged 13 commits into from Oct 19, 2012

Conversation

padolsey
Copy link
Contributor

What was previously the getComputed method is now getBoundingBox.

It takes on optional argument, a Matrix instance which will be used to apply transforms to the calculated bounding-box.

var shape = new Rect(50, 50, 100, 100);

// No transform applied, -- it'll return the bounding-box
// within shape's own coordinate system:
shape.getBoundingBox(); // ~ 0,0,100,100

// Apply shape's own matrix to the calculation
// (gives position in outer coordinate system):
shape.getBoundingBox( shape.attr('matrix') ); // ~ 50,50,100,100

getBoundingBox always returns an object in the form:

{
  top: n,
  left: n,
  width: n,
  height: n,
  bottom: n,
  right: n
}

Included in this pull request is also an algorithm to determine the correct extrema of cubic bezier curves. Checkout the branch and open the bounding-box.js example to see a demonstration of the new getBoundingBox calculation.

@basecode
Copy link
Contributor

  • Afaik we agreed on x and y as aliases for top and left.
  • API wise I think we should think about a convenient wrapper for shape.getBoundingBox( shape.attr('matrix') );
    maybe shape.getBoundingBox(true); => I know true says nothing, but you get the point.

bounds[0].push(p3[0]);
bounds[1].push(p3[1]);

for (var i = 0; i < 2; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a URL that points to the origin of this approach and if possible add some comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the worst part of the implementation because I am not sure how it works. I ported it from a python script I found here: http://blog.hackers-cafe.net/2009/06/how-to-calculate-bezier-curves-bounding.html

I tried understanding the math on the article you linked to but in the end, after struggling, I grabbed this and ported it and it seemed to work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@padolsey please add his MIT License note to our LICENSE file.

@iamdustan
Copy link
Contributor

For related issues and discussions:

These should all be closable when this is merged.

* @param {Matrix} [transform=null] A transform to apply to all points
* before computation.
* @return {Object}
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc block still has the key parameter

@davidaurelio
Copy link
Contributor

Awesome demo. I’ve added a few remarks. Can’t verify the bounds algorithm for cubic bezier curves (no time), but it seems to work.

@davidaurelio
Copy link
Contributor

ready for merge? further objections?

@padolsey
Copy link
Contributor Author

Just to note: This pull request does not fix #2

davidaurelio pushed a commit that referenced this pull request Oct 19, 2012
getBoundingBox improvements

Fixes #74
Fixes #116
@davidaurelio davidaurelio merged commit c1c55a5 into master Oct 19, 2012
@iamdustan
Copy link
Contributor

@padolsey very nice demo, indeed! 👍
I think that would be worth adding to http://demos.bonsaijs.org/

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.

None yet

5 participants