Skip to content

Commit

Permalink
Fixed memory leak in DrawingContext
Browse files Browse the repository at this point in the history
DrawingContext was using placement-new to create objects, but not
manually calling the destructors on them, thus leaking memory when the
object in question itself allocated memory, i.e. std::strings in
TextRequest.
  • Loading branch information
Grumbel committed Aug 12, 2014
1 parent e84ab6b commit 4bd64f6
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 21 deletions.
37 changes: 29 additions & 8 deletions src/video/drawing_context.cpp
Expand Up @@ -53,9 +53,26 @@ DrawingContext::DrawingContext(Renderer& renderer, Lightmap& lightmap) :

DrawingContext::~DrawingContext()
{
clear_drawing_requests(lightmap_requests);
clear_drawing_requests(drawing_requests);

obstack_free(&obst, NULL);
}

void
DrawingContext::clear_drawing_requests(DrawingRequests& requests)
{
for(auto& request : requests)
{
if (request->request_data)
{
request->request_data->~DrawingRequestData();
}
request->~DrawingRequest();
}
requests.clear();
}

void
DrawingContext::draw_surface(SurfacePtr surface, const Vector& position,
float angle, const Color& color, const Blend& blend,
Expand All @@ -81,7 +98,9 @@ DrawingContext::draw_surface(SurfacePtr surface, const Vector& position,
request->color = color;
request->blend = blend;

request->request_data = surface.get();
SurfaceRequest* surfacerequest = new(obst) SurfaceRequest();
surfacerequest->surface = surface.get();
request->request_data = surfacerequest;

requests->push_back(request);
}
Expand Down Expand Up @@ -236,7 +255,7 @@ DrawingContext::draw_filled_rect(const Rectf& rect, const Color& color, float ra
fillrectrequest->radius = radius;
request->request_data = fillrectrequest;

requests->push_back(request);
requests->push_back(request);
}

void
Expand All @@ -253,20 +272,20 @@ DrawingContext::draw_inverse_ellipse(const Vector& pos, const Vector& size, cons
request->alpha = transform.alpha;

InverseEllipseRequest* ellipse = new(obst)InverseEllipseRequest;

ellipse->color = color;
ellipse->color.alpha = color.alpha * transform.alpha;
ellipse->size = size;
request->request_data = ellipse;

requests->push_back(request);
requests->push_back(request);
}

Rectf
DrawingContext::get_cliprect() const
{
return Rectf(get_translation().x, get_translation().y,
get_translation().x + SCREEN_WIDTH,
get_translation().x + SCREEN_WIDTH,
get_translation().y + SCREEN_HEIGHT);
}

Expand Down Expand Up @@ -322,10 +341,12 @@ DrawingContext::do_drawing()
request->layer = LAYER_HUD - 1;
drawing_requests.push_back(request);
}
lightmap_requests.clear();

clear_drawing_requests(lightmap_requests);

handle_drawing_requests(drawing_requests);
drawing_requests.clear();
clear_drawing_requests(drawing_requests);

obstack_free(&obst, NULL);
obstack_init(&obst);

Expand Down Expand Up @@ -495,7 +516,7 @@ DrawingContext::set_ambient_color( Color new_color )
ambient_color = new_color;
}

void
void
DrawingContext::take_screenshot()
{
screenshot_requested = true;
Expand Down
4 changes: 3 additions & 1 deletion src/video/drawing_context.hpp
Expand Up @@ -141,7 +141,7 @@ class DrawingContext

Transform() :
translation(),
drawing_effect(NO_EFFECT),
drawing_effect(NO_EFFECT),
alpha(1.0f)
{ }

Expand All @@ -151,6 +151,8 @@ class DrawingContext
}
};

void clear_drawing_requests(DrawingRequests& requests);

