headers
Gerrit Voß | 20 Apr 2010 22:07

Re: RFC/PATCH: Create RenderTreeNodes from TreeBuilders


Hi,

On Tue, 2010-04-20 at 14:37 -0500, Carsten Neumann wrote:
> 	Hello all,
> 
> attached patch series changes the way we create RenderTreeNodes from 
> doing it inside RenderPartition::dropFunctor to TreeBuilderBase::add (or 
> derived).
> This seems a cleaner interface as the RPart now does not have to care 
> what the different TreeBuilder actually store in the RTNode. It also 
> allows moving occlusion culling specific data into OCRenderTreeNode that 
> is only used by the OcclusionCullingTreeBuilder, reducing the size of 
> the base RTNode.
> 
> Comments? [1]
> 

at least a minor one, could you undo these changes, at least to existing
code:

-StateSortTreeBuilder::StateSortTreeBuilder(void) :
-    _oSorter(),
-    _mFallbackSorter()
+StateSortTreeBuilder::StateSortTreeBuilder(void)
+    : _pRoot          (NULL)
+    , _oSorter        ()
+    , _mFallbackSorter()
 {
 }
(Continue reading)

Permalink | Reply |
headers
Carsten Neumann | 20 Apr 2010 23:02
Picon

Re: RFC/PATCH: Create RenderTreeNodes from TreeBuilders

	Hello Gerrit,

Gerrit Voß wrote:
> at least a minor one, could you undo these changes, at least to existing
> code:
> 
> -StateSortTreeBuilder::StateSortTreeBuilder(void) :
> -    _oSorter(),
> -    _mFallbackSorter()
> +StateSortTreeBuilder::StateSortTreeBuilder(void)
> +    : _pRoot          (NULL)
> +    , _oSorter        ()
> +    , _mFallbackSorter()
>  {
>  }
>  
> 99% of the code uses the first form so either change all or none, but
> please keep it consistent.

ok, fixed, sorry about that; there was also a file missing, so I've 
attached the updated patch series. Thanks.

	Cheers,
		Carsten
Attachment (render_tree_interface.tar.bz2): application/x-bzip, 29 KiB
------------------------------------------------------------------------------

(Continue reading)






Permalink
| Reply
| 



  


  

  








headers


Gerrit Voß
 | 
21 Apr 2010 09:13





Re: RFC/PATCH: Create RenderTreeNodes from TreeBuilders



Hi,

On Tue, 2010-04-20 at 16:02 -0500, Carsten Neumann wrote:
> 	Hello Gerrit,
> 
> Gerrit Voß wrote:
> > at least a minor one, could you undo these changes, at least to existing
> > code:
> > 
> > -StateSortTreeBuilder::StateSortTreeBuilder(void) :
> > -    _oSorter(),
> > -    _mFallbackSorter()
> > +StateSortTreeBuilder::StateSortTreeBuilder(void)
> > +    : _pRoot          (NULL)
> > +    , _oSorter        ()
> > +    , _mFallbackSorter()
> >  {
> >  }
> >  
> > 99% of the code uses the first form so either change all or none, but
> > please keep it consistent.
> 
> ok, fixed, sorry about that; there was also a file missing, so I've 
> attached the updated patch series. Thanks.
> 

0003-fixed-remove-some-dead-code.patch

you are aware that the two code paths are no identical ? The

(Continue reading)






Permalink
| Reply
| 



  


  

  








headers


Carsten Neumann
 | 
21 Apr 2010 16:29






Picon





Re: RFC/PATCH: Create RenderTreeNodes from	TreeBuilders


	Hello Gerrit,

Gerrit Voß wrote:
> 0003-fixed-remove-some-dead-code.patch
> 
> you are aware that the two code paths are no identical ? The
> _mfChunks.clear path might trigger memory operations and other nonsense
> in particular stl implementations (ms). 

yes, I just can't come up with a case were it would make a difference 
(for correctness at least). It seems a bit more consistent to set all 
entries to NULL, as that is what e.g. subChunk does.

> If we jumped the hoops to keep the memory alive by not using clear
> initially I expect there might have been a reason. I'll check.

hm, the jumping through hoops like code probably just comes from the 
conversion from 1.x. There std::transform() is used to subRef the chunks 
and set the mfield entries to NullFC. For 2 a simple loop assigning NULL 
should do it.
I'll change it to just set all entries to NULL (using a plain for loop), 
I'm not convinced this makes a big difference, but for consistency with 
the way the other functions work this is probably the best - and it 
still gets rid of most of the code ;)

	Cheers,
		Carsten

------------------------------------------------------------------------------
_______________________________________________

(Continue reading)






Permalink
| Reply
| 



  


  

  












  





    



Gmane