[REVIEW] Pushing Hugin changes to VIGRA upstream

51 views
Skip to first unread message

Lukáš Jirkovský

unread,
Mar 25, 2011, 4:46:48 PM3/25/11
to hugi...@googlegroups.com
Hello,
the issue of pushing the Hugin specific changes to the libraries and
especially the VIGRA has been discussed several times before. The
consensus was that it's a good thing to do, but nobody did it.

But today I had a free day and I wanted to learn the Mercurial MQ
Extension so I decided to give the $SUBJ a try.

Attached is a series of patches which apply to VIGRA from mercurial
[1]. In the next mail I'll send a patch to make Hugin work with
patched VIGRA 1.8 (it's too big to fit in this email).

Please review them and comment on them, because I'd like to send
them to the upstream soon. I'm not sure about copyrights in some cases
(eg. canvasSize patch) but I think most of it is Pablo's work. I'll look
it up in the subversion history.

Before applying patches you should read their descriptions below:

VIGRA-canvasSize.diff
----------------------------
Implements setCanvasSize() and getCanvasSize functions. These
functions are used for setting canvas size in nona. Necessary for
VIGRA-OpenEXR.diff to work.
This is most likely work of Pablo d'Angelo.

VIGRA-OpenEXR.diff
-------------------------
Implements support for OpenEXR in VIGRA. VIGRA-canvasSize.diff has to
be applied first. Work of Pablo d'Angelo (and the compression bits
have been made by me).

VIGRA-bigTIFF.diff
---------------------
Support for BigTIFF in VIGRAs TIFF decoder. Does not depend on any
other patches. Probably Pablo's work.

VIGRA-PACKBITS.diff
--------------------------
Support for PACKBITS compression in TIFF decoder.  Does not depend on
any other patches. Probably Pablo's work.

VIGRA-FixCrashLogLuv.diff
---------------------------------
Fixes the crash when trying to load LogLuv TIFF. Doesn't add support
for LogLuv though. This has to be applied before
VIGRA-supportLogLuv.diff. This time credit goes to me.

VIGRA-supportLogLuv.diff
--------------------------------
Add support for loading LogLuv TIFF (eg. from Luminance HDR). Depends
on VIGRA-FixCrashLogLuv.diff. Credit goes to me.

Regards,
Lukas


[1] http://www.informatik.uni-hamburg.de/~meine/hg/vigra

VIGRA-canvasSize.diff
VIGRA-OpenEXR.diff
VIGRA-bigTIFF.diff
VIGRA-PACKBITS.diff
VIGRA-FixCrashLogLuv.diff
VIGRA-supportLogLuv.diff

Pablo d'Angelo

unread,
Mar 26, 2011, 3:03:25 AM3/26/11
to hugi...@googlegroups.com
Hi Lukas,

> Please review them and comment on them, because I'd like to send
> them to the upstream soon. I'm not sure about copyrights in some cases
> (eg. canvasSize patch) but I think most of it is Pablo's work. I'll look
> it up in the subversion history.
>

great, looks good, feel free to submit those to vigra!

ciao
Pablo

Lukáš Jirkovský

unread,
Mar 26, 2011, 6:04:10 AM3/26/11
to hugi...@googlegroups.com

Hello Pablo,
thank you for giving it the thumbs up, especially given the fact it's
mostly your work.

I'll wait some time if there was some response from others, meanwhile
I'd like to update the copyright headers of the affected files.

Have a nice day,
Lukas

Lukáš Jirkovský

unread,
Mar 31, 2011, 6:44:38 AM3/31/11
to hugi...@googlegroups.com

Lukáš Jirkovský

unread,
Mar 31, 2011, 9:10:39 AM3/31/11
to hugi...@googlegroups.com
And all patches have been accepted! I'm still working on some tests,
but we are getting closer to a time when Hugin can work with system
wide installation of VIGRA.

Lukas

Bart van Andel

unread,
Mar 31, 2011, 12:06:46 PM3/31/11
to hugi...@googlegroups.com
Nice work! 

I've been working on getting Hugin to cross-compile from Linux to Windows in my precious spare time, and the stock vigra lib already compiles, so this will definitely help! There are still a few packages missing in the cross-compiling system I'm using (mingw-cross-env [0]), I'd have to check which ones, but I hope that in the end I'll be able to do a full cross-compile. This will make the process of releasing Windows binaries even easier (at least for me). Note that I *really don't* have that much spare time, so it may take a while. Of course anyone interested may take a look as well, we're talking open source here :)


--
Bart

Lukáš Jirkovský

unread,
Mar 31, 2011, 2:48:21 PM3/31/11
to hugi...@googlegroups.com

Thank you. I'm glad it could make your work on getting
cross-compilation easier. Your work seems very interesting. I wish you
luck, this could be very useful for Hugin.

Lukas

Stefan Peter

unread,
Mar 31, 2011, 4:15:31 PM3/31/11
to hugi...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Bart

Am 31.03.2011 18:06, schrieb Bart van Andel:
> Nice work!
>
> I've been working on getting Hugin to cross-compile from Linux to
> Windows in my precious spare time, and the stock vigra lib already
> compiles, so this will definitely help!

