When doing velocity extension using FastSweeping, e.g. via openvdb::tools::sdfToExt, the velocity extension grid will use an identity transform, instead of the transform of the SDF grid that is used to sample the ISO-surface. This also means the function passed in to return the velocities on the ISO-surface, is not receiving world-coordinates, but (fractional) index-space coordinates.
This behavior may be intential but is not documented (and hence confusing). Additionally, inside the operator() of the various FastSweeping specializations (e.g. FastSweeping::InitExt), calls similar to this are consistently used:
sumHelper(sum2, ExtValueT(op(xform.indexToWorld(xyz))), d < minD, d2);
The xform here is the mExtGrid xform, which is initialized as follows:
mExtGrid = createGrid<ExtGridT>( background );
As far as I can see, its transform is never modified, which means xform will always be the identity transform, and calling indexToWorld(xyz) just returns xyz.
So I guess that either a) FastSweeping should use the same transform as the provided ISO-surface SDF, which means mExtGrid is not created/initialized correctly or b) FastSweeping should use fractional index-space coordinates only, in which case the calls to indexToWorld in the implementation is unnecessary.
To Reproduce
Create an SDF that represents an arbitrary shape (e.g. sphere) with voxel size 2.0, then use it with sdfToGrid. The ExtOpT op provided function will be called with xyz fractional index-space coordinates, ie: as if the input SDF had voxel size 1.0
Expected behavior
I would expect FastSweeping to use world-space coordinates, which I can directly plug into any velocity function that takes world-coordinates for query locations. The returned extension grid would have the same transform as the input ISO-surface SDF grid.
Additional context
In case the choice to use fractional index-space coordinates for FastSweeping is intentional, the API documentation should probably mention this, and the indexToWorld calls in the various operator() could be removed.
Environment
Linux Ubuntu 22.04 OpenVDB 10.0.1
Describe the bug
When doing velocity extension using FastSweeping, e.g. via
openvdb::tools::sdfToExt, the velocity extension grid will use an identity transform, instead of the transform of the SDF grid that is used to sample the ISO-surface. This also means the function passed in to return the velocities on the ISO-surface, is not receiving world-coordinates, but (fractional) index-space coordinates.This behavior may be intential but is not documented (and hence confusing). Additionally, inside the
operator()of the various FastSweeping specializations (e.g.FastSweeping::InitExt), calls similar to this are consistently used:sumHelper(sum2, ExtValueT(op(xform.indexToWorld(xyz))), d < minD, d2);The
xformhere is themExtGridxform, which is initialized as follows:mExtGrid = createGrid<ExtGridT>( background );As far as I can see, its transform is never modified, which means
xformwill always be the identity transform, and callingindexToWorld(xyz)just returnsxyz.So I guess that either a) FastSweeping should use the same transform as the provided ISO-surface SDF, which means mExtGrid is not created/initialized correctly or b) FastSweeping should use fractional index-space coordinates only, in which case the calls to
indexToWorldin the implementation is unnecessary.To Reproduce
Create an SDF that represents an arbitrary shape (e.g. sphere) with voxel size 2.0, then use it with
sdfToGrid. TheExtOpT opprovided function will be called with xyz fractional index-space coordinates, ie: as if the input SDF had voxel size 1.0Expected behavior
I would expect FastSweeping to use world-space coordinates, which I can directly plug into any velocity function that takes world-coordinates for query locations. The returned extension grid would have the same transform as the input ISO-surface SDF grid.
Additional context
In case the choice to use fractional index-space coordinates for FastSweeping is intentional, the API documentation should probably mention this, and the
indexToWorldcalls in the variousoperator()could be removed.