Volumes Of Fun http://www.volumesoffun.com/phpBB3/ |
|
Upcoming LargeVolume changes http://www.volumesoffun.com/phpBB3/viewtopic.php?f=14&t=513 |
Page 1 of 4 |
Author: | David Williams [ Thu May 16, 2013 10:01 am ] |
Post subject: | Upcoming LargeVolume changes |
Hi all, The LargeVolume class has the potential to be the recommended volume class in PolyVox, but I'm sure many of you are aware that it has some issues. So far we have not made much use of it ourself because the SimpleVolume has been sufficient, but as work proceeds on Cubiquity we're going to need to load larger amounts of data. We have already made some improvements to the LargeVolume since the last release of PolyVox. In particular:
However, there are a couple of areas where more work is needed (at least before we can adopt it in Cubiquity). In particular:
So any thoughts on the above? Any other concerns about the LargeVolume or areas where it could be improved? |
Author: | Freakazo [ Thu May 16, 2013 2:35 pm ] |
Post subject: | Re: Upcoming LargeVolume changes |
In regards with the bounds checking for Large volume and maybe the other classes as well, how about having it be conditionally compiled based on a cmake setting or debug/release build? |
Author: | holocronweaver [ Fri May 17, 2013 12:34 am ] |
Post subject: | Re: Upcoming LargeVolume changes |
I second the idea of an abstract Paging class that the user can implement. This will make using LargeVolume easier and cleaner. I think 'Paging' sounds better than 'Pager', especially since the latter is associated with the antiquated wireless messaging device. |
Author: | David Williams [ Fri May 17, 2013 8:35 am ] |
Post subject: | Re: Upcoming LargeVolume changes |
Freakazo wrote: In regards with the bounds checking for Large volume and maybe the other classes as well, how about having it be conditionally compiled based on a cmake setting or debug/release build? Actually is kind of how it works, in that the getVoxel() function (which does not perform bounds checking) does still have an assert() present. So Bounds checking actually is performed if asserts are enabled, but not otherwise, and it will never throw an exception regardless. You can see it here: https://bitbucket.org/volumesoffun/poly ... elop#cl-82 But you've got me thinking, maybe getVoxel() should instead take a parameter to control whether bounds checks are supressed. Untested code: Code: template <typename VoxelType> VoxelType RawVolume<VoxelType>::getVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos, bool suppressBoundsChecks = false) const { if(!suppressBoundsChecks) { //Do bounds checks and throw an exception } //Get on with the rest of the stuff. } In practice 'suppressBoundsChecks' should be a template parameter so that checking it is free, but I can't write the syntax for that off the top of my head. We could then also have another overload: Code: template <typename VoxelType> VoxelType RawVolume<VoxelType>::getVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos, WrapMode wrapmode) const Which includes a wrap mode, and hence never needs bounds checking anyway (but the wrapping has some small overhead). this would replace getVoxelWithWrapping() which is in develop at the moment. Ok, I will play with this and see what I can come up with. holocronweaver wrote: I second the idea of an abstract Paging class that the user can implement. This will make using LargeVolume easier and cleaner. I think 'Paging' sounds better than 'Pager', especially since the latter is associated with the antiquated wireless messaging device. Actually I think a classname should always be a noun, but something like PagingManager would be possible. I was actually wondering whether 'page' is the right word at all... maybe 'Streamer' or 'OverflowManager' or something. Hmmm... probably not. I'll think about it but maybe Pager is best after all. |
Author: | David Williams [ Fri May 17, 2013 2:59 pm ] |
Post subject: | Re: Upcoming LargeVolume changes |
Ok, I'm making some changes in a branch called bounds-checks. For each volume class there are now two versions of the getVoxel() function: Code: VoxelType getVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos, BoundsCheck eBoundsCheck = BoundsChecks::Full) const; VoxelType getVoxel(int32_t uXPos, int32_t uYPos, int32_t uZPos, WrapMode eWrapMode, VoxelType tBorder = VoxelType()) const; So for example: Code: // Returns the voxel, throwing an exception if it is out of bounds voxel = volume.getVoxel(x, y, z); // Returns the voxel with no bounds check done. voxel = volume.getVoxel(x, y, z, BoundsChecks::None); // Returns the voxel or a border value. Never throws an exception. voxel = volume.getVoxel(x, y, z, WrapModes::Border); I said earlier the parameter to control bounds checking should be a template parameter. Actually I haven't done this, because C++ does not support default parameters for template functions (though C++11 fixes this). I don't want to force users to call getVoxel<BoundsChecks::Full>(x, y, z,) as I think that's confusing. The old getVoxelAt(...) functions are still in place and behave as before, but I expect they will be deprecated. I need to do more testing though and to port internal code to the new system for testing. Anyway, skipping these bounds tests does seem to help performance @Matt - I hope these function signatures don't cause any problems for SWIG bindings. I think they are probably OK? Actually that's another reason to avoid the template parameter solution - I expect that would have complicated SWIG. |
Author: | milliams [ Fri May 17, 2013 4:08 pm ] |
Post subject: | Re: Upcoming LargeVolume changes |
David Williams wrote: @Matt - I hope these function signatures don't cause any problems for SWIG bindings. I think they are probably OK? Actually that's another reason to avoid the template parameter solution - I expect that would have complicated SWIG. I think they should be fine for SWIG. It supports enums just fine as well as default parameters for functions. If it were to be a template parameter then I would just have to generate a wrapped function for each state (i.e. one with checking and one without). This would still be fine but the API would be a little larger. |
Author: | David Williams [ Sat May 18, 2013 10:00 pm ] |
Post subject: | Re: Upcoming LargeVolume changes |
I realise now that both options are probably possible - i.e. a non templatised getVoxel(..., BoundsCheck eBoundsCheck = BoundsChecks::Full) version and also a templatised getVoxel<BoundsCheck>(...) version. Still, it complicates things and there's no indication that it's needed at the moment, but it could probably be added at a later date without breaking backwards compatibility. Also, is the idea of a Pager base class going to be easier for SWIG to wrap than the std::functions? Do you think it will be possible to define a Pager implementation in Python/C#? If it's not possible to provide this then it should be fine to just wrap a version with the default FilePager being used. |
Author: | David Williams [ Mon Jun 17, 2013 9:34 am ] |
Post subject: | Re: Upcoming LargeVolume changes |
Hi all, I've just merged in some work which aims to reduce the amount of bounds checking which is performed when accessing a voxel.The getVoxelAt() function is replaced by getVoxel(), and this provides explicit control over when bounds checking is performed. Please see these docs for more information: The implementation of the templatised versions of the functions proved to be a bit complex due to this C++ limitation, but I worked around that with the method suggested in that thread and the implementation is hidden from the user. Although the framework is in place I haven't made proper use of the new functions inside PolyVox, so you won't really see a speed benefit yet. I'll update the PolyVox internals in time. I've also started replacing the std::functions in LargeVolume with a Pager class. This work is taking place in a seperate branch here: Paging branch |
Author: | David Williams [ Thu Jun 27, 2013 3:04 pm ] |
Post subject: | Re: Upcoming LargeVolume changes |
I just merged the work from the paging branch back into develop. Note that this work is not finished, but the tricky part is over and the tests now pass again. It is more convenient for me to work in develop now so I can properly test the changes within Cubiquity. If you are working with the LargeVolume you should probably hold off updating for a couple of weeks until it settles down and stablizes.
Anyway, it's work in progress so you probably don't want to use it yet, but it should come together soon. |
Author: | David Williams [ Thu Jun 27, 2013 7:52 pm ] |
Post subject: | Re: Upcoming LargeVolume changes |
A quick question for Matt - do you know if we have permission to write to the working directory on the build machine? I guess we do? I'd quite like to make the LargeVolume unit tests make use of the new FilePager, which will mean dumping some data to a disk and then reading it back again. A few megabytes I guess. Current working directory is the easiest place to do this, as C/C++ doesn't provide support for creating folders etc and I'd also like it to just work on Windows. I assume the build machine will delete the whole build folder after the tests and test in a clean folder each time? Maybe we just try it and see... |
Page 1 of 4 | All times are UTC |
Powered by phpBB © 2000, 2002, 2005, 2007 phpBB Group http://www.phpbb.com/ |