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

Rectangle in Math needs to specify y down #15943

Open
DartBot opened this issue Jan 7, 2014 · 6 comments
Open

Rectangle in Math needs to specify y down #15943

DartBot opened this issue Jan 7, 2014 · 6 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-math type-enhancement A request for a change that isn't a bug

Comments

@DartBot
Copy link

DartBot commented Jan 7, 2014

This issue was originally filed by ti...@xythings.com


_RectangleBase documents y being down but this is not shown in Rectangle

Appearing in Math and not a graphics package it would be reasonable to assume y up for a Rectangle.

https://api.dartlang.org/docs/channels/stable/latest/dart_math/Rectangle.html

@sethladd
Copy link
Contributor

sethladd commented Jan 7, 2014

Removed Type-Defect label.
Added Type-Enhancement, Area-Library, Triaged labels.

@lrhn
Copy link
Member

lrhn commented Jan 8, 2014

cc @efortuna.

@lrhn
Copy link
Member

lrhn commented Jan 8, 2014

I agree that this is very surprising for a Rectangle in dart:math.
The default assumption would be Cartesian coordinates.

@efortuna
Copy link
Contributor

efortuna commented Jan 8, 2014

I'm 90% certain we had this discussion back when the code was going in, and you and Florian opted to have "graphics coordinates" in the default case. I can dredge up the email thread.... Back when we were writing this code I did a survey of other language's Rectangle classes, and by and large they used the computer graphics coordinates. The one exception being a specialized math third-party Python package, but again in the default Python rectangle y was down.

We could update the documentation... or change the implementation... I can see rationale for both sides, but adding the Cartesian coordinates does complicate the implementation (not that that should be the determining factor).

@lrhn
Copy link
Member

lrhn commented Jan 9, 2014

Even when I know it is "y down" coordinates, it still keeps surprising me :)
It's probably still the best choice, we just need to reduce the surprise, so the documentation need to be very explicit and up-front about it (like mentioned in first line of the class documentation, and again on the field getter and the constructor parameter).

@DartBot
Copy link
Author

DartBot commented Jan 9, 2014

This comment was originally written by ti...@xythings.com


Documentation is the main thing and I'm happy enough with it just being clear.
It takes a while reading the java Rectangle2D javadoc before it is obvious that it is a
y down implementation (though it is in the awt package).

On a more general level why is there a Rectangle class, and why is it in Math?
(sorry efortuna@google.com if this has been covered before) -
By Rectangle what is meant is a 'y down' rectangle because historically this was
used in raster, scanning implementations and this leaked into other code.
For these sorts of code the Rectangle implementation works but it should not
be confused with this being a general Rectangle, it is a very specific type of Rectangle.

If the package was math.graphics or graphics.math everything would be fine but
a math package implies for me a degree of abstraction, (I would prefer Point to
be Point2D and to use less significant dimension variables than x and y).
A rectangle implementation defined by points can provide max dimension methods
without loss of abstraction, but a constructor with width and height will need to
define the co-ordinate system first.
and so on...

@DartBot DartBot added Type-Enhancement area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. labels Jan 9, 2014
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed priority-unassigned labels Feb 29, 2016
@lrhn lrhn added core-a and removed core-a labels Aug 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-math type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants