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

About depth rendering implementation #25

Open
xEnVrE opened this issue Apr 2, 2020 · 2 comments
Open

About depth rendering implementation #25

xEnVrE opened this issue Apr 2, 2020 · 2 comments

Comments

@xEnVrE
Copy link
Collaborator

xEnVrE commented Apr 2, 2020

Lately, I was testing the depth rendering features implemented in some WIP commits in branch impl/depth.

In my use case, I only needed the depth and not the segmentation to be rendered and, after commenting out this part of the code

cv::Mat ogl_pixel(framebuffer_height_ / tiles_rows_, framebuffer_width_ / tiles_cols_, CV_8UC3);
glReadBuffer(GL_COLOR_ATTACHMENT0);
glPixelStorei(GL_PACK_ALIGNMENT, (ogl_pixel.step & 3) ? 1 : 4);
glPixelStorei(GL_PACK_ROW_LENGTH, ogl_pixel.step / ogl_pixel.elemSize());
glReadPixels(0, framebuffer_height_ - tile_img_height_, tile_img_width_, tile_img_height_, GL_BGR, GL_UNSIGNED_BYTE, ogl_pixel.data);
cv::flip(ogl_pixel, img, 0);

I was able to obtain a notable gain in performance (in terms of fps) that was beneficial for my application.

I have some questions, then, about a possible future stable implementation of depth rendering:

  • does it make sense to implement additional methods of SICAD::superimpose() that only return the depth (so that no time is wasted on segmentation rendering when not required)?
  • alternatively, since the name of che class is SICAD, that I would expand as Superimpose given the CAD model, is it maybe better to develop depth rendering in a separate class? I say so, as depth rendering is different from superimposing something on a given input RGB image. What do you think?

Thank you!

@claudiofantacci
Copy link
Collaborator

Notably, glReadPixels is the slowest call you would ever made on the whole OpenGL pipeline. What you should do, if you can, is to use the API that returns the ID of the memory location on the GPU and make all data manipulation there. However, I understand the point and it would make sense to have a richer API that only returns whatever needed. Maybe we could have a generic basic function that performs the rendering step, then you have separate methods to just get what you need from the GPU. This granularity probably allows a better performance tuning.

For your second point, SICAD makes little sense tbh. It should be SIMesh probably. I would do as said before. What do you think?

@xEnVrE
Copy link
Collaborator Author

xEnVrE commented Apr 4, 2020

I agree. Doubling the part of the code that is shared by segmentation and depth rendering in a new class for depth only would be awful and difficult to maintain. Having a basic method for rendering and then multiple ways to get the desired output is the probably the right way, as you said.

use the API that returns the ID of the memory location on the GPU and make all data manipulation there

I did not consider this option, thank you for the hint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants