- Notifications
You must be signed in to change notification settings - Fork 185
Circle selector addition #17
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
Changes from all commits
521f9ee
c1d3264
5959c4a
3098a5c
9b75188
7880648
4b4e6f2
08c8956
a99d2d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
| @@ -47,10 +47,21 @@ export type Polygon = {| | |
open?: boolean, | ||
points: Array<[number, number]> | ||
|} | ||
export type Circle = {| | ||
...$Exact<BaseRegion>, | ||
type: "circle", | ||
// radius: number, | ||
// x and y indicate the coordinates of the centre of the circle | ||
x: number, | ||
y: number, | ||
// xr and yr indicate the x and y coordinates of the current mouse pointer while dragging. | ||
xr: number, | ||
yr: number | ||
|} | ||
| ||
export type Region = Point | PixelRegion | Box | Polygon | ||
| ||
export const getEnclosingBox = (region: Region) => { | ||
export const getEnclosingBox = (region: Region, iw: number, ih: number) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's unclear to me why
If possible, we should remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I mentioned in my previous comment, xr and yr aren't actually representing the radius exactly but are representing the coordinates to last point while dragging by a mouse. So, I need ih and iw. This works if xr is x-radius and yr is y-radius. Reason I need ih and iw in the reducer: | ||
switch (region.type) { | ||
case "polygon": { | ||
const box = { | ||
| @@ -90,6 +101,18 @@ export const getEnclosingBox = (region: Region) => { | |
return box | ||
} | ||
} | ||
case "circle": { | ||
const radius = Math.sqrt(Math.pow((region.xr-region.x)*iw,2) + Math.pow((region.yr-region.y)*ih,2)) | ||
const box = { | ||
x: (region.x*iw - radius)/iw, | ||
y: (region.y*ih - radius)/ih, | ||
w: 0, | ||
h: 0 | ||
} | ||
box.w = (region.x*iw + radius)/iw - box.x | ||
box.h = (region.y*ih + radius)/ih - box.y | ||
return box | ||
} | ||
} | ||
throw new Error("unknown region") | ||
} | ||
| @@ -102,6 +125,9 @@ export const moveRegion = (region: Region, x: number, y: number) => { | |
case "box": { | ||
return { ...region, x: x - region.w / 2, y: y - region.h / 2 } | ||
} | ||
case "circle": { | ||
return { ...region, x, y, xr: region.xr + x - region.x, yr: region.yr + y - region.y } | ||
} | ||
} | ||
return region | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm lukewarm on this definition because technically in a circle
xr=yr
so it should really just haver
. My suggestion is to have an optionalr?: number
, which is set in the case wherexr=yr
to ber=xr=yr
because this is more consistent with expectation when dealing with circles.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seveibar, I started with a single r as you are suggesting but, since we are transforming the coordinates based on image height and width, single r won't be enough.
In the current code, xr and yr aren't actually representing the radius exactly but are representing the coordinates to last point while dragging by a mouse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gotcha