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

adding some markers inside OpenCV #5386

Merged
merged 1 commit into from
Nov 2, 2015
Merged

adding some markers inside OpenCV #5386

merged 1 commit into from
Nov 2, 2015

Conversation

StevenPuttemans
Copy link

Adding Matlab like markers in OpenCV since I needed them for an application I made.

@LorenaGdL
Copy link
Contributor

Could you provide this for 2.4 branch? It would be awesome :)

@StevenPuttemans
Copy link
Author

Yeah I will backport it to 2.4 also once this one works fine.

@mshabunin
Copy link
Contributor

Thank you! I have two questions:
Why didn't you make just one function and an enumeration with available markers?
Parameter shift should be added, shouldn't it?

@StevenPuttemans
Copy link
Author

@mshabunin so you are suggesting 1 function and then a marker_type enumerator? That could work also, changes will not be that difficult. Secondly, I will look into the shift thing, not sure what it does right now and not sure if it is necessary for markers yet :D

@StevenPuttemans
Copy link
Author

Hmm screwed it up 👯 lets try to fix this commit history

@StevenPuttemans
Copy link
Author

@mshabunin is this what you had in mind?

@mshabunin
Copy link
Contributor

@StevenPuttemans, yes, thank you.

@StevenPuttemans
Copy link
Author

👍

//! Possible set of marker types used for the drawMarker function
enum MarkerTypes
{
MARKER_CROSS = 0, // A crosshair marker shape
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Will do so after the weekend, no time anymore before my trip!

@StevenPuttemans
Copy link
Author

Completely forgot this :D will have a look at it today!

@StevenPuttemans
Copy link
Author

Still some misalignment in the enum documentation. Seems like it needs to go on top of each line. Will fix this first!

@StevenPuttemans
Copy link
Author

/* ADDING A SET OF PREDEFINED MARKERS WHICH COULD BE USED TO HIGHLIGHT POSITIONS IN AN IMAGE */
/* ----------------------------------------------------------------------------------------- */

//! Possible set of marker types used for the drawMarker function
Copy link
Contributor

Choose a reason for hiding this comment

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

hi @StevenPuttemans , it will be nicer if you use cv::drawMarker like
//! Possible set of marker types used for the cv::drawMarker function

@StevenPuttemans
Copy link
Author

@sturkmen72 applied changes suggested!

@vpisarev vpisarev self-assigned this Oct 1, 2015
@vpisarev
Copy link
Contributor

vpisarev commented Oct 1, 2015

@StevenPuttemans, thanks! I will replicate my comment in #5390:

this is useful function! May I suggest to change the order of parameters? i.e. move markerSize before the thickness, since the latter two parameters (thickness and especially lineType) are less important, in my opinion

@StevenPuttemans
Copy link
Author

@vpisarev done in both PRs!

@alalek
Copy link
Member

alalek commented Oct 1, 2015

Still some misalignment in the enum documentation. Seems like it needs to go on top of each line.

It is not necessary, just use //!< prefix

@StevenPuttemans
Copy link
Author

Actually now documentation is aligned in the table. If i put it after the expression then the explanations all sink down one leven

@StevenPuttemans
Copy link
Author

Level*

@alalek
Copy link
Member

alalek commented Oct 1, 2015

I mean to comment object on the same line you should use //!< instead //! (that is used to put comment before object):
https://www.stack.nl/~dimitri/doxygen/manual/docblocks.html#memberdoc

@StevenPuttemans
Copy link
Author

Okay thank you for the info!

@StevenPuttemans
Copy link
Author

Adapted as you suggested, but it seems the HAL test is failing, which has nothing to do with my input :)

@StevenPuttemans
Copy link
Author

Can this be merged? Cleaning up repos and would be usefull for me if this is in OpenCV :)

@vpisarev
Copy link
Contributor

vpisarev commented Nov 2, 2015

sorry for delay. 👍

@StevenPuttemans
Copy link
Author

Thank you very much!

@opencv-pushbot opencv-pushbot merged commit 587dca9 into opencv:master Nov 2, 2015
@StevenPuttemans StevenPuttemans deleted the add_markers branch November 4, 2015 07:51
@amroamroamro
Copy link
Contributor

@StevenPuttemans ping #5905

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.

8 participants