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

Draw line fix so that color actually works, plus extra functions for … #2195

Merged
merged 2 commits into from Dec 1, 2017

Conversation

guidoAI
Copy link
Contributor

@guidoAI guidoAI commented Nov 28, 2017

…drawing crosshairs and rectangles.

// Go through all the points
for (uint16_t i = 0; i < points_cnt; i++) {
// Draw a line from the original position with the flow vector
struct point_t from = {
vectors[i].pos.x / subpixel_factor,
vectors[i].pos.y / subpixel_factor,
0,

Choose a reason for hiding this comment

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

what about the other parameters of the point_t structure ? shouldn't they be initialized properly ?
if only x,y are needed in most case, maybe an init macro can be added to images.h to hide the other parameters initialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good remark! point_t got additional properties in a previous pull request - I believe by Manos. I wonder if this is actually the best way to go, as a point as a mathematical concept would not require properties such as the number of times observed. Perhaps a solution would be an additional struct with a point_t as one of its attributes. However, I feel this is beyond the scope of the current pull request.

Copy link
Member

Choose a reason for hiding this comment

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

Can you make an issue about this so that it is addressed in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's worth an issue

Copy link
Member

@kirkscheper kirkscheper left a comment

Choose a reason for hiding this comment

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

If you run code style I think its ok, see my other small comments.

@@ -578,27 +608,24 @@ void image_show_points(struct image_t *img, struct point_t *points, uint16_t poi
*/
void image_show_flow(struct image_t *img, struct flow_t *vectors, uint16_t points_cnt, uint8_t subpixel_factor)
{
uint8_t color[4] = {255,255,255,255};
int size_crosshair = 5;
Copy link
Member

Choose a reason for hiding this comment

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

In the future, things like this can be declared static so that they are not constantly redeclared. alternatively you can consider using the const descriptor, helps with code clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made it static now

Copy link
Member

Choose a reason for hiding this comment

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

This was not specific to this line but in general, often you define a variable with a constant value, in that case a static or const (or both) descriptor is interesting. Consider it in the future.

// Go through all the points
for (uint16_t i = 0; i < points_cnt; i++) {
// Draw a line from the original position with the flow vector
struct point_t from = {
vectors[i].pos.x / subpixel_factor,
vectors[i].pos.y / subpixel_factor,
0,
Copy link
Member

Choose a reason for hiding this comment

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

Can you make an issue about this so that it is addressed in the future?

else {
temp_color[0] = color[0];
temp_color[2] = color[2];
}
Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to run fix code style. You can also change the settings of your editor so you don't have this problem in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ran fix code style

@kirkscheper kirkscheper merged commit 0adf61c into paparazzi:master Dec 1, 2017
biancabndris pushed a commit to biancabndris/paparazzi that referenced this pull request Aug 29, 2018
… rectangles (paparazzi#2195)

* Draw line fix so that color actually works, plus extra functions for drawing crosshairs and rectangles.

* Added static and formatted code style
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