0

I am implementing a ray tracing program using VS2015 community. The program fills a screen size color buffer (width * height with element type of Eigen::Vector3f), then save the buffer content to a ppm file.

The basic loop (Note the output section at the end of the outer loop):

using Vec3f = Eigen::Vector3f;

Vec3f * buffer = new Vec3f[w * h];

// for each pixel
for (int y = 0; y < h; y++) for (int x = 0; x < w; x++) 
{
    int const i = (h - y - 1) * w + x;
    buffer[i] = Vec3f::Zero();

    // 2x2 subpixel 
    for (int sy = 0; sy < 2; sy++) for (int sx = 0; sx < 2; sx++)
    {
        Vec3f r = Vec3f::Zero();

        // sampling
        for (int s = 0; s < samps; s++) 
        {
            // do some computation and accumulation to r
            // r = ...
        }
        buffer[i] = buffer[i] + r;
    }
    buffer[i] = buffer[i] * 0.25f;

    // debug with an output section
    //if (x % 16 == 0 && y % 16 == 0)
    //  std::cout << buffer[i] << std::endl;

}

I can get proper result with configurations of:

  • Debug, x86 or x64;
  • Release, x86;
  • Release (optimization = \Od), x64.

However, the buffer is all zero vectors with Release (optimization = \O1 or \O2 or \Ox), x64, and what I got is a black picture.

So with Release (optimization = \O1 or \O2 or \Ox), x64, I uncommented the output section to check the values in the buffer. The strange thing is, each pixel that I checked has a correct value, those not checked remains zero vector. For example, if I check every 16 pixels like the upper code, I will get a picture like this (256*256 black tessellated every 16 pixel):

wrong answer with Release optimization=\O1 x64

I googled and read some materials like Surviving the Release Version, but still have no idea. Could anyone provide some experiences dealing with these problems?

Update: The code above is not so detailed, full code is here, depends on Eigen 3.2.6.

stanleyerror
  • 728
  • 1
  • 9
  • 23
  • Can you flesh out your example a bit more? Namely, your calculations for `r`. Without being able to reproduce your output, any and all answers you get will be guesswork. And even though you say you aren't looking for a solution to this problem, but rather general "experiences", questions of those sort are often considered off topic (too broad). But if a specific problem is shown, you may get answers with a more general insight. – Avi Ginsburg Jun 20 '16 at 07:11
  • @AviGinsburg The calculations for `r` is too complicated, so I append the full code's link at the end of the question. – stanleyerror Jun 20 '16 at 09:19
  • *"Too complicated"* is an excuse, that isn't very helpful. Reduce the calculation, until you have the minimal version that still exhibits the behavior. If stripping code changes the result, you know where to continue to investigate. At any rate, questions on SO should be self-contained. – IInspectable Jun 20 '16 at 09:23
  • What if you replace the calculation for `r` by a simple operation, say simple assignment? Will you be able to reproduce the problem with the replacement? – kangshiyin Jun 20 '16 at 10:31
  • @IInspectable OK i will try to reduce it as you suggested, thanks. – stanleyerror Jun 20 '16 at 10:34
  • @Eric I am sorry to say that I have not found the critical point simply enough to reproduce the problem. Some trivial operations behave well – stanleyerror Jun 20 '16 at 10:36
  • At least that means the problem in calculation for 'r'? So you are only showing the correct part of your code... – kangshiyin Jun 20 '16 at 10:42

1 Answers1

2

After going through the code on Ideone, the problem seems to be as follows. In the Scene class, the intersect method returns a bool const &. The returned reference is a local variable. If you examine the Error/Warning logs you'd have seen:

Warning 2 warning C4172: returning address of local variable or temporary ***.cpp 129 1

changing the return type to bool rectifies the problem and the output is similar to that from the 32 bit version.

Avi Ginsburg
  • 10,323
  • 3
  • 29
  • 56
  • This is exactly why an [mcve] is needed, as the source of the problem was not in the original post. – Avi Ginsburg Jun 20 '16 at 11:06
  • Good call. I'm at a loss as to why the OP opted to return a `const bool&` to begin with. Maybe they recently read [GotW #88: A Candidate For the “Most Important const”](https://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/), and then got it all backwards. – IInspectable Jun 20 '16 at 12:29
  • @AviGinsburg I made two mistakes: 1) did not check the warning infomation; 2) did not review the code carefully. Sorry for that. This error attributes to "Uninitialized Local Variables" from http://www.codeproject.com/Articles/548/Surviving-the-Release-Version – stanleyerror Jun 21 '16 at 01:24
  • I guess the real question is why did it seemingly work "correctly" in the 32 bit release version. With that, I can't really help you. – Avi Ginsburg Jun 21 '16 at 02:46
  • @AviGinsburg: It is undefined behavior. Reasoning about why undefined behavior exhibits certain behavior under certain conditions is not really interesting. It's undefined behavior, and appearing to succeed is a valid form of undefined behavior. – IInspectable Jun 21 '16 at 07:54
  • @IInspectable I realize that it's undefined behavior. My pondering was that *presumably* the implementation of cl.exe for both 64bit and 32bit is similar and while it is UB and therefore it is allowed to make me a cup of coffee when run, I would have naively assumed to receive the same cup of coffee from cl. – Avi Ginsburg Jun 21 '16 at 07:58
  • 1
    Parameter passing is radically different between x86 and x64 (stack vs. registers vs. registers with [shadow space](http://stackoverflow.com/q/30190132/1889329)). Different behavior is more likely than identical or similar behavior. If you really need to know, you'd have to look at the generated object code. – IInspectable Jun 21 '16 at 08:07