GSoC patch ideas

[Joe Templeman]

I've been a fan and a user of Hugin for a few years, making my own
panoramas, tutorials, even going as far as using it for professional
purposes but I've never got into the coding side of it. I'm really
enthusiastic about getting into the community and seeing if I can go
for the google summer of code, but I'm having trouble getting ideas
for useful patches.

I'm a second year computer science student and very competent in Java
and C, and have ventured into cpp for a few small projects so I feel I
could make a decent contribution but Im not sure what I can do for a
patch. How big/complex does the patch have to be to be considered
seriously?

Thanks for anything you have to contribute,

Joe Templeman

[Bruno Postle]

On Tue 24-Mar-2009 at 12:55 -0700, Joe Templeman wrote:
>
>I'm a second year computer science student and very competent in Java
>and C, and have ventured into cpp for a few small projects so I feel I
>could make a decent contribution but Im not sure what I can do for a
>patch. How big/complex does the patch have to be to be considered
>seriously?

The purpose of the patch is just to demonstrate that you can set up
a build environment and use the necessary tools such as svn, diff
etc.. The patch doesn't have to be elaborate, anything that shows
an ability to understand the relevant part of the code will do.

You could fix a compiler warning, or tackle one of the bugs in the
tracker e.g:

A simple division by zero error in libpano13:

http://sourceforge.net/tracker/?func=detail&aid=2002617&group_id=96188&atid=613954

A problem with lots of images in PTtiff2psd

http://sourceforge.net/tracker/?func=detail&aid=1923451&group_id=96188&atid=613954

PTmender has very bad handling of temporary files

http://sourceforge.net/tracker/?func=detail&aid=1897838&group_id=96188&atid=613954

A hugin gui layout bug:

http://sourceforge.net/tracker/?func=detail&aid=2686486&group_id=77506&atid=550441

nona opens unused files for no apparent reason:

http://sourceforge.net/tracker/?func=detail&aid=2166837&group_id=77506&atid=550441

Keyboard shortcuts don't work anymore in the Control Points tab:

http://sourceforge.net/tracker/?func=detail&aid=2125481&group_id=77506&atid=550441

--
Bruno

[Yuval Levy]

Hello Joe, welcome to the community.

Joe Templeman wrote:
> How big/complex does the patch have to be to be considered
> seriously?

Bruno has given you some suggestion. What we will consider seriously,
and why:
<http://panospace.wordpress.com/2009/03/25/a-patch-the-first-milestone/>

good luck, and watch the clock.

Yuv

[Guido Kohlmeyer]

> A simple division by zero error in libpano13:
>
> http://sourceforge.net/tracker/?func=detail&aid=2002617&group_id=96188&atid=613954
>

I'm currently working on a fix. Maybe it's not wise to do the same.

Guido

[Yuval Levy]

do you mean it is too difficult for a student, or are you worried about
duplicate effort?

no worries about duplicate efforts.

for you it is a bug hunt, for the students it is about showing us that
they can hack the code base. There is no requirement for the student
patch to be useful or to be applied to the code base when considering
them for GSoC.

Yuv

[dmg]

By the way, if anybody has any questions about libpano, please add me
explicitly to a message to the
mailing list. That will call my attention and I'll reply more promptly.

I also agree with Yuv, since we don't know who will solve it until it
is solved, it is not a bad idea to
have more than one person looking after it.

-dmg

On Wed, Mar 25, 2009 at 9:43 AM, Yuval Levy <goo...@levy.ch> wrote:
>
> for you it is a bug hunt, for the students it is about showing us that
> they can hack the code base. There is no requirement for the student
> patch to be useful or to be applied to the code base when considering
> them for GSoC.

--
--dmg

---
Daniel M. German
http://turingmachine.org

[Guido Kohlmeyer]

Well I cannot rate whether it is too difficult to fix this. I'm too old
:-) and have know overview knowledge about lib's or hugin's sources. I
simply follow the approach to narrow a crash down and I wanted to avoid
double effort.

I digged some hours in the code and hopefully found the reason for the
crash. For Albers Equal Area projection there are some parameter
combinations that are prohibited, phi1 = phi1 = 0 and phi1 = -phi2. The
later one is not handled correctly.

First of all, the calculation of precomputed values does not handle
these combinations. I implemented checks and simplyfied the calculation.
Additionally the distance calculation does not catch the second
combination. I removed the error message because many many dialogs
appear. It is better to catch the prohibited combinations directly in
Hugin with only one dialog rather than in the lib.

Please find attached a patch for review. I tested it on Win32 (MSVC) and
no crash occured any more.

Comments are highly appreciated,
Guido


Yuval Levy schrieb:

[D M German]

I am sorry Guido I dropped the ball from this.

(This discussion should go into libpano, but there is also an issue
with hugin).

Hi Guido,

I am looking at your patch right now, and I see you changed some of
the computations. The biggest one I don't quite follow is:

replacing:

- C = cos(phi1) * cos(phi1) + 2.0 * n * sin(phi1);

with

+ C = 1.0 + Aux_sin_phi1 * Aux_sin_phi2;

I can try to follow the math but it is easier to ask you ;)

Also, with respect to making it rho0 a huge number instead of letting
the computation proceed with an INF value:

rho0 = ( (n != 0) ? (Aux_1 / n) : (1.7E+308) );

I would prefer we use INF and deal with that where appropriate, which
is the result of the calculation of the projection. Is hugin handling
INF properly? It might be a problem in hugin, not in libpano.

- twiceN = sin(phi1) + sin(phi2);
- n = twiceN /2.0;

- rho0 = sqrt(C - 2.0 * n * sin(phi0)) / n;
+ // precompute sinus functions
+ Aux_sin_phi0 = sin(phi0);
+ Aux_sin_phi1 = sin(phi1);
+ Aux_sin_phi2 = sin(phi2);

+ Aux_2N = Aux_sin_phi1 + Aux_sin_phi2;
+ n = Aux_2N / 2.0;
+
+ // C = cos(phi1) * cos(phi1) + 2.0 * n * sin(phi1);
+ // rho0 = sqrt(C - 2.0 * n * sin(phi0)) / n;
+ Aux_1 = (C - (Aux_2N * Aux_sin_phi0));
+ Aux_1 = ( (Aux_1 > 0) ? (sqrt(Aux_1)) : (0.0) );
+ rho0 = ( (n != 0) ? (Aux_1 / n) : (1.7E+308) );
+

--
--
Daniel M. German
http://turingmachine.org/
http://silvernegative.com/
dmg (at) uvic (dot) ca
replace (at) with @ and (dot) with .

--
Daniel M. German "Sooner or later all the peoples of the world,
without regard to the political system
under which they live,
will have to discover a way
Martin Luther King, jr. -> to live together in peace."
http://turingmachine.org/
http://silvernegative.com/
dmg (at) uvic (dot) ca
replace (at) with @ and (dot) with .

[Bart.van.Andel]

Although I'm not really into this code, shouldn't a divide by zero
result in NAN instead of INF? Of course the limit of 1/n with n->0 is
INF, but only for n>0. For n<0 (a very small negative value), 1/n
actually goes to -INF. At exactly n=0, 1/n is not defined. It's a
small detail, but NAN is more accurate here than INF, if we're going
to use that.

Best,
Bart

[rew]

On Apr 21, 1:31 pm, "Bart.van.Andel" <bavanan...@gmail.com> wrote:
> Although I'm not really into this code, shouldn't a divide by zero
> result in NAN instead of INF? Of course the limit of 1/n with n->0 is
> INF, but only for n>0.

Possibly, both numbers being divided are always positive, so that
"+INF" is more accurate. But I'm not familiar enough with the code
to know that either.

[paul womack]

Bart.van.Andel wrote:
> Although I'm not really into this code, shouldn't a divide by zero
> result in NAN instead of INF?

According to this:

http://en.wikipedia.org/wiki/Division_by_zero

It's well defined, and works as you would hope:

IEEE floating-point standard, supported by almost all modern processors, specifies that every floating point arithmetic operation, including division by zero, has a well-defined result. In IEEE 754 arithmetic, a ÷ 0 is positive infinity when a is positive, negative infinity when a is negative, and NaN (not a number) when a = 0. The infinity signs change when dividing by -0 instead. This is possible because in IEEE 754 there are two zero values, plus zero and minus zero, and thus no ambiguity.

BugBear

[Bart.van.Andel]

On 21 apr, 15:27, paul womack <pwom...@papermule.co.uk> wrote:
> IEEE floating-point standard, supported by almost all modern processors, specifies that every floating point arithmetic operation, including division by zero, has a well-defined result. In IEEE 754 arithmetic, a ÷ 0 is positive infinity when a is positive, negative infinity when a is negative, and NaN (not a number) when a = 0. The infinity signs change when dividing by -0 instead. This is possible because in IEEE 754 there are two zero values, plus zero and minus zero, and thus no ambiguity.

Ah yes, I see, I hadn't considered it that way. Then the question is,
is a check for a=0 necessary? Doesn't the arithmetic take this into
account automatically (as in, it computes the result as +INF or -INF,
depending on the value of Aux_1)?

Best,
Bart

[D M German]

paul womack twisted the bytes to say:

paul> Bart.van.Andel wrote:
>> Although I'm not really into this code, shouldn't a divide by zero
>> result in NAN instead of INF?

paul> According to this:

paul> http://en.wikipedia.org/wiki/Division_by_zero

paul> It's well defined, and works as you would hope:

Yes, try this:

----------------------------------------------------------------------
#include <stdio.h>
#include <math.h>
#include <assert.h>

int main(void)
{

double x = 0;

double y;

y = 1/x;
printf("hello world [%f]\n", y);
assert(isinf(y));
assert(isnan(y));
}
----------------------------------------------------------------------
paul> IEEE floating-point standard, supported by almost all modern
paul> processors, specifies that every floating point arithmetic
paul> operation, including division by zero, has a well-defined
paul> result. In IEEE 754 arithmetic, a ÷ 0 is positive infinity when
paul> a is positive, negative infinity when a is negative, and NaN
paul> (not a number) when a = 0. The infinity signs change when
paul> dividing by -0 instead. This is possible because in IEEE 754
paul> there are two zero values, plus zero and minus zero, and thus
paul> no ambiguity.

BugBear

paul>

--
--
Daniel M. German
http://turingmachine.org/
http://silvernegative.com/
dmg (at) uvic (dot) ca
replace (at) with @ and (dot) with .

--

Daniel M. German "I'm crazy, but I'm not stupid"
Jackie Chan

[dmg]

Actually, the solution to avoid the -INF, +INF is to check for "is
finite?" using the
isfinite macro. From its man page:

The isfinite() macro shall determine whether its argument has a finite
value (zero, subnormal, or normal, and not infinite or NaN). First, an
argument represented in a format wider than its semantic type is con‐
verted to its semantic type. Then determination is based on the type of
the argument.

-dmg