ImageMonkey / imagemonkey-core

ImageMonkey is an attempt to create a free, public open source image dataset.
https://imagemonkey.io
47 stars 10 forks source link

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

Open dobkeratops opened 3 years ago

dobkeratops commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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