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

[SofaKernel] Clean API message for Image loading #503

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions SofaKernel/framework/sofa/helper/io/Image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ void Image::init(unsigned width, unsigned height, unsigned bpp)
channels = RGBA;
break;
default:
msg_error("Image") << "init(): Unsupported bpp: " << bpp << msgendl;
msg_error() << "init(): Unsupported bpp: " << bpp << msgendl;
return;
}

Expand All @@ -396,7 +396,7 @@ bool Image::load(std::string filename)
{
SOFA_UNUSED(filename);

msg_warning("Image") << "This Image format did not implement load()";
msg_warning() << "This Image format did not implement load()";
return false;
}

Expand All @@ -405,17 +405,33 @@ bool Image::save(std::string filename, int compression_level)
SOFA_UNUSED(filename);
SOFA_UNUSED(compression_level);

msg_warning("Image") << "This Image format did not implement save()";
msg_warning() << "This Image format did not implement save()";
return false;
}

Image* Image::Create(std::string filename)
{
std::string loader="default";
std::string extension="default";
std::string::size_type p = filename.rfind('.');
if (p!=std::string::npos)
loader = std::string(filename, p+1);
return FactoryImage::CreateObject(loader, filename);
extension = std::string(filename, p+1);
Image* createdImage = FactoryImage::CreateObject(extension, filename);
if( extension != "default" )
{
if(!createdImage )
{
helper::vector<std::string> validExtensions;
helper::io::Image::FactoryImage::getInstance()->uniqueKeys(std::back_inserter(validExtensions));
msg_error("Image") << "Could not load image with extension " << extension << ". Valid extensions: " << validExtensions;
}
}
else
{
helper::vector<std::string> validExtensions;
helper::io::Image::FactoryImage::getInstance()->uniqueKeys(std::back_inserter(validExtensions));
msg_error("Image") << "No extension detected. Valid extensions: " << validExtensions;
Copy link
Contributor

Choose a reason for hiding this comment

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

Tips of the day:
If you have a lot of msg_error("xxx") you can use the macro
MSG_REGISTER_CLASS(....)
After using it you can just do msg_info() <<
search example of how to use it in the sofa source code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there doc on this macro @damienmarchal ?
I wrote the macro outside the namespace in the .h file but it did not work. Apparently there is some tricks but I don't want to make any forward definition or write the macro in the .cpp

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no tricks, the macro is just a plain gold old macro that replace the macro with a longer text. I don't understand the connection you make with forward definition and not having the macro used in the .cpp. But you can find a lot of uses of the macro in the code base (eg: SphereLoader.cpp, meshTrian.cpp, ...)

I definitively support anyone writing the corresponding docs in the messaging documentation :)

}
return createdImage;
}

} // namespace io
Expand Down
3 changes: 3 additions & 0 deletions SofaKernel/framework/sofa/helper/io/Image.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,7 @@ extern template class SOFA_HELPER_API Factory<std::string, io::Image, std::strin

} // namespace sofa

/// This line register Image to the messaging system
MSG_REGISTER_CLASS(sofa::helper::io::Image, "Image")

#endif
43 changes: 21 additions & 22 deletions SofaKernel/modules/SofaBaseVisual/VisualModelImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ void VisualModelImpl::setMesh(helper::io::Mesh &objLoader, bool tex)
g.quad0 = facet2tq[g0.p0][NBQ];
g.nbq = facet2tq[g0.p0+g0.nbp][NBQ] - g.quad0;
if (g.materialId == -1 && !g0.materialName.empty())
sout << "face group " << ig << " name " << g0.materialName << " uses missing material " << g0.materialName << " " << sendl;
msg_info() << "face group " << ig << " name " << g0.materialName << " uses missing material " << g0.materialName << " ";
}
}

Expand Down Expand Up @@ -337,7 +337,7 @@ void VisualModelImpl::setMesh(helper::io::Mesh &objLoader, bool tex)
nbVOut += s;
}

sout << nbVIn << " input positions, " << nbVOut << " final vertices. " << sendl;
msg_info() << nbVIn << " input positions, " << nbVOut << " final vertices. ";

