Drawing refactor #209
Drawing refactor #209
Conversation
javiber
commented
Oct 26, 2022
•
javiber
commented
- Separated drawing.py into multiple files into a new module.
- Merged drawing functions for TrackedObjects and Detections using the new Drawable interface
- Encapsulated cv drawing into new Drawer class
- Added Color palette concepts and changed how we define colors
- Changed some default values to make the drawings look nicer
javiber
force-pushed
the
drawing-refactor
branch
5 times, most recently
from
6326f1d
to
bd8262c
Compare
There was a problem hiding this comment.
Great work @javiber! I left a bunch of comments, but it seems almost ready to merge.
norfair/drawing/draw_boxes.py
Outdated
| text_size : Optional[float], optional | ||
| Size of the title, the value is used as a multiplier of the base size of the font. | ||
| By default the size is scaled automatically based on the frame size. | ||
| text_color : Optional[int], optional |
There was a problem hiding this comment.
It isn't consistent with the type hint.
| text_size = label_size | ||
| # end | ||
|
|
||
| if color is None: |
There was a problem hiding this comment.
color shouldn't be None right? At least the type hint doesn't mention it.
There was a problem hiding this comment.
It shouldn't but it can happen when this function is called in draw_tracked_boxes so I added this here to make the function a bit more robust
|
|
||
| if drawables is None: | ||
| return | ||
| if color is None: |
There was a problem hiding this comment.
color shouldn't be None right? At least the type hint doesn't mention it.
norfair/drawing/draw_boxes.py
Outdated
| else: | ||
| obj_text_color = color |
There was a problem hiding this comment.
Shouldn't we assign text_color or its parsed color to obj_text_color instead of color?
There was a problem hiding this comment.
Good catch! I need to parse text_color and use that value here
| # the anchor will become the bottom-left of the text, | ||
| # we select-top left of the bbox compensating for the thickness of the box | ||
| text_anchor = ( | ||
| points[0, 0] - thickness // 2, | ||
| points[0, 1] - thickness // 2 - 1, | ||
| ) |
There was a problem hiding this comment.
Isn't this already done inside Drawer.text?
There was a problem hiding this comment.
you mean the compensation for the thickness? that needs to be done twice, one considering the thickness of the box and the the other with the thickness of the text
javiber
force-pushed
the
drawing-refactor
branch
from
8f21139
to
df9d897
Compare
norfair/drawing/color.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| string : str |
There was a problem hiding this comment.
string -> hex_value in docstring
- fixed a bug with the text color on draw_boxes.py and draw_points.py - renamed params on color.py - added mising typehints in color.py - typos in color.py - removed unused imports in absolute_grid.y
javiber
force-pushed
the
drawing-refactor
branch
from
d567ec6
to
f99b534
Compare
