Skip to content

[opencv4] turn on 'intrinsics' feature for CI builds#48466

Closed
tophyr wants to merge 2 commits into
microsoft:masterfrom
tophyr:opencv4-intrinsics
Closed

[opencv4] turn on 'intrinsics' feature for CI builds#48466
tophyr wants to merge 2 commits into
microsoft:masterfrom
tophyr:opencv4-intrinsics

Conversation

@tophyr
Copy link
Copy Markdown
Contributor

@tophyr tophyr commented Nov 25, 2025

  • Changes comply with the maintainer guide.
  • SHA512s are updated for each updated download.
  • The "supports" clause reflects platforms that may be fixed by this new version.
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@tophyr
Copy link
Copy Markdown
Contributor Author

tophyr commented Nov 25, 2025

@dg0yt @vicroms i think this is the last thing in the way of #48451, #48424 and eventually #48304: opencv4's port exposes its CV_ENABLE_INTRINSICS variable (which defaults to ON in upstream) via a feature, and if that feature isn't specified then the variable gets set to OFF. It sounds like OpenCV4 is poorly-tested on OSX and Windows, and so has some bugs when intrinsics are not enabled (again, counter to the default behavior).

Here I'm attempting to tell the CI system that opencv4[vulkan,intrinsics] is supposed to pass (whereas just opencv4[vulkan] expects failure), but it doesn't look like the ci.feature.baseline.txt file likes the =pass syntax like ci.baseline.txt does. Unfortunately, ci.baseline.txt doesn't have the ability to specify features.

I'm actually kind of inclined to simply make the intrinsics feature default to ON in the port, and then maybe add a no-intrinsics feature that turns it off. ARM64 Windows and OS X consumers would only benefit from that since they're broken in that state currently, but x64 builds and Linux builds might be surprised.

Thoughts?

Comment thread scripts/ci.baseline.txt
Comment on lines -1022 to -1028
vcpkg-ci-opencv:arm64-osx=pass
# OpenCV appears to be trying to use __fp16 on a platform that does not support that
# buildtrees\opencv4\src\4.11.0-46ecfbc8ae.clean\modules\dnn\src\layers\cpu_kernels\conv_winograd_f63.simd.hpp(1371): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
#vcpkg-ci-opencv:arm64-windows-static-md=pass
opencv4:arm64-windows-static-md=fail
#vcpkg-ci-opencv:arm64-windows=pass
opencv4:arm64-windows=fail
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do not remove expected pass from "pass" section of this file.

Comment on lines 195 to 211
opencc:x64-android=fail
opencc(uwp)=fail # opencc/deps/marisa-0.2.5/lib/marisa/grimoire/io/mapper.cc currently doesn't support UWP.
opencv4[core,cuda,world](windows)=options
opencv4[dnn,intrinsics]:arm64-osx=pass
opencv4[dnn,intrinsics](arm & windows & !uwp)=pass
opencv4[dshow](!windows | uwp)=feature-fails
opencv4[halide](!android & !(windows & staticcrt) & !uwp & !(arm & windows))=feature-fails # error C2039: 'attachHalide': is not a member of 'cv::dnn::GeluFunctor'
opencv4[opengl]:x64-linux=feature-fails
opencv4[opengl](android)=feature-fails
opencv4[opengl](windows&!uwp)=feature-fails # Option WITH_OPENGL is enabled but corresponding dependency have not been found: "HAVE_OPENGL" is FALSE
opencv4[ovis](android)=feature-fails
opencv4[vtk,intrinsics]:arm64-windows-static-md=feature-fails
opencv4[vtk,intrinsics]:arm64-windows=feature-fails
opencv4[vulkan,intrinsics]:arm64-osx=pass
opencv4[vulkan,intrinsics](arm & windows & !uwp) = pass
openmama:x64-windows-static-md=fail
openmesh:arm64-windows=fail
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is the "fail" section of the file. Do only add "fail" here.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Nov 25, 2025

  • Do not add negative features.
  • In ci.baseline.txt, required "pass" prevents unexpected cascade
  • In ci.features.baseline.txt, no entry means expected pass.

