Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduced Anisotropic Surfacing Kernels and PCA methods #1634

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Idclip
Copy link
Contributor

@Idclip Idclip commented Jul 4, 2023

New:
    - Introduced anisotropic surfacing tools in points/PointRasterizeSDF.h
    and principal component analysis (PCA) methods in points/PrincipalComponentAnalysis.h.
    The latter tool analyses point neighborhoods and computes affine
    transformations representing elliptical distributions. The prior tool
    takes rotational and stretch point attributes (optionally computed from
    the prior PCA methods) and generates anisotropic surfaces.

API Changes:
    - Re-worked the API in PointRasterizeSDF.h with the addition of anisotropic
    surfacing kernels. The new API exposes more concise free functions which
    take structs of parameters. The old API remains supported but is
    deprecated.

Improvements:
    - Improved the performance of PointRasterizeSDF.h tools with larger radii
    with better bound computation approximations.

Fixes:
    - Fixed a bug in points::rasterizeSpheres and points::rasterizeSmoothSpheres
    which could cause sections of the generated surface to incorrectly be marked
    as negative interior values.
    - Fixed a bug in points::rasterizeSmoothSpheres which could result in
    incomplete surface reconstruction with significantly larger search distances.
    - Fixed some precision issues in various Matrix methods

@Idclip Idclip force-pushed the feature/anisotropic_surfacing branch 5 times, most recently from 6435a72 to 138a872 Compare July 5, 2023 23:24
@Idclip Idclip force-pushed the feature/anisotropic_surfacing branch 4 times, most recently from 38380c4 to 5b26645 Compare September 26, 2023 07:30
Copy link
Contributor

@danrbailey danrbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned, please remove templated interrupter from all of these API methods.

/// @param interrupt An optional interrupter.
template <typename PointDataGridT,
typename FilterT = NullFilter,
typename InterrupterT = util::NullInterrupter>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if we don't add new methods with interrupter template arguments as it complicates explicit template instantiation. All the Houdini SOPs now use HoudiniInterrupter which sub-classes util::NullInterrupter, so fine to just provide util::NullInterrupter as an un-templated argument.

mWeightedPositions[id] += Psrc * weight; // @note: world space position is weighted
#else
// @warning much slower than accessing the buffers directly
mWeights->set(id, mWeights->get(id) + weight);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the setUnsafe() and getUnsafe() methods resolve the performance issues or is there something else you gain (simd?) from going directly to the buffers?

@Idclip Idclip force-pushed the feature/anisotropic_surfacing branch from 5b26645 to de71777 Compare October 24, 2023 22:33
… PointRasterize and other fixes

Signed-off-by: Nick Avramoussis <[email protected]>
…ys activate the correct topology if the search radius was larger than the point radius

Signed-off-by: Nick Avramoussis <[email protected]>
…r non-anisotropic neighbourhoods on the PCA methods

Signed-off-by: Nick Avramoussis <[email protected]>
…ps rasterizer wouldn't be found by point transfer

Signed-off-by: Nick Avramoussis <[email protected]>
@Idclip Idclip force-pushed the feature/anisotropic_surfacing branch from faf1b1c to 7f8b438 Compare January 5, 2024 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants