Source code review

103 views
Skip to first unread message

Bernd D

unread,
Sep 14, 2016, 5:53:15 AM9/14/16
to hugin and other free panoramic software
Is here someone, who can check the following inconsistencies in the source code?

1.) pano_modify.exe calculate a wrong (vertical?) FoV.

 %HUGINSDK%/hugin-hg/src/hugin_base/algorithms/nona/CalculateFOV.cpp
function hugin_utils::FDiff2D CalculateFOV::calcFOV(const PanoramaData& panorama):

    if(fov.x<40)
   
{
        fov
.x+=1;
        fov
.y+=1; // [BD] I add this line, so that y is equal to x treated
   
};
   
return fov;

fov.x and fov.y must be calculated equivalent. What reason is there for the if statement? The gap at 40.0 make no sense. It looks like a dirty trick to bend the correct result to a desired result. I use always fov < 4, then it seems to work better with +1. But this is only a subjective impression.
 
2.)
 %HUGINSDK%\libpano-hg\math.c
function tiltForward:

 
       s = z1 / z0;
       s
= z0 /(x_dest * (-sin(phi2)/cos(phi2))  + y_dest * sin(phi)/cos(phi) + z0);


The first calculation of s is unused. Perhaps an unnoticed error.

Kind regards,
  Bernd

T. Modes

unread,
Sep 17, 2016, 10:22:04 AM9/17/16
to hugin and other free panoramic software


Am Mittwoch, 14. September 2016 11:53:15 UTC+2 schrieb Bernd D:

1.) pano_modify.exe calculate a wrong (vertical?) FoV.

fov.x and fov.y must be calculated equivalent.

No. The horizontal and vertical fov are independent of each other.
So I added the special treatment for smaller fov also for the vertical case.

What reason is there for the if statement? The gap at 40.0 make no sense.
I added a comment to the code, why it is used.

If pano_modify calculate a wrong fov, so plese provide the pto file, so we can have a look on it.
 
It looks like a dirty trick to bend the correct result to a desired result. I use always fov < 4, then it seems to work better with +1. But this is only a subjective impression.
 
2.)
 %HUGINSDK%\libpano-hg\math.c
function tiltForward:

The first calculation of s is unused. Perhaps an unnoticed error.

Thanks, fixed in libpano.

PS: Please state also the line numbers in the files. This makes it more easy to find the place.

Bernd D

unread,
Sep 20, 2016, 8:47:50 AM9/20/16
to hugin and other free panoramic software
I have attached a test image in original 1000x1000 pixels and a 4 tiles cut with 800x800 pixels:
T00.tif: TrX 0 TrY 0 [pix].
T01.tif: TrX 200 TrY 0 [pix].
T02.tif: TrX 0 TrY 200 [pix].
T03.tif: TrX 200 TrY 200 [pix].
I want to show 4 issues:
  1. On site Optimizer in table Image Orientation, column Trx, TrY, TrZ show only rounded to integer values.
  2. On site Stitcher, edit fields Horizontal and Vertical show only rounded to integer values.
  3. On site Stitcher, button Calculate field of view: Calculate for Vertical a value that is Horizontal-1 (this is reported on my first post).
  4. For the correct values, you have to reduce the images FoV. Smaller values result in more accuracy. But values <1.0 does not support by all panotools. This is perhaps not a bug, but I cannot find any better settings, which operate independently of FoV.
Step by step instructions to reproduce the issues
 
Per default the -–fullscale option for cpfind.exe is not set. Without this option cpfind work not correct (find lesser CPs and misplace this). In the following, I assume that this has been set. Look at Files | Preferences | site Control Point Detectors | Hugin’s CPFind (Default).
 
I have saved the resulting state in the file v10.pto. You can leave out the following steps by simply loading this file.  
  • Open hugin, switch to Expert Interface
  • With Add images.. load the 4 images T00.tif..T03.tif. Enter in HFOV (v): 10 
  • Right Click in image table, for rows/images 1..3, Popup menu: Lens |New lens
  • Select OptimiseGeometric: Custom parameter
  • Select OptimisePhotometric: Custom parameter
  • On Optimiser tab: Select TrX and TrY and unselect all other parameters
  • On Exposure tab: Unselect all parameters
  • On Stitcher tab: Select Projection: Rectilinear
  • On Photos tab: Click Create control points button (this creates 6x25=150 CPs)
  • On Optimiser tab, click the Optimise now! button. The result is not show in the table, because Trx and TrY is rounded to an integer! (Issue 1).
 
Now to the error, reported on my first post: 
  • On Stitcher tab, click the Calculate field of view button: Hugin calculated: Horizontal: 15 Vertical: 14 (Issue 3)
  • Click Calculate optimal size, Hugin calculate Width: 1204 Height 1123 (Issue 3 & 4)
  • Click Fit crop to images, Hugin calculate Left: 201 Top: 161 Right: 1202 Bottom 1123 (Issue 3 & 4)
  • Save as v10err1.pto
Click Stitch! and look at the generated image: It size is 1001x962 pixels, has left and right a one pixel margin and the red crosses have shadows. The vertical error is significant greater as in X direction.
 
