Skip to content

correction in facial areas and expanding logic#87

Merged
serengil merged 1 commit intomasterfrom
feat-task-0203-facial-area-correction
Mar 2, 2024
Merged

correction in facial areas and expanding logic#87
serengil merged 1 commit intomasterfrom
feat-task-0203-facial-area-correction

Conversation

@serengil
Copy link
Owner

@serengil serengil commented Mar 2, 2024

Tickets

#86

What has been done

With this PR, facial area variables x, y, w, h are set correctly.

How to test

make lint && make test

@serengil serengil merged commit e02bc41 into master Mar 2, 2024
@serengil serengil deleted the feat-task-0203-facial-area-correction branch March 2, 2024 21:07
@serengil serengil mentioned this pull request Mar 2, 2024
Comment on lines +248 to +249
w = facial_area[2] - x
h = facial_area[3] - y
Copy link

@LilianBoulard LilianBoulard Mar 22, 2024

Choose a reason for hiding this comment

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

Is this a typo? I don't understand why that would be minus, and re-implementing with + instead of - works on my environment (with retina-face 0.0.15).

Copy link
Owner Author

Choose a reason for hiding this comment

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

Because 3rd and 4th points are x and y coordinates of right down area. We can find width and height by subtracting them to left up area coordinates.

Copy link
Owner Author

Choose a reason for hiding this comment

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

If you use plus, then you will have greater facial area. But it may exceeds the image coordinates and throw an exception. Correct calculation should be minus.

Choose a reason for hiding this comment

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

Okay, thanks for the clarification!

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.

2 participants

Comments