added optional mask to cv::threshold by chacha21 · Pull Request #26842 · opencv/opencv

chacha21 · 2025-01-24T23:12:34Z

Proposal for #26777

To avoid code duplication, and keep performance when no mask is used, inner implementation always propagate the const cv::Mat& mask, but they use a template parameter that let the compiler optimize out unecessary tests when the mask is not to be used.

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 another license that is incompatible with OpenCV
The PR is proposed to the proper branch
There is a reference to the 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

To avoid code duplication, and keep performance when no mask is used, inner implementation always propagate the const cv::Mat& mask, but they use a template<bool useMask> parameter that let the compiler optimize out unecessary tests when the mask not to be used.

if constexpr(...) requires C++17

the "readParity" is designed to handle the fact that vector operations always read 16 mask values, while data type can represent less pixels

This reverts commit a68db5a.

First fix, but the tests sometimes fail, still investigating...

…e, disabling it for now Still don't understand why unit test randomly fails

…e, disabling it for now Still don't understand why unit test randomly fails prepare mutli-channel handling

When therere were non-vectorized pixels remaining, the mask pointer was not properly reset that was a memory fault (not always triggered), and the test case only hit the case when that last pixel was greater than the threshold

not yet ready for cn>1

new strategy to load masks and duplicate bytes to match src.elemSize() instead of complex byte duplication in loaded mask vector elements, use vx_lut() instead with sliding indices

now that cn>1 works, the simplest case src.type() == CV_8UC1 can be a fast path

modules/imgproc/test/test_thresh.cpp

ocl should have failed before for the case cn>1 somehow, the ocl currently does not fail as expected even when trying to corrupt the output by setting stupid values to dst in the kernel The test is not reliable, and I don't know why

there was an error in ocl kernel args, it wasn't even run and relied on CPU implementation

opencv-alalek · 2025-01-31T04:13:21Z

We need "clear" definition what values should be used as result outside of the mask.

Al least, this is why cv::compare() doesn't have mask support.

P.S. If your mask is a rectangle - then you should use ROI approach instead.

chacha21 · 2025-01-31T06:23:35Z

We need "clear" definition what values should be used as result outside of the mask.

Al least, this is why cv::compare() doesn't have mask support.

Yes, that's what I thought when I started working on the feature.
My most useful usecase is to leave outliers untouched. This assumes that the dst image was pre-existing, otherwise they will be undefined pixel values after being allocated by cv::threshold()
I use such a feature to apply different binarizations to different (non rectangular) parts of an image according to some criteria.

asmorkalov · 2025-01-31T09:30:15Z

@chacha21 Could you list use-cases for the PR?
The proposal was discussed on weekly core team meeting and the team has doubt if we need the change. Alexander's concern is on of reasons.

chacha21 · 2025-01-31T10:12:26Z

@chacha21 Could you list use-cases for the PR? The proposal was discussed on weekly core team meeting and the team has doubt if we need the change. Alexander's concern is on of reasons.

Ok. First of all, no problem to decline the PR, I won't try to force push it.

Here are my two use cases :

case 1

A gray-level image with different areas (not rectangular) having different lighting conditions
not a single threshold will binarize the whole image, there must be one threshold (from OTSU) per area
currently, I have to use multiple masks and combinations to build to final image, and computing the OTSU threshold for a non rectangular area is a problem because of outliers pixels

case 2

a gray-level image is displayed to the user
the user can draw non-rectangular ROIs (e.g. RotatedRect areas) and want to binarize inside those areas in order to preview how binarization will highlight information of interest

asmorkalov · 2025-03-03T06:14:47Z

Hello @chacha21 Sorry for delay. We discussed to the PR on the Core team meeting and we are ready to merge the PR with several fixes:

The new function dst should be defined as InputoutputArray and the function should check that the output buffer is pre-allocated and has enough size. Document the behaviour.
The dst value with mask==0 should not be modified as you in your proposal. The behaviour is documented.
HAL is not needed here. The case is too exotic.
We recommend to use another name like thresholdWithMask to reduce chance of API collision in Python and Java.
Add python test for the new function to ensure it's wrapped correctly.

The fixes are proposed to get rid of undefined behaviour for the case where mask==0 and wrap things to Python, Java, etc correctly.

asmorkalov · 2025-03-03T06:15:19Z

Please also rebase and fix merge conflicts.

- do not overload threshold, use thresholdWithMask instead - document required dst if mask is used - do not use HAL implementation for the masked case

chacha21 · 2025-03-07T15:11:32Z

Add python test for the new function to ensure it's wrapped correctly.

I am sorry I have 0 skills in Python

added accuracy test using a rectangular ROI

modules/imgproc/test/test_thresh.cpp

modules/imgproc/src/opencl/threshold_mask.cl

modules/imgproc/src/hal_replacement.hpp

- split accuracy tests for different cases (otsu/triangle/fixed) - clean up non existent HAL versions - update reference to author

chacha21 added 4 commits January 25, 2025 00:09