Now we correct issue 3: 
  • Enter Vertical 15
  • Click Calculate optimal size, Hugin calculate Width: 1204 Height 1204 (Issue 4)
  • Click Fit crop to images, Hugin calculate Left: 201 Top: 201 (Issue 4) Right: 1203 Bottom: 1203 (Issue 4)
  • Save as v10err2.pto
Click Stitch! and look at the generated image: It size is 1001x1001 pixels. It has on all sites a one pixel margin. The red crosses are OK.
 
Now let us reduce the Issue 5: 
  • Load v1.pto this is the same as v10.pto but the images has HFOV (v)=1
  • On Stitcher tab, click the Calculate field of view button: Hugin calculated: Horizontal: 1.5 Vertical: 0.5 but show 2 and 1 (Issue 2)
  • Enter Vertical: 1.5, Horizontal: 1.5
  • Click Calculate optimal size, Hugin calculate Width: 1200 Height 1200
  • Click Fit crop to images, Hugin calculate Left: 200 Top: 200 Right: 1200 Bottom: 1200
  • Save as v1ok.pto  
Click Stitch! and look at the generated image: Now we have the expected result!

The current pano_modify.exe has the same issues. I think hugin using this program or the same source code. If is fixed then

pano_modify.exe --projection=0 --fov=AUTO --canvas=AUTO --crop=AUTO -o out.pto in.pto
provides the correct values to the p line in out.pto.

Bernd D

unread,
Sep 20, 2016, 9:02:05 AM9/20/16
to hugin and other free panoramic software

T. Modes

unread,
Sep 24, 2016, 9:19:00 AM9/24/16
to hugin and other free panoramic software


Am Dienstag, 20. September 2016 14:47:50 UTC+2 schrieb Bernd D:
I want to show 4 issues:
  1. On site Optimizer in table Image Orientation, column Trx, TrY, TrZ show only rounded to integer values.
No. The values are shown with 1 digit.
  1. On site Stitcher, edit fields Horizontal and Vertical show only rounded to integer values.
Yes. The optimal fov calculates with a resolution of 1 deg. And the most use case is a panorama with a high fov. So for this use case you don't need a higher resolution.
  1. On site Stitcher, button Calculate field of view: Calculate for Vertical a value that is Horizontal-1 (this is reported on my first post).
 The is a special case and happens only in this particular case. This is already fixed in the repository, as written in my answer already.

Per default the -–fullscale option for cpfind.exe is not set. Without this option cpfind work not correct (find lesser CPs and misplace this).
Please don't state such general statements. In most case cpfind works fine without fullscale. There may be case where full scale is better suited, e.g. your images with a very low resolution and a repeating pattern. But this is specific to this use case and does not apply in general.

Bernd D

unread,
Sep 26, 2016, 5:17:22 AM9/26/16
to hugin and other free panoramic software


Am Samstag, 24. September 2016 15:19:00 UTC+2 schrieb T. Modes:


Am Dienstag, 20. September 2016 14:47:50 UTC+2 schrieb Bernd D:
On site Optimizer in table Image Orientation, column Trx, TrY, TrZ show only rounded to integer values.
No. The values are shown with 1 digit.
  Ok. And “0.04999999999999999” is show as “0” That's what I mean.
  1. On site Stitcher, edit fields Horizontal and Vertical show only rounded to integer values.
Yes. The optimal fov calculates with a resolution of 1 deg. And the most use case is a panorama with a high fov. So for this use case you don't need a higher resolution.

No, the calculation is with double precision. Only the GUI Value is rounded. If you let the edit field untouched then hugin use the unrounded value.

For a 360° panorama with 10000pix is 1° error = 10000pix/1° * 360° = 28pix



Per default the -–fullscale option for cpfind.exe is not set. Without this option cpfind work not correct (find lesser CPs and misplace this).
Please don't state such general statements. In most case cpfind works fine without fullscale. There may be case where full scale is better suited, e.g. your images with a very low resolution and a repeating pattern. But this is specific to this use case and does not apply in general.
 

Sorry, I do not agree. It would have saved me several days troubleshooting if someone had warned me against this pitfall.


Of course, this is not an error in cpfind. cpfind results can only be as accurate as the input data. Without --fullscale, the accuracy is at least halved. That may be acceptable. But I have total failures seen in the absence of this option, which resulted that hugin run more than 3 hours on a 25x25 mosaic of images with 1000x1000pix (Overlap approx 150pix). The pictures were taken on a scanning electron microscope. After this time I have give up and kill the process. With --fullscale it needs the expected approximately 30 minutes.

Bruno Postle

unread,
Sep 26, 2016, 5:47:24 AM9/26/16
to hugi...@googlegroups.com


On 26 September 2016 10:17:22 BST, Bernd D wrote:
>
>Of course, this is not an error in cpfind. cpfind results can only be as
>accurate as the input data. Without --fullscale, the accuracy is at least
>halved. That may be acceptable. But I have total failures seen in the
>absence of this option, which resulted that hugin run more than 3 hours on
>a 25x25 mosaic of images with 1000x1000pix (Overlap approx 150pix). The
>pictures were taken on a scanning electron microscope.

This is a special case, Hugin has to assume that most input images are photos taken with Bayer sensors that have higher pixel resolution than necessary.

I haven't tested this, but it is also possible that downsizing the pictures removes high frequency artefacts that would confuse the feature detector (leading to better results for digital photos).

--
Bruno
Reply all
Reply to author
Forward
0 new messages