Fix #2899 Problems switching barb or flow renderer between 2D and 3D variables

[StasJ]

Fix #2899

[clyne]

@StasJ this fixes the problem, but it looks like a hack to work around another, larger problem. Namely, the vector version of the overloaded Box::GetExtents() method will return a two element vector if Box::IsPlanar() is true. PRegionSelector1D::sliderValueChanged calls the std::vector version of GetExtents(), modifies the two-element vector returned (when IsPlanar() is true), and then calls Box::SetExtents() with a modified two-element vector, erasing the memory of the 3-element extents that the Box previously contained. I think a more robust fix would be to change PRegionSelector1D::sliderValueChanged() to use the CoordType version of Box::{Get,Set}Extents(), which always returns/sets a 3-element array. We've been trying to move away from std::vector, with variable size, for the representation of coordinates and use the fixed size CoordType (or DimsType) instead. Thoughts?

[sgpearse]

I'll approve this fix. I get what John is saying in his review.

John's suggestion isn't major but I would have done the same thing - use old Box methods that rely on std::vector. I wasn't aware of CoordType until yesterday and I think this problem is going to hurt us again unless we can fully adopt CoordType and DimsType instead of vectors.

[StasJ]

@StasJ this fixes the problem, but it looks like a hack to work around another, larger problem. Namely, the vector version of the overloaded Box::GetExtents() method will return a two element vector if Box::IsPlanar() is true. PRegionSelector1D::sliderValueChanged calls the std::vector version of GetExtents(), modifies the two-element vector returned (when IsPlanar() is true), and then calls Box::SetExtents() with a modified two-element vector, erasing the memory of the 3-element extents that the Box previously contained. I think a more robust fix would be to change PRegionSelector1D::sliderValueChanged() to use the CoordType version of Box::{Get,Set}Extents(), which always returns/sets a 3-element array. We've been trying to move away from std::vector, with variable size, for the representation of coordinates and use the fixed size CoordType (or DimsType) instead. Thoughts?

Makes sense. The CoordType version of Box::SetExtents() was added only yesterday so I didn't see it in my working branch. I changed PRegionSelector1D::sliderValueChanged() to use CoordType.

[clyne]

@StasJ can we now back out the changes made to RenderParams::SetDefaultVariables()? That was the hope. Thanks.

[StasJ]

@StasJ can we now back out the changes made to RenderParams::SetDefaultVariables()? That was the hope. Thanks.

I think it makes sense to keep that code as a failsafe since it only ensures when switching from 2D to 3D that the renderer will have non-zero Z extents. Although redundant in this specific bug report, it could end up being useful in other cases.

[clyne]

As implemented might it not result in an un-intensional/unnecessary reset of the Z extents for datasets such as the NOAA UGRID/geoflow data, which have Z extents from 6378000 to 6395000 (assuming FLT_EPSILON is on the order of 1e-5 as documented)?

[StasJ]

@clyne I backed out the changes made to RenderParams::SetDefaultVariables