if (nbVIn != nbVOut)
vsplit = true;
Expand Down Expand Up @@ -440,7 +440,7 @@ void VisualModelImpl::setMesh(helper::io::Mesh &objLoader, bool tex)
idxs[j] = vertTexNormMap[verts[j]][std::make_pair((tex?texs[j]:-1), (m_useNormals.getValue() ? norms[j] : 0))];
if ((unsigned)idxs[j] >= (unsigned)nbVOut)
{
serr << this->getPathName()<<" index "<<idxs[j]<<" out of range"<<sendl;
msg_error() << this->getPathName()<<" index "<<idxs[j]<<" out of range";
idxs[j] = 0;
}
}
Expand Down Expand Up @@ -481,11 +481,17 @@ bool VisualModelImpl::load(const std::string& filename, const std::string& loade
std::string textureFilename(textureName);
if (sofa::helper::system::DataRepository.findFile(textureFilename))
{
sout << "loading file " << textureName << sendl;
loadTexture(textureName);
msg_info() << "loading file " << textureName;
bool textureLoaded = loadTexture(textureName);
if(!textureLoaded)
{
msg_error()<<"Texture "<<textureName<<" cannot be loaded";
}
}
else
serr << "Texture \"" << textureName << "\" not found" << sendl;
{
msg_error() << "Texture \"" << textureName << "\" not found";
}
}

// Make sure all Data are up-to-date
Expand Down Expand Up @@ -554,14 +560,14 @@ bool VisualModelImpl::load(const std::string& filename, const std::string& loade
}
else
{
serr << "Mesh \"" << filename << "\" not found" << sendl;
msg_error() << "Mesh \"" << filename << "\" not found";
}
}
else
{
if ((m_positions.getValue()).size() == 0 && (m_vertices2.getValue()).size() == 0)
{
sout << "VisualModel: will use Topology." << sendl;
msg_info() << "will use Topology.";
useTopology = true;
}

Expand Down Expand Up @@ -770,7 +776,7 @@ void VisualModelImpl::init()
}
else
{
sout << "Use topology " << m_topology->getName() << sendl;
msg_info() << "Use topology " << m_topology->getName();
// add the functions to handle topology changes.
if (m_handleDynamicTopology.getValue())
{
Expand Down Expand Up @@ -817,7 +823,6 @@ void VisualModelImpl::computeNormals()
if (vertNormIdx.empty())
{
int nbn = (vertices).size();
//serr << "CN0("<<nbn<<")"<<sendl;

ResizableExtVector<Deriv>& normals = *(m_vnormals.beginEdit());

Expand Down Expand Up @@ -868,7 +873,6 @@ void VisualModelImpl::computeNormals()
if (vertNormIdx[i] >= nbn)
nbn = vertNormIdx[i]+1;
}
//serr << "CN1("<<nbn<<")"<<sendl;

normals.resize(nbn);
for (int i = 0; i < nbn; i++)
Expand Down Expand Up @@ -1116,11 +1120,6 @@ void VisualModelImpl::updateVisual()
last = m_vtexcoords.getValue().size();
}
*/
//sout << "VMI::updateVisual()" << sendl;
//if ((m_positions.getValue()).size()>10)
// sout << "positions[10] = " << m_positions.getValue()[10] << sendl;
//if ((m_vertices.getValue()).size()>10)
// sout << "vertices[10] = " << m_vertices.getValue()[10] << sendl;
if (modified && (!getVertices().empty() || useTopology))
{
if (useTopology)
Expand Down Expand Up @@ -1515,16 +1514,16 @@ void VisualModelImpl::handleTopologyChange()

if(!is_in_shell)
{
sout << "INFO_print : Vis - triangle is forgotten in SHELL !!! global indices (point, triangle) = ( " << last << " , " << ind_forgotten << " )" << sendl;
msg_info() << "INFO_print : Vis - triangle is forgotten in SHELL !!! global indices (point, triangle) = ( " << last << " , " << ind_forgotten << " )";
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here...remove "INFO_print:"


if(ind_forgotten<m_topology->getNbTriangles())
{
const core::topology::BaseMeshTopology::Triangle t_forgotten = m_topology->getTriangle(ind_forgotten);
sout << "INFO_print : Vis - last = " << last << sendl;
sout << "INFO_print : Vis - lastIndexVec[i] = " << lastIndexVec[i] << sendl;
sout << "INFO_print : Vis - tab.size() = " << tab.size() << " , tab = " << tab << sendl;
sout << "INFO_print : Vis - t_local rectified = " << triangles[j_loc] << sendl;
sout << "INFO_print : Vis - t_global = " << t_forgotten << sendl;
msg_info() << "Vis - last = " << last << msgendl
<< "Vis - lastIndexVec[i] = " << lastIndexVec[i] << msgendl
<< "Vis - tab.size() = " << tab.size() << " , tab = " << tab << msgendl
<< "Vis - t_local rectified = " << triangles[j_loc] << msgendl
<< "Vis - t_global = " << t_forgotten;
}
}
}
Expand Down