private:
Renderer& renderer;
Lightmap& lightmap;
Expand Down
49 changes: 39 additions & 10 deletions src/video/drawing_request.hpp
Expand Up @@ -44,11 +44,11 @@ enum {
LAYER_OBJECTS = 50,
// Objects that pass through walls
LAYER_FLOATINGOBJECTS = 150,
//
//
LAYER_FOREGROUNDTILES = 200,
//
//
LAYER_FOREGROUND0 = 300,
//
//
LAYER_FOREGROUND1 = 400,
// Hitpoints, time, coins, etc.
LAYER_HUD = 500,
Expand Down Expand Up @@ -80,7 +80,26 @@ enum RequestType
SURFACE, SURFACE_PART, TEXT, GRADIENT, FILLRECT, INVERSEELLIPSE, DRAW_LIGHTMAP, GETLIGHT
};

struct SurfacePartRequest
struct DrawingRequestData
{
virtual ~DrawingRequestData()
{}
};

struct SurfaceRequest : public DrawingRequestData
{
SurfaceRequest() :
surface()
{}

const Surface* surface;

private:
SurfaceRequest(const SurfaceRequest&) = delete;
SurfaceRequest& operator=(const SurfaceRequest&) = delete;
};

struct SurfacePartRequest : public DrawingRequestData
{
SurfacePartRequest() :
surface(),
Expand All @@ -91,9 +110,13 @@ struct SurfacePartRequest
const Surface* surface;
Vector source;
Vector size;

private:
SurfacePartRequest(const SurfacePartRequest&) = delete;
SurfacePartRequest& operator=(const SurfacePartRequest&) = delete;
};

struct TextRequest
struct TextRequest : public DrawingRequestData
{
TextRequest() :
font(),
Expand All @@ -110,7 +133,7 @@ struct TextRequest
TextRequest& operator=(const TextRequest&);
};

struct GradientRequest
struct GradientRequest : public DrawingRequestData
{
GradientRequest() :
top(),
Expand All @@ -123,7 +146,7 @@ struct GradientRequest
Vector size;
};

struct FillRectRequest
struct FillRectRequest : public DrawingRequestData
{
FillRectRequest() :
color(),
Expand All @@ -136,7 +159,7 @@ struct FillRectRequest
float radius;
};

struct InverseEllipseRequest
struct InverseEllipseRequest : public DrawingRequestData
{
InverseEllipseRequest() :
color(),
Expand All @@ -160,7 +183,7 @@ struct DrawingRequest
float angle;
Color color;

void* request_data;
DrawingRequestData* request_data;

DrawingRequest() :
target(),
Expand All @@ -181,9 +204,15 @@ struct DrawingRequest
}
};

struct GetLightRequest
struct GetLightRequest : public DrawingRequestData
{
GetLightRequest() : color_ptr() {}

Color* color_ptr;

private:
GetLightRequest(const GetLightRequest&) = delete;
GetLightRequest& operator=(const GetLightRequest&) = delete;
};

#endif
Expand Down
2 changes: 1 addition & 1 deletion src/video/gl/gl_renderer.cpp
Expand Up @@ -111,7 +111,7 @@ GLRenderer::~GLRenderer()
void
GLRenderer::draw_surface(const DrawingRequest& request)
{
const Surface* surface = (const Surface*) request.request_data;
const Surface* surface = static_cast<const SurfaceRequest*>(request.request_data)->surface;
if(surface == NULL)
{
return;
Expand Down
2 changes: 1 addition & 1 deletion src/video/sdl/sdl_painter.cpp
Expand Up @@ -58,7 +58,7 @@ SDL_BlendMode blend2sdl(const Blend& blend)
void
SDLPainter::draw_surface(SDL_Renderer* renderer, const DrawingRequest& request)
{
const Surface* surface = (const Surface*) request.request_data;
const Surface* surface = static_cast<const SurfaceRequest*>(request.request_data)->surface;
boost::shared_ptr<SDLTexture> sdltexture = boost::dynamic_pointer_cast<SDLTexture>(surface->get_texture());

SDL_Rect dst_rect;
Expand Down

0 comments on commit 4bd64f6

Please sign in to comment.