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

Info - (0,0) vertex bug - iPad point move #291

Open
dobkeratops opened this issue Apr 17, 2021 · 6 comments
Open

Info - (0,0) vertex bug - iPad point move #291

dobkeratops opened this issue Apr 17, 2021 · 6 comments

Comments

@dobkeratops
Copy link

dobkeratops commented Apr 17, 2021

Repro steps on the iPad - seems like moving a vertex does result in it appearing at (0,0). It happens more often accidentally on the iPad screen.
(I chose a vertex near the top left to make a low risk error to test it)

Doesn’t seem to happen on the PC though.. :/ however there was a visual difference - the moved iPad vertex didn’t seem to change the outline. Perhaps it’s an obscure browser issue :/

Fixing the work area would make it less likely.. (fewer accidental moves) I can also set my draghandle size lower to make it less likely (and zoom in if I need to adjust)

I figured we can just ignore the vertices at (0,0). I’d prioritise work area over this unless a fix is easy.
51F0DEE1-953F-4619-B279-F7D3FF4B1AB6

@bbernhard
Copy link
Collaborator

Many thanks for the bug report! I've seen quite a few annotations with those "snapped to (0/0) vertices" in the past, but I had no clue how to reproduce that. But with your bug report I finally was able to reproduce the issue. I haven't had time to look deeper into that, but I'll definitely have a look at that one (as it's pretty annoying).

the moved iPad vertex didn’t seem to change the outline.

I think it does change the outline - at least in my experiments. It doesn't change it right away, but if you persist the changes and look at the annotation again, one point is at 0/0.

But I wouldn't worry about that too much. Once that bug is fixed, we can query all the annotations with (0/0) vertices and fix those issues manually. As it's just one point that needs to be moved, I think that should be quite fast.

@bbernhard
Copy link
Collaborator

short update: I think I've found the issue - at least it's now behaving correctly in the simulator. But before I push it to production I want to give it a try on a real tablet first - which I'll do in the next days. I'll let you now as soon as it's live.

@dobkeratops
Copy link
Author

Good news. I figure eventually manually fixing all of them will be doable . In the meantime it should be fine to just filter out those vertices.

I was thinking of estimating an accuracy per polygon.. blur the training channel. If we find a (0,0) vertex, just set that low, blur the outline a lot. Polygons with more vertices are usually more accurate. I figure most polys would be blurred by about 5-10% of the screen size.

the real concern is “1st impressions” for potential users .. they might have a knee jerk reaction that the data is untrustworthy, whereas I know there’s very few errors and they’re easy to filter out.

@bbernhard
Copy link
Collaborator

Totally agreed! As soon as the fix is pushed to production we should definitely fix the issues the bug has caused. If the number is a reasonable amount I guess we could through those manually and fix them by hand (as each annotation has a unique id we could use a url parameter to load the annotation in the browser; e.g ?annotation_id=xxxx; If I remember correctly you suggested something similar not that long ago). In case the list of annotations is too big, I guess we have to fix them with the help of a script).

I think the most difficult task will probably be to find all the affected annotations. Maybe it's already sufficient to look for all annotations with a (0/0) vertex. This will probably also return some false positives, but hopefully we don't have that many valid annotations with a (0/0) vertex - I guess most of the time you probably won't be able to hit (0/0) exactly. In case we get too many false positives we probably have to fine tune the algorithm a bit.

@bbernhard
Copy link
Collaborator

short update: The changes were pushed to production today. I also queried the database for all annotations with (0/0) vertices. (I only considered poly points here and ignored other shapes (like rectangles), as I think the problem was related to poly points only). In total I found 101 annotations with poly points at (0/0). I went through all those manually and fixed any issues I found. Out of those 101 annotations only 10 suffered from the "(0/0) vertices bug". Out of those 10 annotations 5 were somehow "completely broken" (see attached images). I am not sure how they ended up in a state like this. I guess it could be another bug, that is still hiding in there somewhere. But as those annotations all suffered from the "(0/0) vertices bug", I think it could also be a side effect of that bug. One of the things I really hate about Javascript is, that it tries to continue even when when some variables are null/undefined. I would really prefer if it would crash properly on an error instead of continuing in a half broken state. And I think that's also what could have happened here. As there were some data structures undefined and null due the "(0/0) vertices bug" it might be possible that switching between the labels in the unified mode caused those totally broken annotations.

Selection_112
Selection_111

@dobkeratops
Copy link
Author

Great news. Right the I forgot about the “completely broken” ones aswell. Not sure how to find those but at least it seems to be image wide , browsing through images will find them.
I could get into the habit of adding a label “error” if I find those just flicking through

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

No branches or pull requests

2 participants