Ray Tracer implementation


#34

use release it’ll be much faster, you might need debug if something crashes or you need to add breakpoints… but to test performance you need to do so in release, somethings like ofPixels iterators only really make a difference in release


#35

I’ve finally get parallel_for.
I’ve included the header /usr/local/lib/tbb in the xcode project, now I can #include "parallel_for.h" and use parallel_for.

I’m trying to understand the syntax, what you’ve suggested:

tbb::parallel_for(auto line : image->getPixels().getLines()){

It is not working, tbb expects a const blocked_range<size_t>& that I do not know what it could be in this case, but I think I should first define a container with all the pixel and then have an iterator, or define an index, that goes through them.

https://software.intel.com/en-us/blogs/2009/08/03/parallel_for-is-easier-with-lambdas-intel-threading-building-blocks

Do you have any hints?


#36
parallel_for( blocked_range<size_t>(0,pixels.getHeight()),
	    [=](const blocked_range<size_t>& r) {
	      for(auto line: pixels.getLines(r.begin(), r.end() - r.begin()){
                  auto y = line.getLineNum();
                  auto x = 0;
                  for (auto pixel: line.getPixels()) {
                    glm::vec3 P;
                    glm::vec3 w;

                    // Find the ray through (x, y) and the center of projection
                    camera.getPrimaryRay(float(x) + 0.5f, float(y) + 0.5f, width, height, P, w);
                    auto color = L_i(ofxRTRay(P, w));
                    pixel[0] = color.r;
                    pixel[1] = color.g;
                    pixel[2] = color.b;
                    pixel[3] = color.a;
                    x++;
                }
            }
         }
);

something like that should work. the way it works is:

parallel_for( blocked_range<size_t>(0,pixels.getHeight()),

inits the parallel for from 0…height which will call the passed lamda function passing the range it has to process.

in the lambda blocked_range is a size_t range where begin is the first line you have to process and end the last one + 1

take into account that x and y are now used by different threads so you can’t declare them outside of the for loop as before


#37

Wow. I see the problem with x and y init.
This line is actually giving me a problem:

for(auto line: pixels.getLines(r.begin(), r.end() - r.begin())) {

The error is No matching member function for call to 'getLines'

I’ve read the getLines signature, and I’ve tried to pass an init value, and the size, like this, but it is not working:

for(auto line: pixels.getLines(0, r.size())){

But is giving me the same error.Also, worth to mention that I’m initializing the pixels variable before parallel_for, with auto pixels = image->getPixels();

Also, in the loop the code completition is not suggesting me the method getLines

But I’ve it before the loop


#38

And of course also this is not working

    ofPixels_ <unsigned char>pixels = image->getPixels();
    parallel_for( tbb::blocked_range<size_t>(0,pixels.getHeight()),
                 [=](const tbb::blocked_range<size_t>& r) {
                     for(auto &line: pixels.getConstLines(r.begin(), r.end() - r.begin())) {

because when I try to access the pixel pixel[1] = color.g; it gives me an error because it is a const value.


#39

Thank you @roymacdonald and @arturo for your help in this issue, I’m back again with this problem with multithreading that is still not solved.
This is what I’ve tried. As getLines or getConstLines are returning const values, I’ve thought that it could make sense to have an empty ofPixel object, that I could fulfil during the loop. I’ve declared ofPixel renderPixel in my header and, in the loop, I’ve changed the code that set the color for each pixel:

 for (auto &pixel: line.getPixels()) {
     glm::vec3 P;
     glm::vec3 w;

     // Find the ray through (x, y) and the center of projection
     camera.getPrimaryRay(float(x) + 0.5f, float(y) + 0.5f, width, height, P, w);
     auto color = L_i(ofxRTRay(P, w));
     renderPixels.setColor(x,y,color);
//                             pixel[0] = color.r;
//                             pixel[1] = color.g;
//                             pixel[2] = color.b;
//                             pixel[3] = color.a;
     x++;
 }

But his does not work because (I think) the thread access the renderPixel object at the same time and there is some concurrency issue:

I’m now stuck with this problem, any suggestion or attempt is really appreciated.
The current status of the project is in the github repository https://github.com/edap/ofxRayTracer


#40

if you use no reference in the for loop you should get an object you can also write to


#41

if you use no reference in the for loop you should get an object you can also write to


#42

where do you mean?
If i change this:

for (auto &pixel: line.getPixels()) {

to this

for (auto pixel: line.getPixels()) {

I get an error:


#43

can you post the full function or at least all the for loops


#44

Sure. The whole class is here https://github.com/edap/ofxRayTracer/blob/master/src/ofxRayTracer.cpp

The method that loops through the rectangle and cast the ray is:

void ofxRayTracer::traceImage(const ofxRTPinholeCamera& camera, ofRectangle& rectangle, shared_ptr<ofImage>& image){
    const int width = int(rectangle.getWidth());
    const int height = int(rectangle.getHeight());

    auto startAtTime = ofGetElapsedTimeMillis();
    ofPixels_ <unsigned char>pixels = image->getPixels();

    parallel_for( tbb::blocked_range<size_t>(0,pixels.getHeight()),
                 [=](const tbb::blocked_range<size_t>& r) {
                     for(auto line: pixels.getConstLines(r.begin(), r.end() - r.begin())) {
                         auto y = line.getLineNum();
                         auto x = 0;
                         for (auto &pixel: line.getPixels()) {
                             glm::vec3 P;
                             glm::vec3 w;

                             // Find the ray through (x, y) and the center of projection
                             camera.getPrimaryRay(float(x) + 0.5f, float(y) + 0.5f, width, height, P, w);
                             auto color = L_i(ofxRTRay(P, w));
                             //renderPixels.setColor(x,y,color);
                             pixel[0] = color.r;
                             pixel[1] = color.g;
                             pixel[2] = color.b;
                             pixel[3] = color.a;
                             x++;
                         }
                     }
                }
    );

    image->update();
    displayTime(ofGetElapsedTimeMillis() - startAtTime);
}


#45

i think the problem is here:

for(auto line: pixels.getConstLines(r.begin(), r.end() - r.begin())) {

getConstLines returns const lines so you can’t modify anything from there on. Try using getLines instead


#46

I can’t, the method getLines is not available. The error message says: No matching member function for call to getLines
In the post Ray Tracer implementation there is a screenshot that explains the problem and why I went for getConstLines


#47

ah, can you try using:

[&](const tbb::blocked_range<size_t>& r) {

instead of

[=](const tbb::blocked_range<size_t>& r) {

note the & instead of = which means use the context of the lambda function by reference instead of by copy


#48

This fixed the const lines problem, but still, i can’t set the new value of the pixel


#49

And if I try to use another ofPixel object (renderPixel) instead that one where I’m iterating, the program crash with the errors in the post Ray Tracer implementation

    ofPixels renderPixels;
    renderPixels.allocate(width, height, OF_IMAGE_COLOR_ALPHA);
    ofPixels_ <unsigned char>pixels = image->getPixels();

    parallel_for( tbb::blocked_range<size_t>(0,pixels.getHeight()),
                 [&](const tbb::blocked_range<size_t>& r) {
                     for(auto line: pixels.getLines(r.begin(), r.end() - r.begin())) {
                         auto y = line.getLineNum();
                         auto x = 0;
                         for (auto &pixel: line.getPixels()) {
                             glm::vec3 P;
                             glm::vec3 w;

                             // Find the ray through (x, y) and the center of projection
                             camera.getPrimaryRay(float(x) + 0.5f, float(y) + 0.5f, width, height, P, w);
                             auto color = L_i(ofxRTRay(P, w));
                             renderPixels.setColor(x,y,color);
                             //pixel[1] = color.g;
                             //pixel[2] = color.b;
                             //pixel[3] = color.a;
                             x++;
                         }
                     }
                }
    );

    //image->update();
    image->setFromPixels(renderPixels);
    displayTime(ofGetElapsedTimeMillis() - startAtTime);

#50

And, If I use setColor instead pixel[index], like:

    ofPixels_ <unsigned char>pixels = image->getPixels();

    parallel_for( tbb::blocked_range<size_t>(0,pixels.getHeight()),
                 [&](const tbb::blocked_range<size_t>& r) {
                     for(auto line: pixels.getLines(r.begin(), r.end() - r.begin())) {
                         auto y = line.getLineNum();
                         auto x = 0;
                         for (auto &pixel: line.getPixels()) {
                             glm::vec3 P;
                             glm::vec3 w;

                             // Find the ray through (x, y) and the center of projection
                             camera.getPrimaryRay(float(x) + 0.5f, float(y) + 0.5f, width, height, P, w);
                             auto color = L_i(ofxRTRay(P, w));
                             pixels.setColor(x, y, color);
                             //pixel[1] = color.g;
                             //pixel[2] = color.b;
                             //pixel[3] = color.a;
                             x++;
                         }
                     }
                }
    );
    image->update();
    displayTime(ofGetElapsedTimeMillis() - startAtTime);

The error is

So, I do not think that is fine to access the ofPixel object by reference in the loop.


#51

have you tried

 for (auto pixel: line.getPixels()) {

#52

Yes, I have the same error as in the post Ray Tracer implementation

    ofPixels_ <unsigned char>pixels = image->getPixels();

    parallel_for( tbb::blocked_range<size_t>(0,pixels.getHeight()),
                 [&](const tbb::blocked_range<size_t>& r) {
                     for(auto line: pixels.getLines(r.begin(), r.end() - r.begin())) {
                         auto y = line.getLineNum();
                         auto x = 0;
                         //for (auto &pixel: line.getPixels()) {
                         for (auto pixel: line.getPixels()) {
                             glm::vec3 P;
                             glm::vec3 w;

                             // Find the ray through (x, y) and the center of projection
                             camera.getPrimaryRay(float(x) + 0.5f, float(y) + 0.5f, width, height, P, w);
                             auto color = L_i(ofxRTRay(P, w));
                             //pixels.setColor(x, y, color);
                             pixel[1] = color.g;
                             pixel[2] = color.b;
                             pixel[3] = color.a;
                             x++;
                         }
                     }
                }
    );
    image->update();

#53

i think this is just a bug in the static analizer, for me i see the same error but compiling works fine with this code:

    ofPixels & pixels = image->getPixels();

    parallel_for( tbb::blocked_range<size_t>(0,pixels.getHeight()),
                 [&](const tbb::blocked_range<size_t>& r) {
                     for(auto line: pixels.getLines(r.begin(), r.end() - r.begin())) {
                         auto y = line.getLineNum();
                         auto x = 0;
                         for (auto pixel: line.getPixels()) {
                             glm::vec3 P;
                             glm::vec3 w;

                             // Find the ray through (x, y) and the center of projection
                             camera.getPrimaryRay(float(x) + 0.5f, float(y) + 0.5f, width, height, P, w);
                             auto color = L_i(ofxRTRay(P, w));
                             //pixels.setColor(x, y, color);
                             pixel[1] = color.g;
                             pixel[2] = color.b;
                             pixel[3] = color.a;
                             x++;
                         }
                     }
                }
    );
    image->update();

otherwise try explicitly setting the type instead of auto:

    ofPixels & pixels = image->getPixels();

    parallel_for( tbb::blocked_range<size_t>(0,pixels.getHeight()),
                 [&](const tbb::blocked_range<size_t>& r) {
                     for(ofPixels::Line line: pixels.getLines(r.begin(), r.end() - r.begin())) {
                         auto y = line.getLineNum();
                         auto x = 0;
                         for (ofPixels::Pixel pixel: line.getPixels()) {
                             glm::vec3 P;
                             glm::vec3 w;

                             // Find the ray through (x, y) and the center of projection
                             camera.getPrimaryRay(float(x) + 0.5f, float(y) + 0.5f, width, height, P, w);
                             auto color = L_i(ofxRTRay(P, w));
                             //pixels.setColor(x, y, color);
                             pixel[1] = color.g;
                             pixel[2] = color.b;
                             pixel[3] = color.a;
                             x++;
                         }
                     }
                }
    );
    image->update();

also be careful to use a reference when initially calling image->getPixels, otherwise you are making a copy and the last image->update won’t really update the original