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

[@thi.ng/geom-resample] Angular resolution resample unimplemented #345

Open
blainelewis1 opened this issue Apr 19, 2022 · 2 comments
Open

Comments

@blainelewis1
Copy link

I noticed there is a parameter to resample to angular resolution: https://github.com/thi-ng/umbrella/blob/develop/packages/geom-api/src/sample.ts

However, I am not getting the expected result when I use theta as a parameter:

resample([[0,0], [0,1],[1,1], [1,1], [1,1], [1,2]], { theta: Math.PI / 6 })

Expected:

[[0,0], [0,1],[1,1], [1,2]]

Received:

[[0,0],[0,0.15],[0,0.3],[0,0.44999999999999996],[0,0.6],[0,0.7499999999999999],[0,0.8999999999999999],[0.04999999999999982,1],[0.19999999999999996,1],[0.34999999999999987,1],[0.4999999999999998,1],[0.6499999999999999,1],[0.7999999999999998,1],[0.9500000000000002,1],[1,1.1],[1,1.2500000000000004],[1,1.4000000000000004],[1,1.5500000000000007],[1,1.7000000000000006],[1,1.850000000000001],[1,2]]

I believe this is just the default behaviour and the resample by angular resolution is unimplemented, see:

https://github.com/thi-ng/umbrella/blob/develop/packages/geom-resample/src/simplify.ts

and

https://github.com/thi-ng/umbrella/blob/develop/packages/geom-resample/src/resample.ts

Neither mention theta which I think means the feature was documented but not implemented.

@postspectacular
Copy link
Member

Hi @blainelewis1 - the theta option is only used for the sampling of circles, ellipses and arcs (because these're the only shapes where that option makes sense):

const circleOpts = (

let num = opts.theta ? Math.round(delta / opts.theta) : opts.num!;

The reason the option is listed in the SamplingOpts interface is due to the polymorphic nature of operations in the main thi.ng/geom package, e.g. the vertices() implementations (and many others). Maybe the theta option should be moved from the base interface into a new/extended version (can't think of a good name right now though, SamplingOptsXXX)

In any way, for now I should add a more explicit note to the theta docstring to point out this sampling option will only be used by some shapes...

@JeffreyPalmer
Copy link

I actually also just had this exact misunderstanding. I was trying to find a way of specifying that I wanted samples to be every d distance, but to have the sampler automatically increase resolution if the curvature of the path reached a certain threshold. I finally realized that this combination of constraints wasn't possible, but I did spend some time trying to figure it out.

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

No branches or pull requests

3 participants