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

Help patch cleanups and harmonization #50

Merged
merged 42 commits into from
Mar 31, 2021

Conversation

sebescudie
Copy link
Collaborator

Opening this PR so we can discuss things about help patches cleaning before merging.

In 340ff31, i reworked patches from the General category. I'd like to show some information on why HoldLatestCopy is used in HowTo Apply operations asynchronously instead of a plain HoldLatest. Guess it has to do with the fact that we're dealing with images here, but could you shed some light on this @ravazquez ?

Thanks!

@sebescudie sebescudie marked this pull request as draft January 20, 2021 12:27
@sebescudie
Copy link
Collaborator Author

sebescudie commented Jan 20, 2021

In 4f4844a, I drastically simplified the HowTo Count pixel that are not black. It was bloated with too many nodes, which made the very goal of the patch unclear.
Took the AND (Filter) node out and brought it to a new patch with OR (Filter) and XOR (Filter) : HowTo Apply boolean operations on images, which basically shows what HowTo Count pixel [...] did, with OR and XOR added.

Happy to discuss!

@sebescudie sebescudie linked an issue Jan 20, 2021 that may be closed by this pull request
@sebescudie sebescudie added this to In progress in documentation Jan 20, 2021
@ravazquez
Copy link
Collaborator

Opening this PR so we can discuss things about help patches cleaning before merging.

In 340ff31, i reworked patches from the General category. I'd like to show some information on why HoldLatestCopy is used in HowTo Apply operations asynchronously instead of a plain HoldLatest. Guess it has to do with the fact that we're dealing with images here, but could you shed some light on this @ravazquez ?

Thanks!

In the case of OpenCV we try to copy the image data or allocate new memory as little as possible for efficiency, this means that we reuse the same image reference (or the same memory) over and over which is fine for most cases. However there are cases in which you need to further do stuff with the image (modify it), or cases in which the image is being written into from another thread while you are reading it, both of which would give you memaccess and corrupted data problems either upstream or downstream.

In general, to avoid reading into memory which may be being written into or which may not be ready, you can use HoldLatestCopy which will just copy the data over and hold the copy, allowing any further processing on the original image to be of no concern to you, this of course comes with a cost.

As a rule of thumb always try using HoldLatest unless it gives you issues, in which case you should use HoldLatestCopy.

@azeno can probably explain this in much more detail.

@sebescudie
Copy link
Collaborator Author

@ravazquez Thanks, that's clear to me and surely enough for some in-patch explanations.

@ravazquez ravazquez merged commit 0f84f52 into develop Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
documentation
In progress
Development

Successfully merging this pull request may close these issues.

Harmonize/clean existing help patches
2 participants