Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stereo Photometry: Add robustness to unexpected inputs #1452

Merged
merged 17 commits into from
Jun 12, 2023

Conversation

cbentejac
Copy link
Contributor

@cbentejac cbentejac commented Jun 5, 2023

Description

Related Meshroom pull request: alicevision/Meshroom#2034

This PR builds on top of #999 and adds robustness to several cases that were not handled in the initial version. In particular:

  • If the input images were not placed in a folder starting with "ps_", they were not grouped together (using their pose ID), and were all evaluated on their own during the photometric stereo stage, causing crashes;
  • If spheres were not detected in some images, they are not written in the final JSON that describes the spheres' positions for every image, and they are not used in later stages;
  • If no mask folder is provided for the photometric stereo stage, no path is made up to try and load it (but the result is the same as if a path was provided but the mask failed to be opened: all the pixels in the image will be used);
  • Single images are now handled;
  • If all the provided images were not meant for stereo photometry, all the stages go through up to the photometric stereo one, which detects that no image was correct, and throws an explicit error. No crash occurs.

The method used to downscale images has also been replaced with an already existing one from the imageAlgo module.

Additionally, some clean-up has been done on the different files:

  • 4-space indentation is used everywhere;
  • Lines that were too long got split;
  • Some logs were fixed;
  • Timers have been added for the executables that were lacking one.

Features list

  • Fix crashes when the images are not located in the expected folder;
  • Add support for single images;
  • Fix crashes when sphere are not detected in the image(s).

Additionally fix documentation in `lightingEstimation.hpp`.
If there are some images in the input SfMData for which no sphere has
been detected, attempting to retrieve their sphere parameters in the tree
will cause a crash.

This commit checks that the node corresponding to an input image exists
before attempting to retrieve its parameters: if it does not, said node
is skipped and the image is not considered as an image to process during
the lighting calibration.

When writing the final JSON file that will contain the lighting models, the
list of images that do contain a sphere is provided as well as the SfMData.
This way, the file will only contain images containing spheres for which
the computation of a lighting model is valid.

Additionally, some variables in snake case have been renamed.
When writing the final JSON file that sums up the detected spheres for each
input image, we do not include images for which no sphere has been
detected, to avoid filling the JSON with information that cannot be used.

Later on, during the lighting calibration part, images from the input
SfMData that cannot be found in the sphere detection's JSON will be
ignored, which will prevent issues in further stages.
Stop using "downscaleImageInplace" which was added back to the Image
module for compilation purposes and use "resizeImage" from `imageAlgo`
instead. `<image/resampling.h>` includes are replaced with
`<image/imageAlgo.h>`.
This function is not used anywhere anymore and has been replaced with
"resizeImage" methods, defined in `imageAlgo`.
If no mask path is provided as an input, an empty string should be passed
to the `loadMask` function rather than making up a path.

When entering the `loadMask` function, there is now a distinction between
a mask that wasn't provided and a mask that doesn't exist. In both cases,
this leads to using a mask in which all the pixels are set to 1, but the
corresponding log will differ (in the first case, it is expected, while
it is not in the second, thus triggering a warning).
…image

If no sphere has been detected for an input image, its light has not been
calibrated, and building its light intensities' matrix will result in an
empty matrix.

During the photometric stereo, the light intensities are directly accessed
without any checks, as it is assumed that at this point, the light must
have been calibrated. If the matrix is empty, accessing it will cause a
crash.

This commit prevents this by checking beforehand that the size of the
matrix of light intensities corresponds to the size of input images. If
this is not the case, the image is skipped with a warning message.

By the end of the process, if all the images have been skipped, an error
is thrown, since it means that either no input was meant for the
photometric stereo or something wrong occurred during the previous stages.
If the all the processed images were included in lists of size 1, it means
that none of them shared the same pose ID. To share the same pose ID,
images must be grouped together in a folder starting with "ps_" when they
are provided to the `CameraInit` node.

If there was no group of images, this might mean that images were not
placed in correct folders, or that all the processed images were
independent. A warning is issued to let the user know that the results
might not be the expected ones because of the location of the input images.
The descriptions of parameters is written on a different line for
readability purposes.
@cbentejac cbentejac added this to the 3.1.0 milestone Jun 5, 2023
@cbentejac cbentejac self-assigned this Jun 5, 2023
Comment on lines +165 to +185
MatrixXf rhoTimesN(nbRows_all_rhoTimesN, 9);
{
std::size_t nbRow = 0;
for (const MatrixXf& matrix : _all_rhoTimesN.at(channel))
{
for (int matrixRow = 0; matrixRow < matrix.rows(); ++matrixRow)
rhoTimesN.row(nbRow + matrixRow) = matrix.row(matrixRow);
nbRow += matrix.rows();
}
}

// agglomerate pictureChannel
MatrixXf pictureChannel(nbRows_all_pictureChannel, 1);
{
std::size_t nbRow = 0;
for (const MatrixXf& matrix : _all_pictureChannel.at(channel))
{
for(int matrixRow = 0; matrixRow < matrix.rows(); ++matrixRow)
pictureChannel.row(nbRow + matrixRow) = matrix.row(matrixRow);
nbRow += matrix.rows();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make a function? the code seems the same...

Additionally use camel case instead of snake case for the few variables
that were still in snake case.
Additionally rename some variables with typos and some variables that used
snake case instead of camel case.
@fabiencastan fabiencastan merged commit ba20b09 into develop Jun 12, 2023
@fabiencastan fabiencastan deleted the dev/psImprovements branch June 12, 2023 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants