LiangliangNan / Easy3D

A lightweight, easy-to-use, and efficient C++ library for processing and rendering 3D data
GNU General Public License v3.0
1.37k stars 245 forks source link

Multiple threads - Failed creating array buffer / vertex buffer #99

Closed laxnpander closed 2 years ago

laxnpander commented 2 years ago

Hey again,

I am facing a new issue, maybe you can help me with that. I think it is once again linked to the fact, that I dynamically update a 3D point cloud + some drawables. I get the following error every once in a while:

E 23/11/2021 09:07:39.940 vertex_array_object.cpp:111] failed creating array buffer
E 23/11/2021 09:07:39.940 drawable.cpp:147] failed creating vertex buffer

Whenever this error occures, there is a graphic bug in my viewer. You can see the effects here: https://drive.google.com/file/d/1i5n3XzKs92T5Hnv5G0g8yHqUzVGThat0/view?usp=sharing

What happens is I think there is some memory access error, so that when he tries to render the LinesDrawable he uses the wrong points or indices to connect the dots. I observe other effects connected to this as well, where with a new update my whole point cloud suddenly turns green and in the next frame it is okay (RGB colored) again. My feeling is, this might be related to the fact that the viewer has not finished rendering the last scene and is already called to update again. Do you think this is possible? And is there a way to check whether the graphics card / rendering backend is currently busy?

You already said it is not designed to work well with dynamic updating performancewise. I wouldn't mind performance loss in general, but visual artifacts like this are quite annoying. Do you have a suggestion how to solve it? I already tried to protect my calls to "update_vertex_buffer" with a mutex, so I don't have concurrent calls on it. But it does not seem to help, probably because the Graphics Card is involved.

For reference here is my viewer code. Interesting part is the processKeyframe / drawKeyframe function. Every camera is added as drawable, not sure if this is also not the best way to handle it?

#include <visualization/easy3d.h>

#include <easy3d/fileio/point_cloud_io.h>
#include <easy3d/renderer/camera.h>
#include <easy3d/core/vec.h>

#include <core/projective_geometry.h>

using namespace spear;

Easy3d::Easy3d(const nl::json &json)
 : m_is_running(false),
   m_n_kfs(0),
   m_cloud(nullptr),
   m_viewer(new Easy3dViewer("SPEAR")),
   m_draw_cloud(nullptr),
   m_draw_traj(nullptr)
{
  // Setup 3D Viewer
  easy3d::logging::initialize(true, true, true, "default", 0);

  initModel();
}

Easy3d::~Easy3d()
{
  delete m_cloud;
  delete m_viewer;
}

void Easy3d::run()
{
  if (!m_is_running)
    m_viewer->run();
}

void Easy3d::close()
{
  m_viewer->exit();
}

void Easy3d::clear()
{
  m_viewer->delete_model(m_cloud);
  m_viewer->delete_drawable(m_draw_cloud);
  m_viewer->delete_drawable(m_draw_traj);

  for (const auto &it : m_draw_cams)
    m_viewer->delete_drawable(it);

  m_lookup_lm_vertex.clear();

  m_viewer->update();

  initModel();
}

void Easy3d::initModel()
{
  m_cloud = new easy3d::PointCloud;

  m_cloud->add_vertex_property<easy3d::vec3>("v:color");
  m_cloud->add_vertex_property<unsigned int>("v:nobs");
  m_cloud->add_vertex_property<real>("v:alpha");

  // PointCloud must not be empty when it is added to the viewer. We therefore add one point.
  easy3d::PointCloud::Vertex v = m_cloud->add_vertex(easy3d::vec3(0.0, 0.0, 0.0));

  m_viewer->add_model(m_cloud);
  m_draw_cloud = m_cloud->renderer()->get_points_drawable("vertices");
  m_draw_cloud->set_impostor_type(easy3d::PointsDrawable::PLAIN);
  m_draw_cloud->set_point_size(3.0f);
  m_draw_cloud->set_property_coloring(easy3d::State::VERTEX, "v:color");
  m_draw_cloud->set_visible(true);

  m_cloud->delete_vertex(v);
}

void Easy3d::addFrame(const Frame::WPtr &f_wp)
{
  auto f = f_wp.lock();

  if (f && f->cam()->hasPose())
  {
    m_mutex_queue_f.lock();
    //m_queue_f.push_front(f);
    m_mutex_queue_f.unlock();
  }

}

void Easy3d::addKeyframe(const Keyframe::WPtr &kf_wp)
{
  auto kf = kf_wp.lock();

  if (kf)
  {
    m_mutex_queue_kf.lock();
    m_queue_kf.push_front(kf);
    m_mutex_queue_kf.unlock();
  }
}

void Easy3d::addLandmark(const Landmark::WPtr &lm_wp)
{
  auto lm = lm_wp.lock();

  if (lm)
  {
    m_mutex_queue_lms.lock();
    m_queue_lms.push_front(lm);
    m_mutex_queue_lms.unlock(); 
  }
}

void Easy3d::update()
{
  processFrames();
  processKeyframes();
  processLandmarks();
}

void Easy3d::processFrames()
{
  if (m_queue_f.empty() || !m_viewer->current_model())
    return;

  m_mutex_queue_f.lock();
  std::deque<Frame::Ptr> frames = m_queue_f;
  m_queue_f.clear();
  m_mutex_queue_f.unlock();

  m_mutex_update.lock();

  for (const auto &f : frames)
  {
    if (m_f_prev && f->cam()->hasPose())
    {
      drawFrame(f);
      m_viewer->update();
    }
    m_f_prev = f;
  }

  m_mutex_update.unlock();
}

void Easy3d::processKeyframes()
{
  if (m_queue_kf.empty() || !m_viewer->current_model())
    return;

  m_mutex_queue_kf.lock();
  std::deque<Keyframe::WPtr> keyframes = m_queue_kf;
  m_queue_kf.clear();
  m_mutex_queue_kf.unlock();
  for (const auto &it : keyframes)
  {
    auto kf = it.lock();

    easy3d::LinesDrawable* kf_drawn = drawKeyframe(kf);
    m_draw_cams.emplace_back(kf_drawn);
    m_viewer->add_drawable(kf_drawn);
    m_viewer->update();
    m_lookup_drawn_from_kfid[kf->id()] = kf_drawn;
    m_n_kfs++;
  }

  m_mutex_update.unlock();

}

void Easy3d::processLandmarks()
{
  // Check for empty is safe, because this function is the only one removing elements from the queue
  if (m_queue_lms.empty())
    return;

  m_mutex_queue_lms.lock();
  std::deque<Landmark::WPtr> landmarks = m_queue_lms;
  m_queue_lms.clear();
  m_mutex_queue_lms.unlock();

  m_mutex_update.lock();

  auto points = m_cloud->get_vertex_property<easy3d::vec3>("v:point");
  auto colors = m_cloud->get_vertex_property<easy3d::vec3>("v:color");
  auto nobs   = m_cloud->get_vertex_property<unsigned int>("v:nobs");
  auto alpha  = m_cloud->get_vertex_property<real>("v:alpha");

  // Extract positional information from landmarks
  for (const auto& it : landmarks)
  {
    auto lm = it.lock();
    if (lm)
    {
      easy3d::vec3 pos((float)lm->x(), (float)lm->y(), (float)lm->z());

      easy3d::PointCloud::Vertex v;

      auto it = m_lookup_lm_vertex.find(lm->id());
      if (it == m_lookup_lm_vertex.end())
      {
        v = m_cloud->add_vertex(pos);
        m_lookup_lm_vertex[lm->id()] = v;
      }
      else
      {
        v = it->second;
      }

      points[v] = pos;
      nobs[v]   = lm->nobs();
      alpha[v]  = lm->alpha();
      colors[v]  = toVec3(lm->color());

      m_box.add_point(pos);
    }
  } 

  m_draw_cloud->update_vertex_buffer(points.vector(), true);
  m_draw_cloud->update_color_buffer(colors.vector(), true);

  m_viewer->camera()->setSceneRadius( 
    std::max(
      easy3d::distance(m_viewer->camera()->sceneCenter(), m_box.min_point()),
      easy3d::distance(m_viewer->camera()->sceneCenter(), m_box.max_point())
      )
  );

  m_viewer->update();
  m_mutex_update.unlock();
}

void Easy3d::drawFrame(const Frame::Ptr &f)
{
  Vector3r t_curr = f->cam()->twc().translation();
  Vector3r t_prev = m_f_prev->cam()->twc().translation();

  const std::string name = "frame_" + f->id();

  const std::vector<easy3d::vec3> pts_line_series
  {
    easy3d::vec3((float)t_curr(0), (float)t_curr(1), (float)t_curr(2)),
    easy3d::vec3((float)t_prev(0), (float)t_prev(1), (float)t_prev(2))
  };

  const std::vector<unsigned int> idx_line_series
  {
    1, 0
  };

  auto draw_traj = new easy3d::LinesDrawable(name);
  draw_traj->set_model(m_cloud);
  draw_traj->update_vertex_buffer(pts_line_series);
  draw_traj->update_element_buffer(idx_line_series);
  draw_traj->set_uniform_coloring(easy3d::vec4(0.0f, 0.0f, 1.0f, 1.0f));    // r, g, b, a
  draw_traj->set_line_width(1.0f);
  m_viewer->add_drawable(draw_traj);
}

easy3d::LinesDrawable* Easy3d::drawKeyframe(const Keyframe::Ptr &kf) const
{
  SE3r twc = kf->cam()->twc();

  // First create the image boundaries as point cloud
  const std::vector<Point2r> bounds
  {
    {0.0, 0.0},
    {(real)kf->cam()->width(), 0.0},
    {0.0, (real)kf->cam()->height()},
    {(real)kf->cam()->width(), (real)kf->cam()->height()},
  };

  // Then project them to a defined depth
  const std::vector<real> depths(bounds.size(), 0.3);
  std::vector<Vector3r> pts_projected = kf->cam()->projectPointsToWorld(twc, bounds, depths);

  // Collect the 3d points as drawables for easy3d
  std::vector<easy3d::vec3> pts_drawable
  {
    easy3d::vec3((float)twc.translation()[0], (float)twc.translation()[1], (float)twc.translation()[2]),
    easy3d::vec3((float)pts_projected[0][0],  (float)pts_projected[0][1],  (float)pts_projected[0][2]),
    easy3d::vec3((float)pts_projected[1][0],  (float)pts_projected[1][1],  (float)pts_projected[1][2]),
    easy3d::vec3((float)pts_projected[2][0],  (float)pts_projected[2][1],  (float)pts_projected[2][2]),
    easy3d::vec3((float)pts_projected[3][0],  (float)pts_projected[3][1],  (float)pts_projected[3][2]),
  };

  // Create a polygon
  std::vector<unsigned int> idx_drawable = {
    1, 2, 2, 4, 4, 3, 3, 1, 0, 1, 0, 2, 0, 3, 0, 4
  };

  // Now add all the (x,y,z) of covisible keyframes so we can visualize them too
  for (const auto &it : kf->covisibilities())
  {
    auto kfi = it.second.lock();
    if (kfi)
    {
      Vector3r t = kfi->cam()->twc().translation();
      pts_drawable.push_back(easy3d::vec3(t(0), t(1), t(2)));

      int idx = pts_drawable.size()-1;
      idx_drawable.push_back(0);
      idx_drawable.push_back(idx);
    }
  }

  // Create the final object
  std::string name = "cam_" + m_n_kfs;

  auto draw_cam = new easy3d::LinesDrawable(name.c_str());
  draw_cam->set_model(m_cloud);
  draw_cam->update_vertex_buffer(pts_drawable, true);
  draw_cam->update_element_buffer(idx_drawable);
  draw_cam->set_uniform_coloring(easy3d::vec4(0.0f, 0.0f, 1.0f, 1.0f));    // r, g, b, a
  draw_cam->set_line_width(1.0f);

  return draw_cam;
}

easy3d::vec3 Easy3d::toVec3(const cv::Vec3b &cv_vec) const
{
  return std::move(easy3d::vec3(
    cv_vec(2)/255.0,
    cv_vec(1)/255.0,
    cv_vec(0)/255.0
  ));
}
LiangliangNan commented 2 years ago

My feeling is, this might be related to the fact that the viewer has not finished rendering the last scene and is already called to update again. Do you think this is possible? And is there a way to check whether the graphics card / rendering backend is currently busy?

Yes. That can cause an issue, and the graphics card behavior is not defined.

You already said it is not designed to work well with dynamic updating performancewise. I wouldn't mind performance loss in general, but visual artifacts like this are quite annoying. Do you have a suggestion how to solve it? I already tried to protect my calls to "update_vertex_buffer" with a mutex, so I don't have concurrent calls on it. But it does not seem to help, probably because the Graphics Card is involved.

"mutex" works for CPUs, but GPUs work completely differently.

For reference here is my viewer code. Interesting part is the processKeyframe / drawKeyframe function. Every camera is added as drawable, not sure if this is also not the best way to handle it?

If you have only a few cameras/images, then you will not notice the performance overhead. But if you have tens, hundreds, or even more cameras/images, I suggest you use a single drawable for all the cameras.

For easier debugging, I suggest you dump all keyframes into a file (*.kf) and load them into Mapple to see they are correct. If correctly visualized in Mapple, then it must be something wrong with the rendering or buffer data transfer. If still the same, then the keyframes are not computed correctly.

laxnpander commented 2 years ago

Yes. That can cause an issue, and the graphics card behavior is not defined.

Is there a way to avoid this? Can I check whether or not creation of the buffer was successful or not?

"mutex" works for CPUs, but GPUs work completely differently.

Jeah sure, my hope was that locking the mutex while creating the buffer also avoids the GPU from multi threaded access (as it is triggered from CPU processes). But this does not seem to work reliably.

If you have only a few cameras/images, then you will not notice the performance overhead. But if you have tens, hundreds, or even more cameras/images, I suggest you use a single drawable for all the cameras.

I tried that, but wasn't really happy. If I remember correctly, the visuals didn't update for the drawables even after calling update_vertex_buffer and a viewer update. Not sure why.

For easier debugging, I suggest you dump all keyframes into a file (*.kf) and load them into Mapple to see they are correct. If correctly visualized in Mapple, then it must be something wrong with the rendering or buffer data transfer. If still the same, then the keyframes are not computed correctly.

I am pretty confident, that the camera position / rotation is fine. It really looks like a memory issue.

LiangliangNan commented 2 years ago

If you modify one of the examples in Easy3D so it can reproduce the issue, I will be happy to investigate the issue. Any also, please let me know how it works without multi-threading.

laxnpander commented 2 years ago

@LiangliangNan: Okay, I will try to create something reproduceable within this week.

laxnpander commented 2 years ago

I will close this for now, I don't find the time to examine the problem in more detail. I will reopen once it comes back up again.

LiangliangNan commented 2 years ago

Okay. Thanks