Skip to content

Round fix in OnMouse#20149

Merged
opencv-pushbot merged 1 commit intoopencv:3.4from
rogday:on_mouse_floor
Jun 2, 2021
Merged

Round fix in OnMouse#20149
opencv-pushbot merged 1 commit intoopencv:3.4from
rogday:on_mouse_floor

Conversation

@rogday
Copy link
Copy Markdown
Member

@rogday rogday commented May 24, 2021

Fixing #20141

pt32f.x = cvRound(event_button->x);
pt32f.y = cvRound(event_button->y);
pt32f.x = cvFloor(event_button->x);
pt32f.y = cvFloor(event_button->y);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Which values are come from GTK to trigger this problem?

Copy link
Copy Markdown
Member Author

@rogday rogday May 24, 2021

Choose a reason for hiding this comment

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

For 512 by 512 image it can be 511.89, for example. It worked fine in gtk2 because it outputs already floored values.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

Also there are cases with image size less than minimal window size (e.g 10x10) - need to check that this mode work properly too, because floor() doesn't guarantee that we don't exceed image sizes.

To make fix reliable, we should restore (uncomment) the condition below:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure window has minimal size. I just tested 1x1 image and it sort of works(I couldnt click on this pixel but I think its my fault).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@alalek, should I restore the condition just in case or is this PR okay as it is?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I prefer to restore the condition or use clamp().
cvRound => cvFloor replacement is incomplete (there are several places left) and probably we still need that condition.

BTW, There is similar problem with JavaScript (e.g, here).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I brought back bounds check and fixed rounding for motion too. I see, in JS canvas sends events about its border too.

@asmorkalov asmorkalov requested a review from alalek June 1, 2021 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pixel over access with mouse callback function

3 participants