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

Builder Pattern applied to the Builder struct, errors instead of panics #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/algorithm/bridson.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use {Builder, Vector, Float};
use {PoissonConfiguration, Vector, Float};
use algorithm::{Creator, Algorithm};
use utils::*;

Expand All @@ -22,7 +22,7 @@ impl<F, V> Creator<F, V> for Bridson
{
type Algo = Algo<F, V>;

fn create(poisson: &Builder<F, V>) -> Self::Algo {
fn create(poisson: &PoissonConfiguration<F, V>) -> Self::Algo {
Algo {
grid: Grid::new(poisson.radius, poisson.poisson_type),
active_samples: vec![],
Expand All @@ -46,7 +46,7 @@ impl<F, V> Algorithm<F, V> for Algo<F, V>
where F: Float,
V: Vector<F>
{
fn next<R>(&mut self, poisson: &mut Builder<F, V>, rng: &mut R) -> Option<V>
fn next<R>(&mut self, poisson: &mut PoissonConfiguration<F, V>, rng: &mut R) -> Option<V>
where R: Rng
{
while !self.active_samples.is_empty() {
Expand Down Expand Up @@ -78,7 +78,7 @@ impl<F, V> Algorithm<F, V> for Algo<F, V>
None
}

fn size_hint(&self, poisson: &Builder<F, V>) -> (usize, Option<usize>) {
fn size_hint(&self, poisson: &PoissonConfiguration<F, V>) -> (usize, Option<usize>) {
// Calculating upper bound should work because there is this many places left in the grid and no more can fit into it.
let upper = if self.grid.cells() > self.success {
self.grid.cells() - self.success
Expand Down Expand Up @@ -112,7 +112,7 @@ impl<F, V> Algorithm<F, V> for Algo<F, V>
}
}

fn stays_legal(&self, poisson: &Builder<F, V>, sample: V) -> bool {
fn stays_legal(&self, poisson: &PoissonConfiguration<F, V>, sample: V) -> bool {
let index = sample_to_index(&sample, self.grid.side());
is_disk_free(&self.grid, poisson, index, 0, sample.clone(), &self.outside)
}
Expand All @@ -122,7 +122,7 @@ impl<F, V> Algo<F, V>
where F: Float,
V: Vector<F>
{
fn insert_if_valid(&mut self, poisson: &mut Builder<F, V>, index: V, sample: V) -> bool {
fn insert_if_valid(&mut self, poisson: &mut PoissonConfiguration<F, V>, index: V, sample: V) -> bool {
if is_disk_free(&self.grid,
poisson,
index.clone(),
Expand Down
14 changes: 7 additions & 7 deletions src/algorithm/ebeida.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use {Builder, Vector, Float};
use {PoissonConfiguration, Vector, Float};
use algorithm::{Creator, Algorithm};
use utils::*;

Expand All @@ -19,7 +19,7 @@ impl<F, V> Creator<F, V> for Ebeida
{
type Algo = Algo<F, V>;

fn create(poisson: &Builder<F, V>) -> Self::Algo {
fn create(poisson: &PoissonConfiguration<F, V>) -> Self::Algo {
let dim = V::dim(None);
let grid = Grid::new(poisson.radius, poisson.poisson_type);
let mut indices = Vec::with_capacity(grid.cells() * dim);
Expand Down Expand Up @@ -70,7 +70,7 @@ impl<F, V> Algorithm<F, V> for Algo<F, V>
where F: Float,
V: Vector<F>
{
fn next<R>(&mut self, poisson: &mut Builder<F, V>, rng: &mut R) -> Option<V>
fn next<R>(&mut self, poisson: &mut PoissonConfiguration<F, V>, rng: &mut R) -> Option<V>
where R: Rng
{
if self.indices.is_empty() {
Expand Down Expand Up @@ -136,7 +136,7 @@ impl<F, V> Algorithm<F, V> for Algo<F, V>
}
}

fn size_hint(&self, poisson: &Builder<F, V>) -> (usize, Option<usize>) {
fn size_hint(&self, poisson: &PoissonConfiguration<F, V>) -> (usize, Option<usize>) {
// Calculating lower bound should work because we calculate how much volume is left to be filled at worst case and
// how much sphere can fill it at best case and just figure out how many fills are still needed.
let dim = V::dim(None);
Expand Down Expand Up @@ -167,7 +167,7 @@ impl<F, V> Algorithm<F, V> for Algo<F, V>
}
}

fn stays_legal(&self, poisson: &Builder<F, V>, sample: V) -> bool {
fn stays_legal(&self, poisson: &PoissonConfiguration<F, V>, sample: V) -> bool {
let index = sample_to_index(&sample, self.grid.side());
is_disk_free(&self.grid, poisson, index, 0, sample.clone(), &self.outside)
}
Expand All @@ -177,7 +177,7 @@ impl<F, V> Algo<F, V>
where F: Float,
V: Vector<F>
{
fn subdivide(&mut self, poisson: &Builder<F, V>) {
fn subdivide(&mut self, poisson: &PoissonConfiguration<F, V>) {
let choices = &[0, 1];
let (grid, outside, level) = (&self.grid, &self.outside, self.level);
self.indices.flat_map_inplace(|i| {
Expand All @@ -189,7 +189,7 @@ impl<F, V> Algo<F, V>
}

fn covered<F, V>(grid: &Grid<F, V>,
poisson: &Builder<F, V>,
poisson: &PoissonConfiguration<F, V>,
outside: &[V],
index: V,
level: usize)
Expand Down
16 changes: 8 additions & 8 deletions src/algorithm/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Module that contains traits that describe poisson-disk distribution generating algorithms.

use {Builder, Vector, Float};
use {PoissonConfiguration, Vector, Float};

use rand::Rng;

Expand All @@ -20,23 +20,23 @@ pub trait Creator<F, V>: Copy + Debug
type Algo: Algorithm<F, V>;

/// Creates new algorithm.
fn create(&Builder<F, V>) -> Self::Algo;
fn create(&PoissonConfiguration<F, V>) -> Self::Algo;
}

/// Trait that describes what poisson-disk distribution generating algorithm needs.
pub trait Algorithm<F, V>
where F: Float,
V: Vector<F>,
{
/// Advances algorithm based on Builder and Rng.
fn next<R>(&mut self, &mut Builder<F, V>, &mut R) -> Option<V> where R: Rng;
/// Advances algorithm based on PoissonConfiguration and Rng.
fn next<R>(&mut self, &mut PoissonConfiguration<F, V>, &mut R) -> Option<V> where R: Rng;

/// Return lower and upper bound of samples remaining for algorithm to generate based on Builder.
fn size_hint(&self, &Builder<F, V>) -> (usize, Option<usize>);
/// Return lower and upper bound of samples remaining for algorithm to generate based on PoissonConfiguration.
fn size_hint(&self, &PoissonConfiguration<F, V>) -> (usize, Option<usize>);

/// Restricts the algorithm with arbitary sample.
fn restrict(&mut self, V);

/// Checks if sample is valid based on Builder.
fn stays_legal(&self, &Builder<F, V>, V) -> bool;
/// Checks if sample is valid based on PoissonConfiguration.
fn stays_legal(&self, &PoissonConfiguration<F, V>, V) -> bool;
}
148 changes: 112 additions & 36 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
//!
//! fn main() {
//! let poisson =
//! Builder::<_, Vec2>::with_radius(0.1, Type::Normal)
//! .build(rand::weak_rng(), algorithm::Ebeida);
//! Builder::<_, Vec2>::new().with_poisson_type(Type::Normal).with_radius(0.1)
//! .build(rand::weak_rng(), algorithm::Ebeida).unwrap();
//! let samples = poisson.generate();
//! println!("{:?}", samples);
//! }
Expand Down Expand Up @@ -92,9 +92,21 @@ impl Default for Type {
}
}

/// Builder for the generator.
#[derive(Default, Clone, Debug, PartialEq)]
pub struct Builder<F, V>
/// This is the error type for the `Builder::build` function.
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub enum ConfigurationError {
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm... When I was first writing the code I didn't go with custom error as all the errors were caused by missuse of the API and thus programming errors which means that there isn't anything meaningful to do if code triggers them.
Maybe if one calculates radius via other code before setting it via builder then it could have use?
So maybe custom error is better anyways.

Copy link
Owner

Choose a reason for hiding this comment

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

I also don't really like having *Unspecified errors as they are coding errors and there is nothing to do about them run time.
And *MultiplyDefined errors seem odd too as setting value twice isn't nessacary error and can just restrict the APIs usage.
If the errors were encode type system ala session types I would feel better about them. But that would feel bit too heavy for this.

Have to think about this.

RadiusInvalid,
RadiusUnspecified,
RadiusMultiplyDefined,
PoissonTypeUnspecified,
PoissonTypeMultiplyDefined,
SampleCountInvalid,
SampleApproximationUnsupported,
PoissonTypeUnknownAtRadiusCalculation,
}

#[derive(Clone, Debug, PartialEq)]
pub struct PoissonConfiguration<F, V>
Copy link
Owner

Choose a reason for hiding this comment

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

Having configuration object way better than what was before. I felt weird having all kinds of methods taking builder as parameter.
I would change the name to just Configuration as I have done with Generator, Algorithm and others.
The idea is that from outside perspective we have poisson::Configuration instead poisson::PoissonConfiguration.
If the user has conflict with their own type/trait or with another librarys they can always do:
use poisson::Configuration as PoissonConfiguration;.

where F: Float,
V: Vector<F>
{
Expand All @@ -103,64 +115,128 @@ pub struct Builder<F, V>
_marker: PhantomData<V>,
}

/// Builder for the Generator.
#[derive(Default, Clone, Debug, PartialEq)]
pub struct Builder<F, V>
where F: Float,
V: Vector<F>
{
radius: Option<F>,
poisson_type: Option<Type>,
error: Option<ConfigurationError>,
_marker: PhantomData<V>,
}

impl<V, F> Builder<F, V>
where F: Float,
V: Vector<F>
{
/// New Builder with type of distribution and radius specified.
/// The radius should be ]0, √2 / 2]
pub fn with_radius(radius: F, poisson_type: Type) -> Self {
assert!(F::cast(0) < radius);
assert!(radius <=
NumCast::from(2f64.sqrt() / 2.).expect("Casting constant should always work."));
/// Creates a new Builder with all options unset
pub fn new() -> Self {
Builder {
radius: radius,
poisson_type: poisson_type,
radius: None,
poisson_type: None,
error: None,
_marker: PhantomData,
}
}

/// New Builder with type of distribution and relative radius specified.
/// The relative radius should be ]0, 1]
pub fn with_relative_radius(relative: F, poisson_type: Type) -> Self {
assert!(relative >= F::cast(0));
assert!(relative <= F::cast(1));
Builder {
radius: relative *
NumCast::from(2f64.sqrt() / 2.).expect("Casting constant should always work."),
poisson_type: poisson_type,
_marker: PhantomData,
/// Confirms that the radius has not been set before.
/// Sets the error accordingly otherwise.
fn set_radius(&mut self, radius: F) {
if self.radius.is_some() {
self.error = Some(ConfigurationError::RadiusMultiplyDefined);
} else {
self.radius = Some(radius);
}
}

/// Confirms that the Poisson type has not been set before.
/// Sets the error accordingly otherwise.
fn set_poisson_type(&mut self, pt: Type) {
if self.poisson_type.is_some() {
self.error = Some(ConfigurationError::PoissonTypeMultiplyDefined);
} else {
self.poisson_type = Some(pt);
}
}

/// Configures the Builder to a specific radius.
/// The radius must be ]0, √2 / 2]
pub fn with_radius(mut self, radius: F) -> Self {
if F::cast(0) >= radius || radius > NumCast::from(2f64.sqrt() / 2.).expect("Casting constant should always work.") {
self.error = Some(ConfigurationError::RadiusInvalid);
}
self.set_radius(radius);
self
}

/// New Builder with type of distribution, approximate amount of samples and relative radius specified.
/// Configures the Builder to use the specified relative radius.
/// The relative radius must be ]0, 1]
pub fn with_relative_radius(mut self, relative: F) -> Self {
if relative < F::cast(0) || relative > F::cast(1) {
self.error = Some(ConfigurationError::RadiusInvalid);
}
self.set_radius(relative * NumCast::from(2f64.sqrt() / 2.).expect("Casting constant should always work."));
self
}

/// Configure the Builder with an approximate amount of samples and a relative radius.
/// The amount of samples should be larger than 0.
/// The relative radius should be [0, 1].
/// For non-perioditic this is supported only for 2, 3 and 4 dimensional generation.
pub fn with_samples(samples: usize, relative: F, poisson_type: Type) -> Self {
Builder {
radius: calc_radius::<F, V>(samples, relative, poisson_type),
poisson_type: poisson_type,
_marker: PhantomData,
/// Call this function only after setting the Poisson type.
pub fn with_samples(mut self, samples: usize, relative: F) -> Self {
match self.poisson_type {
Some(poisson_type) => {
match calc_radius::<F, V>(samples, relative, poisson_type) {
Ok(radius) => self.set_radius(radius),
Err(err) => self.error = Some(err),
}
}
None => {
self.error = Some(ConfigurationError::PoissonTypeUnknownAtRadiusCalculation);
}
}
self
}

/// Configure the Builder to use the specified Poisson type.
pub fn with_poisson_type(mut self, poisson_type: Type) -> Self {
self.set_poisson_type(poisson_type);
self
}

/// Returns the radius of the generator.
pub fn radius(&self) -> F {
pub fn radius(&self) -> Option<F> {
self.radius
}

/// Returns the type of the generator.
pub fn poisson_type(&self) -> Type {
pub fn poisson_type(&self) -> Option<Type> {
self.poisson_type
}

/// Returns the error about the configuration, if any.
pub fn error(&self) -> Result<(),ConfigurationError> {
match self.error {
Some(err) => Err(err),
None => Ok(()),
}
}

/// Builds generator with random number generator and algorithm specified.
pub fn build<R, A>(self, rng: R, _algo: A) -> Generator<F, V, R, A>
pub fn build<R, A>(self, rng: R, _algo: A) -> Result<Generator<F, V, R, A>,ConfigurationError>
where R: Rng,
A: Creator<F, V>
{
Generator::new(self, rng)
try!(self.error());
let config = PoissonConfiguration {
radius: try!(self.radius.ok_or(ConfigurationError::RadiusUnspecified)),
poisson_type: try!(self.poisson_type.ok_or(ConfigurationError::PoissonTypeUnspecified)),
_marker: PhantomData,
};
Ok(Generator::new(config, rng))
}
}

Expand All @@ -172,7 +248,7 @@ pub struct Generator<F, V, R, A>
R: Rng,
A: Creator<F, V>
{
poisson: Builder<F, V>,
poisson: PoissonConfiguration<F, V>,
rng: R,
_algo: PhantomData<A>,
}
Expand All @@ -183,7 +259,7 @@ impl<F, V, R, A> Generator<F, V, R, A>
R: Rng,
A: Creator<F, V>
{
fn new(poisson: Builder<F, V>, rng: R) -> Self {
fn new(poisson: PoissonConfiguration<F, V>, rng: R) -> Self {
Generator {
rng: rng,
poisson: poisson,
Expand Down Expand Up @@ -248,7 +324,7 @@ pub struct PoissonIter<F, V, R, A>
R: Rng,
A: Algorithm<F, V>
{
poisson: Builder<F, V>,
poisson: PoissonConfiguration<F, V>,
rng: R,
algo: A,
}
Expand Down
Loading