-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/603/
-----------------------------------------------------------

(Updated Sept. 20, 2012, 6:15 p.m.)


Review request for Viewer.


Changes
-------

Fixed the harmless (but unneeded) semicolon and indentation that used spaces.


Description
-------

In llvertexbuffer.cpp we call: delete mFence;

mFence is an instance of class LLGLSyncFence which is a sub-class of LLGLFence, 
which is defined in llgl.h.

However, class LLGLFence should have a virtual destructor because it's the base 
class. This patch fixes that potential memory leak, adding a virtual destructor 
to class LLGLFence. The virtual destructor ensures that the destructor for the 
derived class also gets called when we call "delete mfence;".

To quote Björn Pollex, "If you have a class that is supposed to be usable 
polymorphically, it should also be deletable polymorphically." 

(Unless I'm missing something...!)

NOTE: I notice that related code is commented in methods "void 
LLVertexBuffer::placeFence() const" and "void LLVertexBuffer::waitFence() 
const" -- maybe we commented it out because this memory leak was unresolved? 
Perhaps it can be uncommented now? I haven't tried yet. There was no note as to 
why the code is commented out.


Diffs (updated)
-----

  indra/llrender/llgl.h UNKNOWN 

Diff: http://codereview.secondlife.com/r/603/diff/


Testing
-------

I did compile this with OS X 10.8 as a build target successfully. I made other 
changes too, so while my FPS seems improved, it could be from any number of 
issues. I did notice that any llCharacters that are moving around don't get 
rendered properly by my build, but I don't know if it's because of this code 
revision or something else. I need to do further testing on that.


Thanks,

Gistya Eusebio

_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Reply via email to