On Wednesday, July 27, 2011 03:30:06 PM Stuart Buchanan wrote:
> Hi All,
>
> I've got a small patch to improve the FG forests, along with some
> particularly bad C++ I need advice on.
>
> The patch does the following:
> 1) Fixes the longstanding bug where the first set of tree definitions
> in a tile were used for all forests within that tile. This meant that
> if you have an area of deciduous forest next to some evergreen, you'd
> only see one type of tree or the other, depending on which was loaded
> first.
> 2) Re-introduce a feature to fade in trees by distance, so that a
> reduced density is visible at 2*tree-range-m, with full density at
> tree-range-m. As the default tree-range-m is 8km, this help stop
> forests "springing up" well after the rest of the tile is perfectly
> visible.
>
> On my machine I don't see any performance impact, despite the fact
> that more trees are displayed. I'd appreciate it if those with more
> graphics-constrained systems than my own would test this and let me
> know if they think the frame-rate hit is acceptable given the
> improvements in apparent tree coverage.
>
> I'd also be interested to hear if tree range is a performance issue in
> general and people would like control of it in a similar way to the 3D
> clouds distance slider (if anyone uses that!).
>
> The patch is available from http://www.nanjika.co.uk/flightgear/forest.diff
>
> If I have time I'll do battle with git and see if I can submit it via
> gitorious properly...
>
> Now, for the bad C++ :)
>
> Within the patch is the code below. The (*j)-> just looks ugly. Surely
> there's a better way?
>
> I'm sure those of you who write C++ more regularly than me will
> immediately be able to tell me where I'm going wrong!
>
> -Stuart
>
> TreeBin* treebin;
> SGTreeBinList::iterator j;
> bool found = false;
>
> for (j = randomForest.begin(); (j != randomForest.end()) && (!found);
> j++) {
> if (((*j)->texture == mat->get_tree_texture() ) &&
> ((*j)->texture_varieties == mat->get_tree_varieties()) &&
> ((*j)->range == mat->get_tree_range() ) &&
> ((*j)->width == mat->get_tree_width() ) &&
> ((*j)->height == mat->get_tree_height() ) ) {
> treebin = *j;
> found = true;
> }
> }
It appears that SGTreeBinList is a list of pointers to some structure. If
that is the case then you need to dereference two pointers to get stuff from
the structure since the iterator, j, is a pointer. Thus you get *j->texture -
*j dereferences j and -> dereferences the pointer pointed to by j. I don't
think that you gain anything by writing this as (*j)->texture since this is
basically the same thing as *j->texture. IMO *j->texture is easier to read.
But there is one minor and very common issue with the code that should be
fixed. In the for loop
for (..; ..; j++)
should be
for (..; ..; ++j)
if you use j++ the compiler has to make a copy of j with each iteration of the
loop but if you use ++j it does not have to make a copy. This will make the
loop more efficient although only by a small amount.
Hal
------------------------------------------------------------------------------
Got Input? Slashdot Needs You.
Take our quick survey online. Come on, we don't ask for help often.
Plus, you'll get a chance to win $100 to spend on ThinkGeek.
http://p.sf.net/sfu/slashdot-survey
_______________________________________________
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
