Build GraphicsMagick with quantum-depth 16 by default

[carandraug]

The default configure option to build GraphicsMagick is quantum depth 8. This limits GraphicsMagick to always read images with that bitdepth. This can be quite limiting since GraphicsMagick is then used by scientific applications where 16bit images are very common.

There is a very long discussion on a debian bug report #557879 about the subject. I will only quote one the comments by Bob Friesenhahn, the maintainer of GraphicsMagick:

Bob Friesenhahn, the upstream GraphicsMagick maintainer, also recommends
building GraphicsMagick with a 16-bit quantum depth [1]:

It has been a while since 2004, but today that is still my
recommendation if (almost) all the capabilities are needed. Of course
there is a memory penalty, which becomes more severe for people
dealing with high-resolution monochrome files like faxes.

Today's computers come with quite a lot more RAM and people have
become used to dealing with raw digital camera files and 16-bit/sample
TIFF files.

If you were to offer just one set of binaries,
the 16bit version would be a better offering
since it is clearly superior for some tasks
and there is more assurance that quality won't
be lost.

Bob also mentioned the 16-bit build is the one he spends most time optimizing
[2].

That is true and is a reason that the performance of the 16-bit build
does not lag as much behind the 8-bit build as it used to do. In the
GraphicsMagick 1.2 timeframe, considerable effort was put into being
able to import and export from the 16-bit samples with the best
performance.

Can the default in homebrew be changed? I can submit a patch but need to know first if that's not a problem.

[DomT4]

Don't have a problem with the default being changed personally.

[carandraug]

Does b78da5c looks acceptable then?

[DomT4]

Oh, actually, looking at the formula it looks like there is no default currently?

    if build.with? "quantum-depth-32"
      quantum_depth = 32
    elsif build.with? "quantum-depth-16"
      quantum_depth = 16
    elsif build.with? "quantum-depth-8"
      quantum_depth = 8
    end

    args << "--with-quantum-depth=#{quantum_depth}" if quantum_depth

From that, unless people are passing options already, there shouldn't be a arg about quantum depth being passed. Are you seeing something different locally?

[DomT4]

Unless configure is going ahead and assuming a default without instruction (?)

[carandraug]

Yes, configure assumes quantum depth 8 by default which is why changing it downstream can be questionable. But I believe that there's good reasons to do so, upstream maintainer agrees that it's a better default, and at least Fedora and Debian have also changed it.

[DomT4]

Huh. The formula wording implies there's no default there, but the old wording does acknowledge the default of 8. Perhaps something like this then:

diff --git a/Library/Formula/graphicsmagick.rb b/Library/Formula/graphicsmagick.rb
index b3bf7c7..03193e8 100644
--- a/Library/Formula/graphicsmagick.rb
+++ b/Library/Formula/graphicsmagick.rb
@@ -12,7 +12,7 @@ class Graphicsmagick < Formula
   end

   option "with-quantum-depth-8", "Compile with a quantum depth of 8 bit"
-  option "with-quantum-depth-16", "Compile with a quantum depth of 16 bit"
+  option "with-quantum-depth-16", "Compile with a quantum depth of 16 bit (default)"
   option "with-quantum-depth-32", "Compile with a quantum depth of 32 bit"
   option "without-magick-plus-plus", "disable build/install of Magick++"
   option "without-svg", "Compile without svg support"
@@ -60,13 +60,13 @@ class Graphicsmagick < Formula

     if build.with? "quantum-depth-32"
       quantum_depth = 32
-    elsif build.with? "quantum-depth-16"
-      quantum_depth = 16
     elsif build.with? "quantum-depth-8"
       quantum_depth = 8
+    else
+      quantum_depth = 16
     end

-    args << "--with-quantum-depth=#{quantum_depth}" if quantum_depth
+    args << "--with-quantum-depth=#{quantum_depth}"
     args << "--without-x" if build.without? "x11"
     args << "--without-ttf" if build.without? "freetype"
     args << "--without-xml" if build.without? "svg"

@MikeMcQuaid wrote the current wording on quantum depth though, so would be cool if he'd sign off on changing the language.

[carandraug]

Perhaps something like this then:

I'm confused. That's the same I wrote above (see b78da5c). Did you paste the wrong diff?

[DomT4]

They are slightly different. You left in the elsif build.with? "quantum-depth-16" lines, whereas I removed them in favour of the else. Could have been easier for me to just explain that rather than a fresh diff, perhaps, apologies.

[carandraug]

Because it was placed in the middle of comments on the wording about a default option, I thought the issue was the change of the option help text..

To explain the reasoning behind leaving the elsif build.with? "quantum-depth-16", I just found it matches the thought better and leaves a single place to change the default. But anyway, that doesn't really matter.

[DomT4]

Feel free to file a PR to make the change. I'll ping Mike on it for review if he doesn't see it himself. Thanks!