Drawing sprite causes Segmentation Fault

Question

I'm writing a simple game in SFML2, which is based on isometric tiles. Each tile in the map is represented by Tile class. The class contains sf::Sprite object and draw() function which is supposed to draw the sprite on the screen. The problem is that invoking window.draw() causes Segmentation Fault. I've read that it's usually caused by associated sf::Texture pointer not being valid, but I've checked that and it's not the case.

(Texture reference is passed to the Tile constructor using ResourceManager<sf::Texture> object)

tile.hpp:

#pragma once

#include <SFML/Graphics.hpp>
#include "resource_manager.hpp"

class Tile
{
public:
  Tile(const sf::Texture& texture);

  void draw(sf::RenderWindow& window, const float dt);

private:
  sf::Sprite _sprite;
};

tile.cpp:

#include "tile.hpp"

Tile::Tile(const sf::Texture& texture)
{
  _sprite.setTexture(texture);
}

void Tile::draw(sf::RenderWindow& window, const float dt)
{
  std::cout << _sprite.getTexture()->getSize().x << std::endl; //Texture pointer works fine
  window.draw(_sprite); //Segmentation Fault here
}

map.hpp:

#pragma once

#include <vector>
#include <SFML/Graphics.hpp>
#include "resource_holder.hpp"
#include "tile.hpp"

class Map
{
public:
  Map(ResourceHolder& resources, int width, int height);

  void draw(sf::RenderWindow& window, const float dt);

private:
  ResourceHolder& _resources;
  int _width, _height;

  std::vector<Tile> _tiles;
};

map.cpp:

#include "map.hpp"
#include <iostream>

Map::Map(ResourceHolder& resources, int width, int height)
  : _resources(resources), _width(width), _height(height)
{
  _tiles.reserve(width * height);

  for(int y = 0; y < _height; ++y)
  {
    for(int x = 0; x < _width; ++x)
    {
      _tiles[x + y * _width] = Tile(_resources.textures["groundTile_NE"]);
    }
  }
}

void Map::draw(sf::RenderWindow& window, const float dt)
{
  for(int x = 0; x < _width; ++x)
  {
    for(int y = 0; y < _height; ++y)
    {
      _tiles[x + y * _width].draw(window, dt);
    }
 }
}

resource_manager.hpp:

#pragma once

#include <unordered_map>
#include <iostream>

template<typename Resource>
class ResourceManager
{
public:
  ResourceManager(const std::string& directory, const std::string& extension)
    : _directory("assets/" + directory + "/"), _extension("." + extension)
  {}

  Resource& operator[](const std::string& name)
  {
    return get(name);
  }

  Resource& get(const std::string& name)
  {
    if(!exists(name))
      load(name);

    return _resources.at(name);
  }

  bool exists(const std::string& name) const
  {
    return _resources.find(name) != _resources.end();
  }

  void load(const std::string& name)
  {
    Resource res;
    if(!res.loadFromFile(_directory + name + _extension))
    {
      res.loadFromFile(_directory + "_fail_" + _extension);
    }

    _resources.insert({name, res});
  }

private:
  const sf::String _directory;
  const sf::String _extension;

  std::unordered_map<std::string, Resource> _resources;
};

Answer 1

From the SFML source code (Graphics/Texture.cpp), the destructor causes the OpenGL texture buffer to be deleted:

Texture::~Texture()
{
    // Destroy the OpenGL texture
    if (m_texture)
    {
        TransientContextLock lock;

        GLuint texture = static_cast<GLuint>(m_texture);
        glCheck(glDeleteTextures(1, &texture));
    }
}

In your load method, you allocate the object res on the stack. This means that when the method returns, the destructor of Resource will be called on the res object. Your OpenGL handle is deleted, but still stored in the copy of res which you insert into _resources.

To work around this, store pointers of the resource classes instead:

std::unordered_map<std::string, Resource*> _resources;

...

void load(const std::string& name)
{
   Resource* res = new Resource();
   if (!res->loadFromFile(_directory + name + _extension))
   {
      res->loadFromFile(_directory + "_fail_" + _extension);
   }

   _resources.insert({name, res});
}

Resource* operator[](const std::string& name)
{
   return get(name);
}

Resource* get(const std::string& name)
{
   if (!exists(name))
      load(name); // or return nullptr

   return _resources.at(name);
}

But you also need to destroy these pointers when the ResourceManager object is destroyed:

~ResourceManager()
{
   for (auto& pair : _resources)
      delete pair.second;
}

Note: you may be tempted to store references in _resources. But see this link for why that wouldn't be possible.