some bugs when intrinsics are not enabled

Can you add a reference for this?

@tophyr
Copy link
Copy Markdown
Contributor Author

tophyr commented Nov 25, 2025

Actually, I see that opencv4/vcpkg.json does define intrinsics as one of the default features, so probably the last paragraph I wrote above is already the case and it's simply the CI system that needs teaching.

Can you add a reference for this?

Extensive debugging with

#if defined(BLAH)
  #error here BLAH
#endif

lol 😅

Basically, when specifying the port features but not including intrinsics in the feature list, the CMake configuration var CV_ENABLE_INTRINSICS gets set to OFF. When that variable is set to OFF on ARM64 Mac/Windows, OpenCV doesn't include <arm_neon.h> but does still use FP16 datatypes defined in it (which is the bug). I haven't explored why Linux doesn't suffer from this, but I can say that using FP16 datatypes without including arm_neon.h is definitively wrong.

@tophyr
Copy link
Copy Markdown
Contributor Author

tophyr commented Nov 25, 2025

How can I specify to CI that "opencv4:arm64-osx is expected to pass, as long as the intrinsics feature is used"? The arm64-windows builds will likely pass with this feature usage as well.

@tophyr
Copy link
Copy Markdown
Contributor Author

tophyr commented Nov 25, 2025

Maybe the no-intrinsics feature is the way to go, and then I could just define opencv4[no-intrinsics](!linux & arm) = fail?

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Nov 25, 2025

Maybe the no-intrinsics feature is the way to go, and then I could just define opencv4[no-intrinsics](!linux & arm) = fail?

No negative features.

@tophyr
Copy link
Copy Markdown
Contributor Author

tophyr commented Nov 25, 2025

Gotcha. I'm not sure how to express the valid combination to CI then - basically, opencv4[vulkan,intrinsics] works, but opencv4[vulkan] does not.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Nov 25, 2025

basically, opencv4[vulkan,intrinsics] works, but opencv4[vulkan] does not.

And opencv4[core]:arm64-windows works, too.

Maybe the answer is that opencv4[vulkan](arm64) depends on opencv4[intrinsics]? What I see is that opencv4[core,openvino]:arm64-osx fails due to missing types from arm_neon.h. So it is not just an issue of vulkan+arm64-windows.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Nov 25, 2025

basically, opencv4[vulkan,intrinsics] works, but opencv4[vulkan] does not.

And opencv4[core]:arm64-windows works, too.

Maybe the answer is that opencv4[vulkan](arm64) depends on opencv4[intrinsics]? What I see is that opencv4[core,openvino]:arm64-osx fails due to missing types from arm_neon.h. So it is not just an issue of vulkan+arm64-windows.

CI logs checked for arm64-windows, arm64-osx in https://dev.azure.com/vcpkg/public/_build/results?buildId=123265&view=results: vulkan and openvino depend on dnn, errors related to types from arm_neon.h.
Now trying with that dependency added: #48467

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Nov 25, 2025

Now trying with that dependency added: #48467

For osx, my PR was successful at initial attempt.

  • opencv4[core,dnn]:arm64-osx
  • opencv4[core,vulkan]:arm64-osx
  • opencv4[core,openvino]:arm64-osx

For arm64 windows, no improvement yet.

@tophyr
Copy link
Copy Markdown
Contributor Author

tophyr commented Nov 25, 2025

Oh awesome, didn't realize marking dependencies between features would build the port with the union of them. Perfect! Closing this in favor of #48467.

@tophyr tophyr closed this Nov 25, 2025
@tophyr tophyr deleted the opencv4-intrinsics branch November 25, 2025 21:29
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.

2 participants