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

Add WrappingBlueNoise #6

Merged
merged 2 commits into from
Jan 7, 2021
Merged

Add WrappingBlueNoise #6

merged 2 commits into from
Jan 7, 2021

Conversation

benfrankel
Copy link
Contributor

@benfrankel benfrankel commented Jan 4, 2021

WrappingBlueNoise is like BlueNoise, but it considers points on opposite edges of the box to be near each other. This allows the generated blue noise to tile with itself without forming clusters of points at the edges / corners where tiles meet.

I tried to avoid performance regressions for the non-wrapping blue noise case, keep the API simple, and avoid code duplication in that order. There's probably a better way to implement this, though.

@benfrankel
Copy link
Contributor Author

Right now WrappingBlueNoise seems about 2x slower than BlueNoise on my machine, which makes sense. I think it could be optimized by adding a 2-wide border of cells around the grid, then duplicating points that are near the border in insert_point so that the distance calculations in is_valid can be much faster.

Copy link
Owner

@arlyon arlyon left a comment

Choose a reason for hiding this comment

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

Finally got a chance to take a look, code looks great! I didn't realise that the CI wasn't being run on PRs, please rebase on master with the test and I'll merge once it passes.

Cheers

src/lib.rs Show resolved Hide resolved
@arlyon
Copy link
Owner

arlyon commented Jan 5, 2021

Right now WrappingBlueNoise seems about 2x slower than BlueNoise on my machine, which makes sense. I think it could be optimized by adding a 2-wide border of cells around the grid, then duplicating points that are near the border in insert_point so that the distance calculations in is_valid can be much faster.

I think for now the hit is okay. If you would like to create an issue just to keep track we can decide if it needs to be changed down the line.

@benfrankel
Copy link
Contributor Author

Updated the PR, and created an issue: #7.

@arlyon
Copy link
Owner

arlyon commented Jan 7, 2021

Good stuff, thanks for the patch!

@arlyon arlyon merged commit 1050242 into arlyon:master Jan 7, 2021
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.

2 participants