Implementing worklets that modify cell connectivity
Motivation
The TriangleWinding
worklet adjusts cell vertex windings to be consistent with cell normals. It is currently only implemented for vtkm::cont::CellSetExplicit
and vtkm::cont::CellSetSingleType
, and relies on the implementation details of these to directly modify the connectivity array (as an ArrayHandleGroupVecVariable
) in a WorkletMapField
.
This worklet cannot be used with VTK, as VTK uses its own CellSet
implementations to zero-copy vtkCellArray
into vtkm. Since vtkm does not know about VTK's CellSet
s, it cannot support currently these.
Ideally, it'd be preferable to have a worklet that can traverse a CellSet
and be able to modify the connectivity. This cannot be done with a WorkletMapTopology
, since that worklet exists to map topology elements onto each other and changing the topology during these operations leads to ambiguity.
There currently is no good way to modify a generic CellSet
's connectivity in VTK-m without relying on tightly coupled worklets accessing the implementation details of each supported CellSet
.
Desired Outcome
Ideally, we should add a WorkletMapCellSet
that operates on one or more CellSet
s and is able to modify the connectivity of each cell. Only the vertices in each cell may be modified, the cell shape and size will be immutable.
For the TriangleWindings
example, we could have the following:
class TriangleWindings : public WorkletMapCellSet
{
public:
using ControlSignature void(CellSetIn origCells,
CellFieldIn cellNormals,
WholeArrayIn coordinates,
CellSetInOut outputCells);
using ExecutionSignature void(CellPoints<1> origPoints,
_2 normals,
_3 coords,
CellPoints<4> outPoints);
template <typename OrigPoints,
typename Normal,
typename Coords,
typename OutPoints>
void operator()(OrigPoints orig, Normal norm, Coords coords, OutPoints out) const
{
// Look up points from orig in coords
// Compute and compare triangle winding to norm
// Write out, rewinding vert order if needed
}
};
Issues
When attempting to write WorkletMapCellSet
, there are some problems that prevent a clean implementation.
VecFromPortal
The biggest issue is how VecFromPortal
works. ConnectivityExplicit::GetIndices
currently returns a VecFromPortal
, and this has some serious issues with value/reference semantics in the context of being a vtkm::Vec
proxy:
- Copying by value produces an object that still behaves as a reference.
- Modifications to this proxy immediately modify the underlying
ArrayHandle
data, making the behaviors/implementations ofFetch<...>::Load
andFetch<...>::Store
unclear. - A
const VecFromPortal
may be written to usingoperator[]
and will modify theArrayHandle
. - Copying a
VecFromPortal
into anotherVecFromPortal
does not copy the components, but rather copies the reference information (both will point to the same portal/offset/size, rather than updating the target vector's components).
These will result in surprising behavior for users writing this worklet. For instance, taking the CellPoints
execution object by value and using it as a temporary local variable to compute an output will modify the input data.
The surprising and unconventional semantics of VecFromPortal
are most worrying to me when looking at this feature. It's unclear how to implement the Load
/Store
methods cleanly, and users will write code that would be correct with vtkm::Vec
s, but will instead silently and subtly produce incorrect results.
Existing CellSet Fetch classes
The existing control/execution signature tags for CellSet
s (written for WorkletMapTopology
) bypass the FetchTag
/AspectTag
design of the Fetch
mechanism. The ExecObject
passed into Store
and Load
is ignored, and all of the CellSet
access is handled through a custom ThreadIndices
subclass.
This means that the argument tags are always tied to the input domain, which prevents the ability to read from one CellSet
and write to another.
We'd either need separate implementations of these Fetch
classes, or refactor the existing ones (along with a chunk of WorkletMapTopology
).