Repository navigation
feat(hooks): let projects choose which OpenCV to build against - #452
Conversation
dartcv builds the OpenCV version this package pins. That is the right
default, but it leaves no way to build against a specific one, and some
projects need that: a Flutter client whose image processing has to agree
with a backend service pinned to a given OpenCV cannot accept whichever
version the package happens to track, because results move between
releases.
The CMake side already supports all of it — DARTCV_OPENCV_VERSION,
DARTCV_BUILD_OPENCV_FROM_SOURCE with OPENCV_VERSION, and
DARTCV_DISABLE_DOWNLOAD_OPENCV with OpenCV_DIR — but the hook never
passes any of them, so none is reachable from a pubspec.yaml. Setting
them by hand means environment variables threaded through Gradle,
CocoaPods and the hook, which is not something a project can rely on.
This exposes two user defines:
opencv_version: build that upstream tag from source
opencv_dir: use an OpenCV that is already built, optionally
per platform, since a cross-compiled one lives in a
different place for each target
Projects that set neither build exactly as before.
_find_version sets OPENCV_VERSION unconditionally, so a value passed in with -DOPENCV_VERSION was replaced by the pubspec's two lines later. The build then used the pinned version while reporting the requested one, which is the worst of both. The pubspec is now read only when nothing has asked for a version, which is what makes the opencv_version hook option in the previous commit take effect.
|
Pushed a third commit, and the reason is worth writing down: the first version of this PR did not actually work on the
Both paths are now verified against a Dart project depending on this branch:
In both cases One thing that came up on the way, unrelated to this PR but worth knowing if you try it: asking for |
|
@diegolopezrm Hi. I understand the reason why you need this feature. looks okay, but we should also highlight that potential API imcompatible issues if users choose non-default and non-compatible OpenCV version, these issues will not get any support by opencv_dart, would you mind add this in README too? |
Requested in review. Choosing a different OpenCV moves the risk onto the project: OpenCV changes its API between releases, so a non-default version may fail to compile, fail to link, or build cleanly and behave differently at runtime, and none of that is opencv_dart's to carry. Says so where someone is deciding whether to set the option, and asks that a bug be reproduced against the pinned version before it is reported.
|
Thanks for the review. Added in
I added the second paragraph beyond what you asked for, because "unsupported" is easier to accept when the reader can tell whether they are the intended audience. Happy to cut it, or to change the wording of either — you know better than I do how you want to phrase what you will and will not carry. |
|
@diegolopezrm Thanks, that's ok LGTM now, will merge. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #452 +/- ##
=======================================
Coverage 92.24% 92.24%
=======================================
Files 60 60
Lines 11869 11869
=======================================
Hits 10948 10948
Misses 921 921
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
dartcv builds the OpenCV version this package pins, which is the right default. But there is no way for a project to ask for a different one, and some need to: a Flutter client whose image processing has to agree with a backend service pinned to a given OpenCV cannot accept whichever version the package tracks, because results move between releases. Binarization is a good example — a threshold computed slightly differently flips pixels, not fractions of pixels.
The CMake side already supports every variant of this:
DARTCV_BUILD_OPENCV_FROM_SOURCE+OPENCV_VERSIONDARTCV_DISABLE_DOWNLOAD_OPENCV+OpenCV_DIRDARTCV_OPENCV_VERSIONNone of them is reachable from a
pubspec.yaml, because the hook does not pass them. Setting them by hand means threading environment variables through Gradle, CocoaPods and the hook runner, which is not something a project can depend on.This adds two user defines that map onto the machinery already there:
opencv_diris also accepted at the top level, and takes precedence overopencv_version. Projects that set neither build exactly as they do today — the defines are only emitted when the keys are present.Verified end to end
A Dart project depending on this branch, with
opencv_dirpointing at an OpenCV 4.12.0 I built separately:openCvVersion()reports 4.12.0, while this package pins 4.13.0 — so the OpenCV named in the pubspec is the one that ended up linked, andximgproc.niBlackThresholdruns against it.Why
opencv_diris per platformAn OpenCV cross-compiled for Android and one for iOS are separate trees, and on iOS device and simulator are separate again. A single path cannot serve a multi-platform project, so the key is read from the platform map first and falls back to the top level.
Note on the minimum version
opencv_versionaccepts any upstream tag, but not every tag works: dartcv calls APIs added in 4.12, so older ones fail to compile. #451 turns that into a clear message at configure time instead of a wall of C++ errors. This PR does not depend on it, but the two are better together — without it, a project that asks for 4.10 here gets 11 errors that never mention the version.I am happy to rename the keys, nest them under an
opencv:map, or narrow this toopencv_dironly if you would rather not encourage building arbitrary versions from source.