replace if constexpr(...), by if()

c253d82

if constexpr(...) requires C++17

fixed trailing whitespace

0581a0d

clarify code

bd2d5b4

the "readParity" is designed to handle the fact that vector operations always read 16 mask values, while data type can represent less pixels

asmorkalov added feature category: core labels Jan 25, 2025

chacha21 added 15 commits January 25, 2025 12:01

fixed unused variable for some compilation configurations

a68db5a

Revert "fixed unused variable for some compilation configurations"

f3acc5e

This reverts commit a68db5a.

the previous code onl worked with continuous data.

c704588

First fix, but the tests sometimes fail, still investigating...

currently, the mask SMID logic does not support the multi-channel cas…

55176f5

…e, disabling it for now Still don't understand why unit test randomly fails

currently, the mask SMID logic does not support the multi-channel cas…

53b6f48

…e, disabling it for now Still don't understand why unit test randomly fails prepare mutli-channel handling

fixed previous bug

5fdfb0f

When therere were non-vectorized pixels remaining, the mask pointer was not properly reset that was a memory fault (not always triggered), and the test case only hit the case when that last pixel was greater than the threshold

restored code that will (perhaps) handle cn>1 case properly

f42acf8

not yet ready for cn>1

restored code that will (perhaps) handle cn>1 case properly

e2399c2

not yet ready for cn>1

support cn>1

57496a8

new strategy to load masks and duplicate bytes to match src.elemSize() instead of complex byte duplication in loaded mask vector elements, use vx_lut() instead with sliding indices

little optimization in test so that it runs faster

2923424

fixed test

3dfd161

renaming some variables for clarity

71816a4

fix minor warnings

373d5a8

fixed cn>1 case

407c8ce

added fastpath

bc9909f

now that cn>1 works, the simplest case src.type() == CV_8UC1 can be a fast path

asmorkalov reviewed Jan 27, 2025

View reviewed changes

modules/imgproc/test/test_thresh.cpp Outdated Show resolved Hide resolved

chacha21 added 4 commits January 27, 2025 15:43

improved tests

a7c7a3f

fixed ocl

d878d36

ocl should have failed before for the case cn>1 somehow, the ocl currently does not fail as expected even when trying to corrupt the output by setting stupid values to dst in the kernel The test is not reliable, and I don't know why

get rid of vectorized limitation prior to vx_lut usage

8c22303

fixed ocl and ocl_test

2e1cb3f

there was an error in ocl kernel args, it wasn't even run and relied on CPU implementation

asmorkalov self-assigned this Mar 3, 2025

asmorkalov added this to the 4.12.0 milestone Mar 3, 2025

chacha21 added 2 commits March 7, 2025 14:44

Merge remote-tracking branch 'upstream/master' into threshold_with_mask

028bc76

changes according to reviewers

c799822

- do not overload threshold, use thresholdWithMask instead - document required dst if mask is used - do not use HAL implementation for the masked case

fixed OTSU and TRIANGLE masked versions

d30fc8c

added accuracy test using a rectangular ROI

asmorkalov reviewed Mar 11, 2025

View reviewed changes

chacha21 and others added 2 commits March 11, 2025 15:51

modifications as suggested by reviewer

71923b1

- split accuracy tests for different cases (otsu/triangle/fixed) - clean up non existent HAL versions - update reference to author

Code review fixes.

cc18a6a

asmorkalov approved these changes Mar 12, 2025

View reviewed changes

asmorkalov merged commit 0db6a49 into opencv:4.x Mar 12, 2025
49 of 55 checks passed

asmorkalov mentioned this pull request Apr 29, 2025

5.x merge 4.x #27265

Merged

sfe-SparkFro mentioned this pull request Jul 22, 2025

Update to OpenCV 4.12.0 sparkfun/micropython-opencv#39

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

added optional mask to cv::threshold#26842

added optional mask to cv::threshold#26842
asmorkalov merged 28 commits intoopencv:4.xfrom
chacha21:threshold_with_mask

chacha21 commented Jan 24, 2025

Uh oh!

Uh oh!

opencv-alalek commented Jan 31, 2025

Uh oh!

chacha21 commented Jan 31, 2025

Uh oh!

asmorkalov commented Jan 31, 2025

Uh oh!

chacha21 commented Jan 31, 2025 •

edited

Loading

Uh oh!

asmorkalov commented Mar 3, 2025

Uh oh!

asmorkalov commented Mar 3, 2025

Uh oh!

chacha21 commented Mar 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

chacha21 commented Jan 24, 2025

Uh oh!

Uh oh!

opencv-alalek commented Jan 31, 2025

Uh oh!

chacha21 commented Jan 31, 2025

Uh oh!

asmorkalov commented Jan 31, 2025

Uh oh!

chacha21 commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asmorkalov commented Mar 3, 2025

Uh oh!

asmorkalov commented Mar 3, 2025

Uh oh!

chacha21 commented Mar 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chacha21 commented Jan 31, 2025 •

edited

Loading