I had a look at his, too. However, without being able to actually use
the resulting binaries due to my lack of a windows license and my lack
of interest to get one, I figured that I won't be able to support this
kind of port. On the other side, if I had a windows license and would be
willing to create panos on this platform, wouldn't it be more natural to
use the (available) free MS development tools for porting?

I think that hugin needs to attract some more windows users being able
to fend for their OS themselves. Providing binaries that are not tested
by the producer and that can not be properly supported, therefore, does
not help.

Sorry, just my 2 cents.

Regards

Stefan Peter

- --
In theory there is no difference between theory and practice. In
practice there is.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNlODcAAoJEBgqi52L7+L/13kH/ja8c8DNhfUTlPEwXiIdtZVB
Pcf0n1rcEqdBTT3GmNks9Hfr9St/m9ct70EPIu5pIPyDWv+d/2M08KaowQphuJ1D
1BxuLod3DlEfNbdTCpJd0JXoG+/pOxAwslPDfMgKX9BB7LKMgx5OLDRtyHcT5T/O
NbMqFWmkUVXK7+/sWsjtrPNcT8Xf7kRmcqD0CU+19qX9i5VjXxBS58/t7QSUhnyg
q1MJSo2E/5KYU172dfXCEaPCS1zZiok0kvqpdIAdFcU9S3DmJVr/X42/RT5ykS8D
oq509Ml8Hv6m4lMCLglNkRWZKzzcFTxE4UUda8TQzzC3czxCknDpOLLt3wviqO8=
=WQR/
-----END PGP SIGNATURE-----

Bart van Andel

unread,
Mar 31, 2011, 4:39:03 PM3/31/11
to hugi...@googlegroups.com
Hi Stefan,


On Thursday, March 31, 2011 10:15:31 PM UTC+2, Stefan wrote:

Am 31.03.2011 18:06, schrieb Bart van Andel:
> I've been working on getting Hugin to cross-compile from Linux to
> Windows in my precious spare time, and the stock vigra lib already
> compiles, so this will definitely help!

I had a look at his, too. However, without being able to actually use
the resulting binaries due to my lack of a windows license and my lack
of interest to get one, I figured that I won't be able to support this
kind of port. On the other side, if I had a windows license and would be
willing to create panos on this platform, wouldn't it be more natural to
use the (available) free MS development tools for porting?

I understand your situation, but from my point of view, compiling under Linux is usually a bliss. I like the way things can be automated using the shell in Linux. And this mingw-cross-env thing really takes away a lot of pain. For instance, I can build the required libs (minus some that I still need to add) using a single make command. The system will automatically download the sources for me, including dependencies, and build everything into static libs. The only thing left to do then is to compile Hugin (and Enblend, APSCpp, ...) itself, using cmake and some cross-compiling settings. Pretty easy. I'm using a virtual machine to do this.

On the other hand, using the MS path requires many more steps. A whole lot of packages need to be compiled separately. Just take a look at [0] to see what I mean. Ok, if your initial setup is done (building dependencies), building and rebuilding Hugin is not a problem, except when some dependency needs a rebuild...

To me, the Linux approach is much simpler. And if the workflow has been setup correctly, Hugin can be built on a (native) Linux machine, QA tested by a few Windows pre-testers, and then distributed.

I think that hugin needs to attract some more windows users being able
to fend for their OS themselves. Providing binaries that are not tested
by the producer and that can not be properly supported, therefore, does
not help.

I agree about quality assurance. Distributing non-tested binaries is not a good thing. But this kind of testing has to be done for native Windows builds (built on a Windows machine I mean) as well, and depending on the builder, this will happen or not.
 

Sorry, just my 2 cents.

No apologies required, your points are valid.


--
Bart 

Bruno Postle

unread,
Mar 31, 2011, 7:08:29 PM3/31/11
to Hugin ptx

Lukáš, thanks for doing this, it has been needed for a long time.

Regarding fixing the Hugin sources, there is no need to remove the
Hugin copy of the library from the sources if it is possible to
build with the system library as an option. vigra now has a
vigra-config tool which gives output similar to pkg-config, so it
should be possible for cmake to detect if the system vigra is >
1.7.1

--
Bruno

Bart van Andel

unread,
Mar 31, 2011, 7:18:54 PM3/31/11
to hugi...@googlegroups.com
On Friday, April 1, 2011 1:08:29 AM UTC+2, Bruno Postle wrote:

Regarding fixing the Hugin sources, there is no need to remove the
Hugin copy of the library from the sources if it is possible to
build with the system library as an option.

Is there a valid point in keeping a local copy of a library if the official one works? If you mean keeping the local copy until the changes are in a release version of Vigra, I can understand, but otherwise?

--
Bart

Bruno Postle

unread,
Mar 31, 2011, 7:48:01 PM3/31/11
to Hugin ptx

Yes and no, it can't be removed until there is a vigra release, but
we don't know when this will be. Also the pressure from Linux
distributions is for binary packages to use system libraries, they
don't care what unused cruft is in the Hugin tarball.

If the latest vigra happens to fix any of the longstanding bugs that
keep getting reported in the hugin/enblend trackers then it could
even be worth updating the vigra code in hugin/enblend.

--
Bruno

Reply all
Reply to author
Forward
0 new messages