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

fix(add bbox) : the output does not give correct bbox info #21

Merged
merged 5 commits into from Aug 13, 2021

Conversation

yiakwy
Copy link
Contributor

@yiakwy yiakwy commented Aug 5, 2021

  1. the output for coco dataset type does not generate correct output info

before (note bbox is [0, 0, 0, 0]):
before fix

fixed:
fix

  1. also generates correct bbox annotation QGraphic widget which is bettern than default "boundingRect":

add bbox

@geoyee
Copy link
Member

geoyee commented Aug 6, 2021

Thanks for your efforts! Sorry that the one who was responsible for COCO can't check it now. He is on the airplane and will confirm and reply as soon as possible.

@yiakwy
Copy link
Contributor Author

yiakwy commented Aug 8, 2021

Thanks for your efforts! Sorry that the one who was responsible for COCO can't check it now. He is on the airplane and will confirm and reply as soon as possible.

No problem. Just freshly add a patch to compute bbox width and height:

        # according to Qt canvas doc, the smallest corner point is located in "topLeft" while the maximum corner point lies in "bottomRight"
        self.w = self.corner_points[3].x() - self.corner_points[1].x()
        self.h = self.corner_points[3].y() - self.corner_points[1].y()

@linhandev
Copy link
Member

Hi, thanks so much for this great pr. But I hope you can fix some minor bugs before we merge it.

image
After a single left click on a polygon, I think it maybe more helpful to show user solid color only in the polygon not in the entire bb region. The current implementation makes it a bit more difficult to see target boundry.

I suspect ur implementation created an additional polygon on the canvas which blocks the mouse event in deeper objects. The polygon could be dragged in previous implementation and we hope not to damage that.

Hope you can try to solve these two. If unable we can also merge this and try to do it ourselves.

@yiakwy
Copy link
Contributor Author

yiakwy commented Aug 9, 2021

Hi, thanks so much for this great pr. But I hope you can fix some minor bugs before we merge it.

image
After a single left click on a polygon, I think it maybe more helpful to show user solid color only in the polygon not in the entire bb region. The current implementation makes it a bit more difficult to see target boundry.

I suspect ur implementation created an additional polygon on the canvas which blocks the mouse event in deeper objects. The polygon could be dragged in previous implementation and we hope not to damage that.

Hope you can try to solve these two. If unable we can also merge this and try to do it ourselves.

the problem

Sure, If you want bbox is movable, we can make it.

  1. should bbox movable?

Previously I make the widget not responsible to "drag" events by setting

# bbox.py
self.setFlag(QtWidgets.QGraphicsItem.ItemIsMovable, False)

This will also prevent parent from moving when the child is selected:

# bbox.py quick fix
self.setFlag(QtWidgets.QGraphicsItem.ItemIsSelectable, True)

unset the flag, you can drag the parent again:
Screenshot from 2021-08-09 12-12-05

  1. what happen when click onto bbox ?

the parent events emit first, then the child bbox.

hover event :
Screenshot from 2021-08-09 11-27-57

currently only responsible to polygon hover

click event:

the parent method "PolygonAnnotation.focusInEvent" does not response and hence the element is not focused
while the child responses:

# bbox.py
    def focusInEvent(self, ev):
        if self.parent is not None and self.parent.hasFocus():
            return
        self.setBrush(self.borderColor)

A quick fix

# bbox.py
    def _init__(...):
       ...
       self.setFlag(QtWidgets.QGraphicsItem.ItemIsSelectable, False)
       ...

    def focusInEvent(self, ev):
        if self.parent is not None and self.parent.hasFocus():
            return
        # self.setBrush(self.borderColor)

should do what you want

@yiakwy
Copy link
Contributor Author

yiakwy commented Aug 10, 2021

Hi, @linhandev the unwanted behavior fixed.

@linhandev linhandev merged commit a330bc8 into PaddleCV-SIG:main Aug 13, 2021
@linhandev
Copy link
Member

Thank u so much for ur update. This is largely exactly we expect. The pr has been merged.

We sincerely hope that u can join our development team given the knowledge and skills u show doing this. Should u be interested u can reach me at me@linhan.email.

@yiakwy
Copy link
Contributor Author

yiakwy commented Aug 13, 2021

Thank u so much for ur update. This is largely exactly we expect. The pr has been merged.

We sincerely hope that u can join our development team given the knowledge and skills u show doing this. Should u be interested u can reach me at me@linhan.email.

cool

@geoyee
Copy link
Member

geoyee commented Aug 13, 2021

@yiakwy There are some changes in bbox.py. What do you think about them?

  1. The bbox's color is gray and style is Qt.DashDotLine.
  2. Rm mouse and keyboard events, it affects the range of interaction (Can't click inside the rectangle) and del polygons.

@yiakwy
Copy link
Contributor Author

yiakwy commented Aug 15, 2021

@yiakwy There are some changes in bbox.py. What do you think about them?

  1. The bbox's color is gray and style is Qt.DashDotLine.
  2. Rm mouse and keyboard events, it affects the range of interaction (Can't click inside the rectangle) and del polygons.

Hi, here is my suggestion:

  1. As part of annotation, bbox is typically used with same color with other annotation info and painted in real line.
  2. sure no problem

@geoyee
Copy link
Member

geoyee commented Aug 16, 2021

@yiakwy There are some changes in bbox.py. What do you think about them?

  1. The bbox's color is gray and style is Qt.DashDotLine.
  2. Rm mouse and keyboard events, it affects the range of interaction (Can't click inside the rectangle) and del polygons.

Hi, here is my suggestion:

  1. As part of annotation, bbox is typically used with same color with other annotation info and painted in real line.
  2. sure no problem

Thank you:blush:, We think too many solid lines of multiple colors may cause interference, the gray dotted line looks like an auxiliary display function

geoyee pushed a commit that referenced this pull request Sep 15, 2021
fix(add bbox) : the output does not give correct bbox info
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

Successfully merging this pull request may close these issues.

None yet

3 participants