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

section producing incorrect results #322

Closed
bacchanalia opened this issue Mar 4, 2019 · 5 comments
Closed

section producing incorrect results #322

bacchanalia opened this issue Mar 4, 2019 · 5 comments

Comments

@bacchanalia
Copy link


Both red sections have the same start parameters and should therefore start in the same place.

{-# LANGUAGE FlexibleContexts #-}
{-# LANGUAGE TypeFamilies #-}
import Diagrams.Prelude
import Diagrams.Backend.Cairo.CmdLine

ex :: Diagram Cairo
ex = (sec 0 (0.5) <> sec (0.5) 1     # lc red) # pad 1.1
 ||| (sec 0 (0.5) <> sec (0.5) (0.8) # lc red) # pad 1.1
  where
    sec a b = strokeP $ toPath (section (circle 1) a b :: Located (Trail V2 Double))

main = mainWith ex
@byorgey
Copy link
Member

byorgey commented Mar 5, 2019

One thing to note is that as you change the number 0.8 in the above code, the left endpoint of the red arc moves proportionally. I think section is simply broken. As discussed in IRC, my current thinking is that the problem is with the default implementation of section here:

section x t1 t2 = snd (splitAtParam (fst (splitAtParam x t2)) (t1/t2))

Transforming the parameter t1 to the parameter t1/t2 on the result of splitting at t2 assumes that parameters scale linearly. I think this may be true for individual segments, but it isn't true for trails, which give equal weight to each segment, i.e. assign parameter values in the range [i/n, (i+1)/n] to segment i if there are n total segments. If one segment gets split, this reassigns parameter values in a strange way.

@fryguybob
Copy link
Member

The comments in Diagrams.Parametric seem to be explicit about the linear assumption.

@bacchanalia
Copy link
Author

We have the problem that the Parametric instance of Tails depends on the number of segements, but a linear reparameterization for Sectionable can not depend on the number of segments without information about the original parameterization. One possible solution is drop the Sectionable instance for Trails and introduce a new Section type that caries that info with it.

@bacchanalia
Copy link
Author

Proposal: fix the current instances so that they adhere to the property that for a <= b <= c, section t a c = section t a b <> section t b c and note in the documentation that they do not linearly reparameterize the section. Add two new types to wrap multisegment objects, I'll give them the placeholder names ProperSection and ProperParam. ProperSection still is parametric by segment, but has a linear reparameterization, and ProperParam is parametric by arclength.

bacchanalia pushed a commit to bacchanalia/diagrams-lib that referenced this issue Mar 7, 2019
The section instance of SegTree seems to be correct, but the
instances for Located, Trail, and Trail' Line all used the
default definition which uses splitAtParam instead of SegTree's
implementation of section. The default definition of splitAtParam
is only correct for types which consist of a single segment or are
parametrized by arclength (rather than with each segment having
equal weight).
byorgey added a commit that referenced this issue Mar 7, 2019
fix for: section producing incorrect results #322
@byorgey
Copy link
Member

byorgey commented Mar 7, 2019

Closed by #328.

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