On Thu, 20 Sep 2012 07:08:11 -0000 "Gistya Eusebio" <gis...@gmail.com> wrote:
> > > > On Sept. 19, 2012, 2:12 p.m., Aleric Inglewood wrote: > > > There is semi-colon where it doesn't belong. Otherwise ok > > > (Singularity has this patch since a long time). > > Oh really, where? It compiled and ran fine on my machine. Perhaps it's the ONLY semi-colon you added? Semi-colons have the tendency to compile if you add one at random; still doesn't belong there though. With regard to white-space, you also should start the line with a TAB instead of spaces to match the already existing lines, and the added comment is something that should be in the commit message, not in the code. > > > - Gistya > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/603/#review1265 > ----------------------------------------------------------- > > > On Sept. 19, 2012, 8:26 a.m., Gistya Eusebio wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > http://codereview.secondlife.com/r/603/ > > ----------------------------------------------------------- > > > > (Updated Sept. 19, 2012, 8:26 a.m.) > > > > > > Review request for Viewer. > > > > > > 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 > > ----- > > > > 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 > > > > > -- Carlo Wood <ca...@alinoe.com> _______________________________________________ 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
