Hacker News Reader

Comment by cabaalis | Hacker News Reader

I'm way out of practice with C++. (It's been since like 2006.) Framebuffer clear function appears to allocate a new vector.

Does std::vector automatically handle the memory releases? This is called every frame, so I wanted to check here to see if I'm crazy or not.

View on HN

> Does std::vector automatically handle the memory releases?

Yes. What is going on here is that "img" is being assigned. The "old vector" will have its destructor called, which will call the destructor of all of its elements by default.

-------

I assume you're talking about this code: https://github.com/ssloy/tinyraycaster/blob/master/framebuff...

    void FrameBuffer::clear(const uint32_t color) {
        img = std::vector<uint32_t>(w*h, color);
    }

Which seems inefficient to me. But it is correct code, even if it is inefficient. There's no memory leak here, but the code probably would be way more efficient if it were instead a simple memset.

    memset(&img[0], color, w*h*sizeof(uint32_t));

This memset is likely easier to understand, since it doesn't rely upon destructors / constructors / assignment C++ Magic. And its more efficient to boot. Vectors are guaranteed to be contiguous in memory, which is why it is compatible with memset.

EDIT: Hmmm... the img is never initialized. You'd also have to do "img = std::vector<uint32_t>(w*h, color);" somewhere.

----------

The rectangle is also inefficient.

    for (size_t i=0; i<rect_w; i++) {
        for (size_t j=0; j<rect_h; j++) {

If i and j were reversed, the draw rectangle function would be more efficient. As it is, "j" iterates in the wrong direction with regards to cache lines...

    // This would be faster!
    for (size_t j=0; j<rect_h; j++) {
        for (size_t i=0; i<rect_w; i++) {

Overall though, efficiency isn't a goal at all. Its designend to be a quickie project to get things done as soon as possible. Thinking about all of these details slows down development, so there's something to be said about "just getting it done"

Instead of doing a memset, I'd recommend using std::fill (which will likely call memset or another fast implementation behind the scenes): it makes it clear what you are doing, and preempts questions of "is this legal to do" (which it is, but only for POD data types).

I'm far more inclined toward

  img.assign(w*h, color);

myself.

Normally I choose the methods in <algorithm> because they take an ExecutionPolicy parameter, allowing me to make this code parallel if I wish, but both are valid.

Note for readers looking to hack on a raycaster: If you were to clear the first and second half of the buffer to different colors, you've got a ceiling and floor.

>As it is, "j" iterates in the wrong direction with regards to cache lines

I always wondered 'what if' someone released Doom like FPS game in 1993 that required you to flip monitor on its side ;-). Both Ray-casting and Mode-X work best in vertical plane, while video memory access is only efficient when done linearly. Some quick back of a napkin calculations lead me to believe you could deliver 400x320 at the speed of Dooms 320x200.

Yes std::vector owns the underlying continuous array of memory and will free it on the destructor call of vector.

" img = std::vector<uint32_t>(w*h, color); "

What is more interesting if you have been away from c++ is that the assignment is not a copy! The vector in this case is a temporary and it dies on assignment. Thus img takes the guts of this new vector.

Movement semantics actually existed for vector assignment since the original C++98 STL specification.

Movement semantics didn't exist in the language per se, but they were supported back then by std::swap and std::auto_ptr. Today, it is far easier to represent movement semantics with RValues, std::move, and std::unique_ptr.

But yeah, std::vector's assignment operator was originally at least std::swap based and therefore efficient.

Reply on news.ycombinator.com