Skip to content

pollKey implementation for w32 backend of highgui#19411

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
crackwitz:highgui-pollkey
Feb 5, 2021
Merged

pollKey implementation for w32 backend of highgui#19411
opencv-pushbot merged 1 commit intoopencv:masterfrom
crackwitz:highgui-pollkey

Conversation

@crackwitz
Copy link
Copy Markdown
Contributor

@crackwitz crackwitz commented Jan 27, 2021 •
edited by alalek
Loading

first code proposal for #19410

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake
force_builders=Win32,Custom
buildworker:Custom=linux-1,linux-4,linux-6
build_image:Custom=qt:16.04

asmorkalov
asmorkalov requested changes Jan 28, 2021
View reviewed changes
@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Jan 28, 2021

@crackwitz Please pay attention on CI. The PR breaks build.

@crackwitz
Copy link
Copy Markdown
Contributor Author

crackwitz commented Jan 28, 2021 •
edited
Loading

how should I handle this? all the existing code for every function is structured such that there is an extern C implementation of the respective function per toolkit, and then one namespaced wrapper function calling that.

I could remove the CV_IMPL (extern C) from all implementations of cvPollKey so it iself is not exposed. I think that will then require a forward declaration in window.cpp before/inside of cv::pollKey since the C function isn't declared in a common header anymore.

edit: that is... if CVAPI(int) cvPollKey(void); in highgui_c.h needs to go, I'll need to provide that declaration in some other way. maybe I misunderstand.

@crackwitz crackwitz force-pushed the highgui-pollkey branch 2 times, most recently from 870bbfa to 6bf1925 Compare January 29, 2021 21:06
@crackwitz
Copy link
Copy Markdown
Contributor Author

crackwitz commented Jan 29, 2021

ok, looks like I did it right this time. I hope the structure is as desired.

@crackwitz crackwitz requested a review from asmorkalov January 29, 2021 22:17
asmorkalov
asmorkalov approved these changes Feb 1, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@asmorkalov asmorkalov left a comment

Choose a reason for hiding this comment

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

Well done! 👍

@asmorkalov
Copy link
Copy Markdown
Contributor

asmorkalov commented Feb 1, 2021

@crackwitz Please squash commits to have clean changes history in OpenCV git.

alalek
alalek reviewed Feb 1, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Please add/update documentation's cross-links between waitKey(), imshow() calls. So Users can find this functionality.

BTW, This function's purpose is updating UI too (like waitKey())

(function's name is still confusing to me, perhaps @vpisarev can suggest something or approve this)

@crackwitz
Copy link
Copy Markdown
Contributor Author

crackwitz commented Feb 1, 2021 •
edited
Loading

here are the requested changes. I hope I understood the intention.

I'll squash when you're satisfied with the state of the pull request and any discussion has resolved. squashing implies force-push; I hope that's correct.

This function's purpose is updating UI too (like waitKey())

I understand that. I'm not sure if waitKey should be given a more suitable name as well.

@crackwitz
Copy link
Copy Markdown
Contributor Author

crackwitz commented Feb 2, 2021

I've squashed the commits so far and rebased to current master HEAD.

@crackwitz
implementation of pollKey, companion to waitKey
26da491 
w32 backend: implemented
other backends: stubbed or fallback to waitKey
documentation: cross-linked and more precise in some places
alalek
alalek approved these changes Feb 5, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@alalek alalek left a comment

Choose a reason for hiding this comment

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

Thank you!

@opencv-pushbot opencv-pushbot merged commit 9d22764 into opencv:master Feb 5, 2021
@crackwitz crackwitz deleted the highgui-pollkey branch February 5, 2021 20:40
@alalek alalek mentioned this pull request Feb 5, 2021
Merged
@alalek alalek mentioned this pull request Apr 9, 2021
Merged
@alalek alalek mentioned this pull request May 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asmorkalov asmorkalov asmorkalov approved these changes

@alalek alalek alalek approved these changes

Assignees

@asmorkalov asmorkalov

Labels

category: highgui-gui feature platform: win32

Projects

None yet

Milestone

4.5.2

Development

Successfully merging this pull request may close these issues.

4 participants

@crackwitz @asmorkalov @alalek @opencv-pushbot