Skip to content

Fix draw.legend legend box width and allow changing of font size and thickness#677

Merged
liyier90 merged 7 commits intoaisingapore:mainfrom
leeping-ng:fix-legend-zone-count
Jul 27, 2022
Merged

Fix draw.legend legend box width and allow changing of font size and thickness#677
liyier90 merged 7 commits intoaisingapore:mainfrom
leeping-ng:fix-legend-zone-count

Conversation

@leeping-ng
Copy link
Copy Markdown
Contributor

@leeping-ng leeping-ng commented Jul 25, 2022

closes #675 and #676.

Main changes:

  • Fixed bug where the legend box is very thin and doesn't stretch across the entire width of text for zone counting
  • Added improvement where font size of text within legend box can now be configured as follows:
font: {
  size: 0.7,
  thickness: 2
}

Other improvements include removing most hardcoded variables in utils/legend.py.

@leeping-ng leeping-ng changed the title Fix legend zone count Fix draw.legend legend box width and allow changing of font size and thickness Jul 25, 2022
@leeping-ng leeping-ng marked this pull request as ready for review July 25, 2022 10:01
@leeping-ng leeping-ng requested a review from liyier90 July 25, 2022 10:01
liyier90
liyier90 suggested changes Jul 26, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@liyier90 liyier90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In additional to the review comments, I've also noticed that when the width of the legend box content changes back and forth, it can create a distracting effect.

legend_resize
Not sure if this is within the scope of this PR, but maybe scaling the font size based on the length of the content could help alleviate this problem.

Comment thread peekingduck/pipeline/nodes/draw/legend.py Outdated
Comment thread peekingduck/pipeline/nodes/draw/utils/legend.py Outdated
Comment thread peekingduck/pipeline/nodes/draw/utils/legend.py Outdated
Comment thread peekingduck/pipeline/nodes/draw/utils/legend.py Outdated
Comment thread peekingduck/pipeline/nodes/draw/utils/legend.py Outdated
Comment thread peekingduck/pipeline/nodes/draw/utils/legend.py Outdated
Comment thread peekingduck/pipeline/nodes/draw/utils/legend.py Outdated
Comment thread peekingduck/pipeline/nodes/draw/utils/legend.py Outdated
Comment thread peekingduck/pipeline/nodes/draw/utils/legend.py Outdated
@leeping-ng
Copy link
Copy Markdown
Contributor Author

leeping-ng commented Jul 26, 2022

In additional to the review comments, I've also noticed that when the width of the legend box content changes back and forth, it can create a distracting effect.

legend_resize legend_resize Not sure if this is within the scope of this PR, but maybe scaling the font size based on the length of the content could help alleviate this problem.

One solution is max(previous_width, current_width) so that the legend box width only grows but doesn't shrink. I don't think scaling font size is as feasible as it may affect readability of the text, which is what this PR aims to address.

@leeping-ng leeping-ng requested a review from liyier90 July 27, 2022 02:40
@liyier90 liyier90 merged commit f2248d2 into aisingapore:main Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@liyier90 liyier90 liyier90 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

draw.legend background box does not span across the entire width of text for zone counting

3 participants

@leeping-ng @liyier90 @ryzalk