Page MenuHomeFreeBSD

graphics/darktable: Update to 1.6.9

Authored by dumbbell on Oct 16 2015, 5:50 PM.



Add a new option, OPENMP, to build darktable with OpenMP. This requires LLVM 3.7 as a runtime dependency when GCC is not the default compiler. The option is enabled by default because it brings an important performance improvement.

Diff Detail

rP FreeBSD ports repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dumbbell updated this revision to Diff 9454.Oct 16 2015, 5:50 PM
dumbbell retitled this revision from to graphics/darktable: Update to 1.6.9.
dumbbell updated this object.
dumbbell edited the test plan for this revision. (Show Details)
dumbbell added a reviewer: kwm.
kwm added inline comments.Oct 17 2015, 10:27 AM
99 ↗(On Diff #9454)

Not sure this is needed.

104 ↗(On Diff #9454)

This does also need a BUILD_DEPEND

109 ↗(On Diff #9454)

Can darktable find the omp library after linking? Also please check if openMP is actually enabled, ImageMagick and inkscape don't by default when using clang 3.7.

dumbbell added inline comments.Oct 17 2015, 11:02 AM
99 ↗(On Diff #9454)

It didn't work with make(1) complained with a syntax error in the .if below, usually meaning a variable is undefined.

104 ↗(On Diff #9454)

Ok, I will add it.

About RUN_DEPENDS, the correct dependency is ${LOCALBASE}/llvm37/lib/, not clang37. Is there a way to specify that instead?

109 ↗(On Diff #9454)

Darktable works fine on my laptop and lld(1) finds

Darktable cmake conf properly detects -fopenmp with Clang 3.7 and enables OpenMP support.

dumbbell updated this revision to Diff 9464.Oct 17 2015, 11:04 AM

Add devel/llvm37 as a BUILD_DEPENDS

dumbbell updated this revision to Diff 9527.Oct 20 2015, 8:21 AM

Depend on Lua 5.2 explicitly (not any version)

darktable 1.6.9 was officially released, so the archive can be downloaded now with make fetch.

There is no need to update distinfo as it is the exact same archive provided to package managers.

danfe added a subscriber: danfe.Nov 5 2015, 8:18 AM
danfe added inline comments.
104 ↗(On Diff #9527)

I'm not sure if it would correctly define all required dependencies (both build- and run-time), but you might consider using USES=compiler:openmp.

118 ↗(On Diff #9527)

I think this target is not needed (when building docless package, DOCSDIR is ignored).

dumbbell updated this revision to Diff 10029.Nov 8 2015, 1:03 PM
dumbbell marked 4 inline comments as done.

Address @danfe's comment + patch to src/CMakeLists.txt

The patch to src/CMakeLists.txt was prepared for the upcoming darktable 2.0 and backported to darktable 1.6. When darktable 2.0 is compiled with Clang and -ffast-math (which is enabled by default), isnan() always returns false, even if -fno-finite-math-only is specified. darktable 2.0 crashes on startup without this patch. darktable 1.6 still starts but the patch appears to improve stability in lighttable (I get more freezes/lockups without it). So let's apply it now to darktable 1.6.

dumbbell added inline comments.Nov 8 2015, 1:06 PM
104 ↗(On Diff #10029)

USES=compiler:openmp always pulls in GCC. This can't work with darktable because g++ and libstdc++ are used to build the C++ code and it's not compatible with libc++.

The trick used by graphics/rawtherapee is at least as ugly and darktable crashes with it anyway.

dumbbell updated this revision to Diff 10113.Nov 11 2015, 12:15 PM


kwm accepted this revision.Nov 11 2015, 12:18 PM
kwm edited edge metadata.

I have seen the patch-src_CMakeLists.txt and it correct, even if phabricator doesn't show it...

This revision is now accepted and ready to land.Nov 11 2015, 12:18 PM
danfe added inline comments.Nov 11 2015, 12:29 PM
104 ↗(On Diff #10113)

USES=compiler:openmp always pulls in GCC.

Oh, I see. Reading the above comment (Enable OpenMP support with Clang 3.7) it looks like that we'd probably want to relax compiler:openmp dependency on GCC, but that's out of scope of this patch today.

This revision was automatically updated to reflect the committed changes.