-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
Replace vendored SIMD Everywhere by prefix/system install #10961
base: master
Are you sure you want to change the base?
Replace vendored SIMD Everywhere by prefix/system install #10961
Conversation
efd9e74
to
3985104
Compare
3985104
to
2ac6089
Compare
@@ -426,6 +426,14 @@ function(target_export target) | |||
DESTINATION "${package_destination}" | |||
COMPONENT Development | |||
${exclude_variant}) | |||
|
|||
if(target STREQUAL libobs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we also need to export SIMDE here? Are SIMDE types used in function prototypes or inline functions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The headers include a SIMDE header which make it a dependency of libobs header and so the CMake package needs the finder to be installed alongside the package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah the question was more about which ones? Because SIMDE is an implementation detail of libobs
internals and should not be exposed publicly. It doesn't make sense to expose it from an API point of view and if we do that seems like a mistake to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
util/sse-intrin.h
(which hard depends on SIMDE) used in some installed headers:
grep -r "sse-intrin.h" /usr/include/obs/.
/usr/include/obs/./graphics/quat.h:#include "../util/sse-intrin.h"
/usr/include/obs/./graphics/vec3.h:#include "../util/sse-intrin.h"
/usr/include/obs/./graphics/vec4.h:#include "../util/sse-intrin.h"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right and of course we inlined all those functions so we created a tight coupling between API users and the internal data types..
That should have never happened, but I guess that's water under the bridge and can be cleaned up once the current API is not used externally anymore in the future.
56c0582
to
a86a165
Compare
a86a165
to
2736151
Compare
SIMD Everywhere finder needs to be installed alongside libobs CMake package since its headers depends on it. macOS AVX intrinsics is included to make sure that AVX defines are present.
2736151
to
ebbb3a1
Compare
Description
Important
Depends on:
Drop of the CMake legacy (I can implement legacy path if really needed)Close #5067
Replace vendored SIMD Everywhere by prefix/system install.
SIMD Everywhere finder needs to be installed alongside libobs CMake
package since its headers depends on it.
macOS AVX intrinsics is included to make sure that AVX defines are
present.
Motivation and Context
Reduce in-tree vendored deps in OBS Studio repo.
How Has This Been Tested?
Build in CI on my fork.
Types of changes
Checklist: