Day 39 segfault in DrawBitmap

Hello, I've been coding along and am really enjoying the series, thank you Casey for all your hard work and for so gracefully sharing your knowledge.

On day 39 I noticed a bug in my code that caused a segfault when moving our Hero off the screen or back onto it on the Y axis, whilst writing the code to correct the display when the departing bitmap is no longer anchored on screen.

After some investigation in the visual studio debugger I noticed that the line:

int32 MaxY = RoundReal32ToInt32(RealY + (real32)Bitmap->Height);

Was causing the crash as the value of the addition of RealY and the cast to real of the Bitmap->Height were adding an additional line to the matrix for our Hero's display: Y was 163 not 162.

By replacing RealY with the previously calculated IntY, the problem was solved.

int32 MaxY = IntY + Bitmap->Height;

I have no idea why my machine generated this rounding result, and apparently not on other machines. The supplied source code does also show the same behaviour when I compile and run it locally; I would be fascinated to know why this is the case.

I thought it best to leave a note of this here, should anyone else experience the same thing; I could also be wrong about the risk of rounding errors, my machine is a very flawed intel machine, perhaps this is caused by my hardware not meeting the floating point specification too.

Thanks again.


Edited by iain on

How can you move hero offscreen? Day 37 keeps hero centered in screen all the time. And Day 39 has white borders around screen - so you cannot get out of screen.

Your hardware is fine, it is rounding correctly. HandmadeHero code from time to time had bugs - this is normal. They are either fixed in later days, or just happens that they are not triggered.

If your hardware would round incorrectly then you would see crashes left and right in all your applications or even Windows itself.

Are you sure this cast and addition is a problem? Because DrawBitmap does clamp to Bitmap->Height afterwards. So even if MaxY would be 1 larger than expected, it would still not exceed Bitmap->Height because of next comparsion:

    if(MaxY > Buffer->Height)
    {
        MaxY = Buffer->Height;
    }

@mmozeiko I think they are talking about the top of the sprite going out of the buffer area. But as you said, the values are clamped so it shouldn't happen.

@iain Could you share your code so we could check what the problem could be ? Have you tried to verify the values passed to the round function to see if the round does what you expect (as mmozeiko said it seems very unlikely that your processor has an issue) ?

Could you change the title of the thread to say day 39 instead of 37 ?


Edited by Simon Anciaux on

Hi, thank you for having a look at this.

The segfault happens during the process of correcting the screen transitions up and left on day 39, but the code that segfaults was written on day 37; Sorry if I was not clear on that, the code that faults was written on day 37, the bug became apparent on day 39.

I bring up my hardware because this model of Intel skylake does have issues so far as I am aware, just thought it best to mention; Hey I thought that windows and linux were both naturally really buggy! :D

When I run the downloaded source code of day 39, the program segfaults at line 150 in handmade.cpp, this happens on the last loop of the for loop that draws the bitmap and is a buffer underflow, I checked this in the memory window.

real32 A = (real32)((*Source >> 24) & 0xFF) / 255.0f;

I can reliably reproduce this segfault by compiling the downloaded source code of day 39. By setting dPlayerY to -10 in the debugger then stopping just before the blit and stepping through the DrawBitmap function.

I think that the problem is the value of RealY, entering the function it is suspiciously exactly 142.50000

I can happily continue on by making the calculation here in integers, this does stop the segfault, but as mentioned, I think that the value that has been rounded that is getting away, is the one coming into the function if this value were 142.999985 then there is no segfault.

	RealX -= (real32)AlignX;
	RealY -= (real32)AlignY;

	int32 MinX = RoundReal32ToInt32(RealX);
	int32 MinY = RoundReal32ToInt32(RealY);
	int32 MaxX = MinX + Bitmap->Width;
	int32 MaxY = MinY + Bitmap->Height;  

So likely my work around is a hack too; Fascinating bug really. Pretty sure the render that is to come solves this too, but a great way to explore the joys if floating point math.

Thanks,

iain


Edited by iain on
Replying to mmozeiko (#25475)

Hi there, Sure thing yes; I changed the name to be a little more explicit.

I can generate the bug from the source code, without having passed subject to my dubious typing and interpretation.

I think possibly the code leading up to the function call is rounding, I could run through the cycles just before and after it happens as it is only one value that causes it, to try to isolate where it is exactly that this happens.

I think the code leading up to the DrawBitmap function is rounding the PlayerGroundY up as it falls on a value just over 142.999985 to 142.5 and that this is adding an extra row of pixels between the players position and the actual size of the bitmap. So the code that is calculating the remaining bitmap to draw, is expecting the image to be one row longer than it is.

I could check the player pixel position one above and one below for more evidence of this, but am fairly convinced that this is the cause; Of course, I could well be wrong. Thanks for your thoughts on it.

Kind regards, Iain


Replying to mrmixer (#25476)

Ah, this looks promising ...

In the code above the call to DrawBitmap on line 649, if I use doubles in the multiplication then the value is not rounded up.

MetersToPixels*Diff.dY == 127.500000

(double)MetersToPixels*(double)Diff.dY == 127.49999753407064

Consequently when we get to line 103 in DrawBitmap, the use of the rounded value of RealY adds a row to the for loop that occurs a little later on. I think because it is a relative distance from AlignY which is itself anchored to the main window dimensions, making SourceOffsetY to small by 1; The rounded value becomes the row stride in the matrix spatial definition on line 136, which is now 1 row out of step with MaxY.

With this considered and looking at line 108, when a value is at exactly n.5, if we have MinY; Why would the addition of Bitmap->Height, also an integer, ever be wrong?

MinY + Bitmap->Height == 177
RealY + (real32)Bitmap->Height == 177.500000

Edited by iain on
Replying to iain (#25479)

To reproduce it:

  • Add a breakpoint on handmade.cpp on line 659 (drawing the torso);
  • Step in the function;
  • Change RealY to 142.5f.

What is happening is that MinY is negative and MaxY is positive and the round function rounds -39.5f to -40f and 177.5f to 178f causing the difference between the two to be more than the bitmap height by 1. So this is really a bug in handmade hero when the floating point decimal part is 0.5f and one of the value is positive and the other is negative.

I think that rounding behavior is expected (I think it's possible to change the rounding mode, but that would probably just move the issue somewhere else).

The fix you suggested using MaxY = MinY + Bitmap->Height is a good solution to the issue.


Edited by Simon Anciaux on

Thank you very much for taking a look, what nice clear way of reproducing the bug so that it is not so context dependent; Great pointer, I'm taking notes here.

I was so wrapped up in the code that it did not occur to me to check it that way :)


Replying to mrmixer (#25485)