From 9ee55fbe682e24b0ef7e30768b3f6f84497d21d0 Mon Sep 17 00:00:00 2001 From: Oleg Alexandrov Date: Tue, 26 Mar 2024 14:09:42 -0700 Subject: [PATCH] Minor hardening --- src/asp/GUI/MainWidget.cc | 24 ++++++++------- src/asp/Sessions/StereoSession.cc | 21 ++++++------- src/asp/Tools/bundle_adjust.cc | 2 +- src/asp/Tools/parallel_stereo | 51 ++++++++++++++++++++++--------- 4 files changed, 59 insertions(+), 39 deletions(-) diff --git a/src/asp/GUI/MainWidget.cc b/src/asp/GUI/MainWidget.cc index dfb11335c..4144473b8 100644 --- a/src/asp/GUI/MainWidget.cc +++ b/src/asp/GUI/MainWidget.cc @@ -1117,7 +1117,7 @@ BBox2 MainWidget::expand_box_to_keep_aspect_ratio(BBox2 const& box) { //sw4.stop(); //vw_out() << "Render time 4 (seconds): " << sw4.elapsed_seconds() << std::endl; - }else{ + } else { // Overlay georeferenced images //Stopwatch sw5; //sw5.start(); @@ -1144,8 +1144,8 @@ BBox2 MainWidget::expand_box_to_keep_aspect_ratio(BBox2 const& box) { // TODO(oalexan1): Must render only the pixels that changed #pragma omp parallel for // this makes a big difference on Linux - for (int x = screen_box.min().x(); x < screen_box.max().x(); x++){ - for (int y = screen_box.min().y(); y < screen_box.max().y(); y++){ + for (int x = screen_box.min().x(); x < screen_box.max().x(); x++) { + for (int y = screen_box.min().y(); y < screen_box.max().y(); y++) { // Convert from a pixel as seen on screen to the world coordinate system. Vector2 world_pt = screen2world(Vector2(x, y)); @@ -1154,23 +1154,25 @@ BBox2 MainWidget::expand_box_to_keep_aspect_ratio(BBox2 const& box) { Vector2 p; try { p = MainWidget::world2image(world_pt, i); - bool is_in = (p[0] >= 0 && p[0] <= m_images[i].img.cols() - 1 && - p[1] >= 0 && p[1] <= m_images[i].img.rows() - 1); - if (!is_in) continue; // out of range + //bool is_in = (p[0] >= 0 && p[0] <= m_images[i].img.cols() - 1 && + // p[1] >= 0 && p[1] <= m_images[i].img.rows() - 1); + //if (!is_in) continue; // out of range }catch ( const std::exception & e ) { continue; } // Convert to scaled image pixels and snap to integer value - // TODO(oalexan1): This may introduce subpixel artifacts. - p = round(p/scale_out); + // TODO(oalexan1): There is a bug. Try plotting a 3x3 image with a + // georef. It will truncate half of the boundary pixels. + p = floor(p/scale_out); - if (!region_out.contains(p)) continue; // out of range again + // if (!region_out.contains(p)) continue; // out of range again int px = p.x() - region_out.min().x(); int py = p.y() - region_out.min().y(); - if (px < 0 || py < 0 || px >= qimg.width() || py >= qimg.height() ){ - vw_throw(ArgumentErr() << "Book-keeping failure.\n"); + if (px < 0 || py < 0 || px >= qimg.width() || py >= qimg.height()) { + continue; + //vw_throw(ArgumentErr() << "Book-keeping failure.\n"); } qimg2.setPixel(x-screen_box.min().x(), // Fill the temp QImage object y-screen_box.min().y(), diff --git a/src/asp/Sessions/StereoSession.cc b/src/asp/Sessions/StereoSession.cc index 202385388..21edb8009 100644 --- a/src/asp/Sessions/StereoSession.cc +++ b/src/asp/Sessions/StereoSession.cc @@ -156,18 +156,15 @@ namespace asp { } // The DEM the user provided better be the one used for map projection. - // Give a warning, as this may result in subtle differences. - if (input_dem != dem_file) { - vw::vw_out(WarningMessage) - << "The DEM used for map projection is different " - << "from the one provided currently.\n" - << "Mapprojection DEM: " << dem_file << "\n" - << "Current DEM: " << input_dem << "\n" - << "Use 'image_calc -c var_0 --mo DEM_FILE=myDem.tif in.tif -o out.tif -d float32' " - << "to change the DEM name in the " - << "geoheader of mapprojected images. Using the current DEM.\n"; - dem_file = input_dem; - } + // Give an error, as the results can be very different with the wrong DEM. + if (input_dem != dem_file) + vw::vw_throw(ArgumentErr() << "The DEM used for map projection is different " + << "from the one provided currently.\n" + << "Mapprojection DEM: " << dem_file << "\n" + << "Current DEM: " << input_dem << "\n" + << "Use 'image_calc -c var_0 --mo DEM_FILE=myDem.tif in.tif -o out.tif -d float32' " + << "to change the DEM name in the geoheader of mapprojected images, " + << "but only if sure that the DEMs are equivalent.\n"); // If this is set to none, it means no bundle-adjust prefix was used if (adj_prefix == "NONE") diff --git a/src/asp/Tools/bundle_adjust.cc b/src/asp/Tools/bundle_adjust.cc index b23f49481..0b7fd11d3 100644 --- a/src/asp/Tools/bundle_adjust.cc +++ b/src/asp/Tools/bundle_adjust.cc @@ -1792,7 +1792,7 @@ void handle_arguments(int argc, char *argv[], Options& opt) { "Explicitly set the datum semi-minor axis in meters (see above).") ("session-type,t", po::value(&opt.stereo_session)->default_value(""), "Select the stereo session type to use for processing. Usually the program can select this automatically by the file extension, except for xml cameras. See the doc for options.") - ("min-matches", po::value(&opt.min_matches)->default_value(30), + ("min-matches", po::value(&opt.min_matches)->default_value(5), "Set the minimum number of matches between images that will be considered.") ("max-pairwise-matches", po::value(&opt.max_pairwise_matches)->default_value(10000), "Reduce the number of matches per pair of images to at most this " diff --git a/src/asp/Tools/parallel_stereo b/src/asp/Tools/parallel_stereo index 3c9d1a7da..37cae3d6d 100644 --- a/src/asp/Tools/parallel_stereo +++ b/src/asp/Tools/parallel_stereo @@ -770,6 +770,16 @@ def keepOnlySpecified(keep_only, out_prefix, subdirs): pc_file = glob.glob(out_prefix + "-PC.tif") if len(pc_file) > 0: print("Output point cloud: " + pc_file[0]) + +def set_corr_tile_size(new_corr_tile_size, args, settings): + """ + Set corr_tile_size to a new value, and update the settings and args. + """ + + corr_tile_size = new_corr_tile_size + set_option(args, '--corr-tile-size', [str(corr_tile_size)]) + settings['corr_tile_size'] = [str(corr_tile_size)] + return (corr_tile_size, settings, args) if __name__ == '__main__': usage = '''parallel_stereo [options] [] @@ -837,8 +847,8 @@ if __name__ == '__main__': 'successful run combine the results from subdirectories into .tif ' + \ 'files with the given output prefix, and delete the ' + \ 'subdirectories. If set to "essential", keep only PC.tif and the ' + \ - 'files needed to recreate it (those ending with .exr, -L.tif, -F.tif). ' + \ - 'If set to "unchanged", keep the run directory as it ' + \ + 'files needed to recreate it (those ending with .exr, -L.tif, ' + \ + '-F.tif). If set to "unchanged", keep the run directory as it ' + \ 'is. For fine-grained control, specify a quoted list of suffixes of ' + \ 'files to keep, such as ".exr .match -L.tif -PC.tif".') p.add_argument('--sparse-disp-options', dest='sparse_disp_options', @@ -893,6 +903,10 @@ if __name__ == '__main__': if opt.threads_single is None: opt.threads_single = get_num_cpus() + # Validate option keep-only + if opt.keep_only not in ["all_combined", "essential", "unchanged"]: + raise Exception('Invalid value for --keep-only: ' + opt.keep_only) + # If corr-seed-mode was not specified, read it from the file if opt.seed_mode is None: opt.seed_mode = parse_corr_seed_mode(opt.stereo_file) @@ -973,6 +987,9 @@ if __name__ == '__main__': # Set the job size by default when using SGM corr_tile_size = int(settings['corr_tile_size'][0]) if using_padded_tiles: + # TODO(oalexan1): The default behavior should be sgm collar of 256, + # and padded tiles of 2048. This is not the case now. What to do + # for local epipolar alignment? if ('--job-size-w' not in sys.argv[1:]) and ('--job-size-h' not in sys.argv[1:]): # If the user did not manually specify the job size, set it equal # to the correlation tile size. @@ -987,33 +1004,38 @@ if __name__ == '__main__': if opt.job_size_w != corr_tile_size: print("Making --corr-tile-size equal to --job-size-h, that is equal to " \ + str(opt.job_size_h) + ".") - - # Must apply the changes to 3 places - corr_tile_size = opt.job_size_h - set_option(args, '--corr-tile-size', [str(corr_tile_size)]) - settings['corr_tile_size'] = [str(corr_tile_size)] + # Let corr_tile_size be job_size_h. Update in 3 places. + (corr_tile_size, settings, args) = \ + set_corr_tile_size(opt.job_size_h, args, settings) + else: # Make corr_tile_size the minimum of opt.job_size_h and opt.job_size_w if corr_tile_size > opt.job_size_h or corr_tile_size > opt.job_size_w: # Must apply the changes to three places - corr_tile_size = min([opt.job_size_h, opt.job_size_w]) - set_option(args, '--corr-tile-size', [str(corr_tile_size)]) - settings['corr_tile_size'] = [str(corr_tile_size)] + new_val = min([opt.job_size_h, opt.job_size_w]) + (corr_tile_size, settings, args) = \ + set_corr_tile_size(new_val, args, settings) print("Making --corr-tile-size the minimum of --job-size-h and --job-size-w, " \ "that is equal to " + str(corr_tile_size) + ".") print ('Using tiles (before collar addition) of ' + str(opt.job_size_w) + \ ' x ' + str(opt.job_size_h) + ' pixels.') - print('Using a collar (padding) for each tile of ' + settings['collar_size'][0] + ' pixels.') + print('Using a collar (padding) for each tile of ' + settings['collar_size'][0] + \ + ' pixels.') + # Print here the size of padded tiles + #if using_padded_tiles: + # print ('Using padded tiles (after collar addition) of ' + str(corr_tile_size) + \ + # ' x ' + str(corr_tile_size) + ' pixels.') + # See if to resume at triangulation. This logic must happen after we figured # if we need padded tiles, otherwise the bookkeeping will be wrong. if opt.tile_id is None and opt.prev_run_prefix is not None: print("Starting at the triangulation stage while reusing a previous run.") opt.entry_point = Step.tri if opt.stop_point <= Step.tri: - raise Exception('Cannot resume at triangulatoin if --stop-point ' + \ + raise Exception('Cannot resume at triangulation if --stop-point ' + \ 'is set to stop before that.') sym_link_prev_run(opt.prev_run_prefix, out_prefix) @@ -1053,9 +1075,8 @@ if __name__ == '__main__': num_pairs = int(settings['num_stereo_pairs'][0]) if num_pairs > 1: - - # Run multiview logic. - + # Run multiview logic. + # TODO(oalexan1): This must be a function. if opt.keep_only == "all_combined": # Change the default to "unchanged" opt.keep_only = "unchanged"