Smooth normal calculation Update! #941
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,33 +11,137 @@ namespace nbl::asset { | |
| class CVertexWelder { | ||
|
|
||
| public: | ||
| using WeldPredicateFn = std::function<bool(const ICPUPolygonGeometry* geom, uint32_t idx1, uint32_t idx2)>; | ||
|
There was a problem hiding this comment. at this point it can be a pure virtual interface class with pure virtual Because the init needs to check validity of the polygon geometry, e.g. your default predicate should fail the initialization if there are any attribute views which are not formatted (format is unknown) because it doesn't know what data in the buffer belongs to what vertex (because there's no clear mapping) See #921 (comment) |
||
|
|
||
| class DefaultWeldPredicate | ||
| { | ||
| private: | ||
| static bool isAttributeValEqual(const ICPUPolygonGeometry::SDataView& view, uint32_t index1, uint32_t index2, float epsilon) | ||
| { | ||
| if (!view) return true; | ||
| const auto channelCount = getFormatChannelCount(view.composed.format); | ||
| switch (view.composed.rangeFormat) | ||
| { | ||
| case IGeometryBase::EAABBFormat::U64: | ||
| case IGeometryBase::EAABBFormat::U32: | ||
| { | ||
| hlsl::uint64_t4 val1, val2; | ||
| view.decodeElement<hlsl::uint64_t4>(index1, val1); | ||
| view.decodeElement<hlsl::uint64_t4>(index2, val2); | ||
| for (auto channel_i = 0u; channel_i < channelCount; channel_i++) | ||
| if (val1[channel_i] != val2[channel_i]) return false; | ||
| break; | ||
| } | ||
| case IGeometryBase::EAABBFormat::S64: | ||
| case IGeometryBase::EAABBFormat::S32: | ||
| { | ||
| hlsl::int64_t4 val1, val2; | ||
| view.decodeElement<hlsl::int64_t4>(index1, val1); | ||
| view.decodeElement<hlsl::int64_t4>(index2, val2); | ||
| for (auto channel_i = 0u; channel_i < channelCount; channel_i++) | ||
| if (val1[channel_i] != val2[channel_i]) return false; | ||
| break; | ||
| } | ||
| default: | ||
| { | ||
| // TODO: Handle 16,32,64 bit float vectors once the pixel encode/decode functions get reimplemented in HLSL and decodeElement can actually benefit from that. | ||
| hlsl::float64_t4 val1, val2; | ||
| view.decodeElement<hlsl::float64_t4>(index1, val1); | ||
| view.decodeElement<hlsl::float64_t4>(index2, val2); | ||
| for (auto channel_i = 0u; channel_i < channelCount; channel_i++) | ||
| { | ||
| const auto diff = abs(val1[channel_i] - val2[channel_i]); | ||
| if (diff > epsilon) return false; | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| static bool isAttributeDirEqual(const ICPUPolygonGeometry::SDataView& view, uint32_t index1, uint32_t index2, float epsilon) | ||
| { | ||
| if (!view) return true; | ||
| const auto channelCount = getFormatChannelCount(view.composed.format); | ||
| switch (view.composed.rangeFormat) | ||
| { | ||
| case IGeometryBase::EAABBFormat::U64: | ||
| case IGeometryBase::EAABBFormat::U32: | ||
| { | ||
| hlsl::uint64_t4 val1, val2; | ||
| view.decodeElement<hlsl::uint64_t4>(index1, val1); | ||
| view.decodeElement<hlsl::uint64_t4>(index2, val2); | ||
| return (1.0 - hlsl::dot(val1, val2)) < epsilon; | ||
| } | ||
| case IGeometryBase::EAABBFormat::S64: | ||
| case IGeometryBase::EAABBFormat::S32: | ||
| { | ||
| hlsl::int64_t4 val1, val2; | ||
| view.decodeElement<hlsl::int64_t4>(index1, val1); | ||
| view.decodeElement<hlsl::int64_t4>(index2, val2); | ||
| return (1.0 - hlsl::dot(val1, val2)) < epsilon; | ||
| } | ||
|
Comment on lines
+28
to
+83
There was a problem hiding this comment. for formats which don't decode to floats you need to use There's a There was a problem hiding this comment. |
||
| default: | ||
| { | ||
| hlsl::float64_t4 val1, val2; | ||
| view.decodeElement<hlsl::float64_t4>(index1, val1); | ||
| view.decodeElement<hlsl::float64_t4>(index2, val2); | ||
| return (1.0 - hlsl::dot(val1, val2)) < epsilon; | ||
| } | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| float m_epsilon; | ||
|
|
||
| public: | ||
|
|
||
| DefaultWeldPredicate(float epsilon) : m_epsilon(epsilon) {} | ||
|
|
||
| bool operator()(const ICPUPolygonGeometry* polygon, uint32_t index1, uint32_t index2) | ||
| { | ||
| if (!isAttributeValEqual(polygon->getPositionView(), index1, index2, m_epsilon)) | ||
| return false; | ||
| if (!isAttributeDirEqual(polygon->getNormalView(), index1, index2, m_epsilon)) | ||
| return false; | ||
| for (const auto& jointWeightView : polygon->getJointWeightViews()) | ||
| { | ||
| if (!isAttributeValEqual(jointWeightView.indices, index1, index2, m_epsilon)) return false; | ||
| if (!isAttributeValEqual(jointWeightView.weights, index1, index2, m_epsilon)) return false; | ||
| } | ||
| for (const auto& auxAttributeView : polygon->getAuxAttributeViews()) | ||
| if (!isAttributeValEqual(auxAttributeView, index1, index2, m_epsilon)) return false; | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| }; | ||
|
|
||
| template <typename AccelStructureT> | ||
|
There was a problem hiding this comment. there should be some concept you require here |
||
| static core::smart_refctd_ptr<ICPUPolygonGeometry> weldVertices(const ICPUPolygonGeometry* polygon, const AccelStructureT& as, WeldPredicateFn shouldWeldFn) { | ||
| auto outPolygon = core::move_and_static_cast<ICPUPolygonGeometry>(polygon->clone(0u)); | ||
| outPolygon->setIndexing(IPolygonGeometryBase::TriangleList()); | ||
|
|
||
| core::vector<uint32_t> vertexIndexToAsIndex(as.getVertexCount()); | ||
|
|
||
| for (uint32_t vertexData_i = 0u; vertexData_i < as.getVertexCount(); vertexData_i++) | ||
| { | ||
| const auto& vertexData = as.vertices()[vertexData_i]; | ||
| vertexIndexToAsIndex[vertexData.index] = vertexData.index; | ||
|
There was a problem hiding this comment. but this is the same as initializing your There was a problem hiding this comment. it should be vertexIndexToAsIndex[vertexData.index] = vertexData_i. |
||
| } | ||
|
|
||
| static constexpr auto INVALID_INDEX = std::numeric_limits<uint32_t>::max(); | ||
| core::vector<uint32_t> remappedVertexIndexes(as.getVertexCount()); | ||
| std::fill(remappedVertexIndexes.begin(), remappedVertexIndexes.end(), INVALID_INDEX); | ||
|
|
||
| uint32_t maxRemappedIndex = 0; | ||
| // iterate by index, so that we always use the smallest index when multiple vertexes can be welded together | ||
| for (uint32_t index = 0; index < as.getVertexCount(); index++) | ||
| { | ||
| const auto asIndex = vertexIndexToAsIndex[index]; | ||
|
There was a problem hiding this comment. why have that whole There was a problem hiding this comment. also the There was a problem hiding this comment. this needs a different construction https://github.com/Devsh-Graphics-Programming/Nabla/pull/941/files?diff=unified&w=1#r2466545963
Now any vertex thats not in the acceleration structure itself or has no equivalent will be set Invalid in the when you later iterate over the index buffer to create a new one, if you encounter such an invalid remapping it means the vertex was never the in Acceleration Structure and therefore you can abort with an error |
||
| const auto& vertexData = as.vertices()[asIndex]; | ||
| auto& remappedVertexIndex = remappedVertexIndexes[index]; | ||
|
There was a problem hiding this comment. if you went through vertices in order from 0 to max vertex reference, then you could initialize to INVALID here, and not |
||
| as.forEachBroadphaseNeighborCandidates(vertexData, [&, polygon, index](const typename AccelStructureT::vertex_data_t& neighbor) { | ||
| const auto neighborRemappedIndex = remappedVertexIndexes[neighbor.index]; | ||
| if (shouldWeldFn(polygon, index, neighbor.index) && neighborRemappedIndex != INVALID_INDEX) { | ||
|
There was a problem hiding this comment. check the neighbour remap for being initialized first, remember |
||
| remappedVertexIndex = neighborRemappedIndex; | ||
|
There was a problem hiding this comment. btw because of funny cache and aliasing effects (tapping same array), you want to make the |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is cellSize now a integer and vertex count a
float!?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a default value for
vertexCountReservelike 8kThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.