JakobEngel / dso

Direct Sparse Odometry
GNU General Public License v3.0
2.3k stars 910 forks source link

Undistort::readFromFile bug? #219

Open ronir121 opened 4 years ago

ronir121 commented 4 years ago

Hi

At the end of the function Undistort::readFromFile there's this code:

for (int y = 0; y < h; y++)      
    for (int x = 0; x < w; x++) {       
        // make rounding resistant.       
        float ix = remapX[x + y * w];         
        float iy = remapY[x + y * w];        
        if (ix == 0) ix = 0.001;          
        if (iy == 0) iy = 0.001;         
        if (ix == wOrg - 1) ix = wOrg - 1.001;    
        if (iy == hOrg - 1) ix = hOrg - 1.001;   
        if (ix > 0 && iy > 0 && ix < wOrg - 1 && iy < wOrg - 1) {     
            remapX[x + y * w] = ix;             
            remapY[x + y * w] = iy;           
        } else {               
            remapX[x + y * w] = -1;       
            remapY[x + y * w] = -1;         
        }         
    }

I see a couple of problems here (which in the end cause my code to crash due to segmentation fault):

  1. If ix and iy are floats, the following code doesn't run as it's supposed to: if (ix == wOrg - 1) ix = wOrg - 1.001;
    Since you can't really compare floats for identity ...

  2. The following code seems to be mixed up: if (ix == wOrg - 1) ix = wOrg - 1.001;
    if (iy == hOrg - 1) ix = hOrg - 1.001;
    Shouldn't it be: if (ix == wOrg - 1) ix = wOrg - 1.001;
    if (iy == hOrg - 1) iy= hOrg - 1.001; // <----------- iy instead of ix

  3. The line: if (ix > 0 && iy > 0 && ix < wOrg - 1 && iy < wOrg - 1) {
    Shouldn't it be: if (ix > 0 && iy > 0 && ix < wOrg - 1 && iy < hOrg - 1) { // <------ hOrg instead of wOrg

???? I would really appreciate if someone takes a look at it ...

NikolausDemmel commented 4 years ago

Without having looked at the code in more detail, looks like 2 and 3 are typos. For 1, a direct floating point equality may be valid, if the exact value was assigned before.