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

boundingBox computes incorrect bounding box for transformed diagrams #39

Closed
fryguybob opened this issue Aug 4, 2012 · 2 comments
Closed

Comments

@fryguybob
Copy link
Member

(Imported from http://code.google.com/p/diagrams/issues/detail?id=87. Original issue from andreas....@gmail.com on July 25, 2012, 09:03:51 PM UTC)

What steps will reproduce the problem?

  1. The expression
    boundingBox (translate (r2 (10,10)) unitSquare :: D R2) calculates a bounding box of size 10.5 times 10.5 (instead of 1 times 1).

So the result is:
BoundingBox (P (R2 {unR2 = (0.0,0.0)})) (P (R2 {unR2 = (10.5,10.5)}))

instead of the expected output:
BoundingBox (P (R2 {unR2 = (9.5,9.5)})) (P (R2 {unR2 = (10.5,10.5)}))

I tested this with version 0.5.0.1 diagrams-lib

@fryguybob
Copy link
Member Author

(Imported. Original comment by byor...@gmail.com on July 25, 2012, 09:31:37 PM UTC)

Relevant IRC discussion:

(16:08) < fryguybob> abernstein: I just looked at boundingBox a little and I think I'm leaning "that's a bug" at the moment.
(16:09) :)
(16:09) fryguybob: ok
(16:32) < mgsloan> oops
(16:32) < mgsloan> what's wrong with it? I think I may have written it
(16:33) < fryguybob> mgsloan: fromPoints doesn't seem appropriate there. That will only work when the origin is inside the diagram.
(16:35) < fryguybob> mgsloan: For each unit vector and +/- only one value should be used, not all values.
(16:36) < mgsloan> ahh
(16:37) < mgsloan> but envelope returns negative numbers, right?
(16:37) < mgsloan> so it should work anyway
(16:37) < mgsloan> but you're right, that this is probably less efficient than possible
(16:38) < fryguybob> envelopeP gives a point, nothing negative going on there.
(16:45) < mgsloan> right, but it'll still work if the envelope does not contain the origin
(16:46) < byorgey> sounds like a bug in boundingBox, though I haven't looked into the details
(16:46) < byorgey> however, the fact that there is no Transformable instance for BoundingBox is NOT a bug
(16:47) < byorgey> in fact, there used to be one and I removed it. Applying affine transformations to bounding boxes does not make sense.
(16:47) < fryguybob> byorgey: I can see that.
(16:47) < byorgey> However, there IS a HasOrigin instance for BoundingBox (or at least there should be) so you can still translate them
(16:48) < byorgey> but using 'moveTo' and friends instead of 'translate'
(16:49) < mgsloan> right, if you want to apply affine transforms, you should use getAllCorners, transform those, and use fromPoints
(16:49) < byorgey> sure, that will give a conservative approximation to a transformed bounding box
(16:49) < byorgey> but it's not compositional
(16:50) i tried boundingBox (moveTo (p2 (10,10)) unitSquare :: D R2), but i still get a 10.5 times 10.5 box
(16:51) < byorgey> abernstein: and what happens if you do (moveTo (p2 (10,10)) (boundingBox (unitSquare :: D R2))) ?
(16:52) < abernstei> byorgey: that works :)
(16:53) < byorgey> oh, I see what the problem is
(16:53) < byorgey> I think
(16:54) < fryguybob> byorgey: envelopP give a point on the hyperplane, not a point on the bounding box.
(16:54) < byorgey> yes, exactly
(16:54) < fryguybob> (as it should)
(16:54) < byorgey> and in this case it happens to be giving points on each of the four axes.
(16:55) < fryguybob> Right
(16:55) < byorgey> hmm, so what's the right way to implement it
(16:56) < fryguybob> if you take the magnitudes (or use some basis function for efficiency) you can take the combinations of mins an maxs to get the box
(17:01) < fryguybob> If no one figures out something before tonight I'll come up with a fix when I'm not at work :D.
(17:01) < byorgey> ok =)
(17:01) < byorgey> yes, I guess you can use http://hackage.haskell.org/packages/archive/vector-space/0.8.2/doc/html/Data-Basis.html#v:decompose
(17:02) < byorgey> just use envelopeV then call decompose and extract the proper coordinates

@byorgey
Copy link
Member

byorgey commented Aug 5, 2012

Fixed by 77ec072

@byorgey byorgey closed this as completed Aug 5, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants