compas icon indicating copy to clipboard operation
compas copied to clipboard

Request: Vector.from_line, Line.point_from_start, Cylinder.from_line_radius

Open yck011522 opened this issue 4 years ago • 5 comments

Feature Request

As a [user], I want [the following functions] so that [others and I do not have to reimplement it].

Details

Is your feature request related to a problem? Please describe. I find myself often struggling with the limited set of geometry constructors. I know it is just two lines of code to do these functions, but implementing these in the core code made my 'user' code more readable. These three are what I have had in mind over the past few days.

Describe the solution you'd like A clear and concise description of what you want to happen.

  • Vector.from_line and Cylinder.from_line_radius is rather self descriptive.
  • Line.point_from_start(distance) is similar to Line.point(t) but take a distance parameter, so I can have a point on the line at a certain distance.

[edit for more]

  • Line.flip() : Flip the line in place.
  • Line.from_start_direction_length(point, vector, length)

yck011522 avatar Sep 07 '21 12:09 yck011522

I have an implementation for Cylinder.from_line_radius (I'm not sure if it is correct):

    @classmethod
    def from_line_radius(cls, line, radius):
        """Construct a cylinder from its width, height and depth.

        Note that width is along the X-axis, height along Z-axis, and depth along the Y-axis.

        Parameters
        ----------
        line : :class:`compas.geometry.Line`
            Center line of the cylinder.
        radius : float
            Diameter of the cylinder.

        Returns
        -------
        Cylinder
            The constructed cylinder.

        Notes
        -----
        `cylinder.circle` is located at the mid point of the cylinder.

        """
        radius = float(radius)
        normal = Vector.from_start_end(line.start, line.end)
        circle = Circle(Plane(line.point(0.5), normal), radius)

        return cls(circle, line.length)

yck011522 avatar Sep 07 '21 12:09 yck011522

Line.point_from_start(distance)

    def point_from_start(self, distance):
        """A point along the line, at the specified distance from start.

        Parameters
        ----------
        distance : float
            The distance from start. Can be negative or larger than line.length.

        Returns
        -------
        Point
            A point on the line.

        Examples
        --------
        >>> line = Line([10.0, 0.0, 0.0], [20.0, 0.0, 0.0])
        >>> line.point(2.0)
        Point(12.000, 0.000, 0.000)
        """
        if distance == 0:
            return self.start

        v = self.direction * (distance)
        return self.start + v

yck011522 avatar Sep 07 '21 12:09 yck011522

This last thing almost already exists under the name Line.point(t), the only difference being that t represents a percentage of length of the line rather than the distance from the start.

beverlylytle avatar Sep 07 '21 12:09 beverlylytle

Yea, I'm aware @beverlylytle . In fact the last thing I modified from Line.point(t) . I think my point is to make my 'user' code easier to read with more choice of constructors. In this case, evaluating length.

yck011522 avatar Sep 07 '21 13:09 yck011522

Here I attach a sample of my 'user' code to illustrate what I mean. It is a function that generates boolean geometry for a 4-stage complex screw hole. IMG_20210907_152610

    def get_head_side_feature_shapes(self):
        # type: () -> list[Shape]
        """Returns the negative features of the screw.
        (Screw Head Side)
        """
        features = []
        
        # Larger diameter cylinder / Screw Head
        start_pt = self.center_line.point_from_start(-OVERSIZE)
        end_pt = self.center_line.point_from_start(self.screw_lengths[0])
        features.append(Cylinder.from_line_radius(Line(start_pt, end_pt), self.screw_diameters[0]/2))
        
        # Smaller diameter cylinder / Through hole
        start_pt = self.center_line.point_from_start(0)
        end_pt = self.center_line.point_from_start(self.screw_lengths[2])
        features.append(Cylinder.from_line_radius(Line(start_pt, end_pt), self.screw_diameters[1]/2))

        return features

    def get_thread_side_feature_shapes(self):
        # type: () -> list[Shape]
        """Returns the negative features of the screw.  
        (Screw Thread Side)
        """
        features = []

        # Shank extending to thread side
        if self.head_side_thickness < self.screw_lengths[1]:
            start_pt = self.center_line.point_from_start(0)
            end_pt = self.center_line.point_from_start(self.screw_lengths[1])
            features.append(Cylinder.from_line_radius(Line(start_pt, end_pt), self.screw_diameters[1]/2))

        # Larger diameter cylinder / Screw Thread
        start_pt = self.center_line.point_from_start(self.screw_lengths[1]-OVERSIZE)
        end_pt = self.center_line.point_from_start(self.screw_lengths[2])
        features.append(Cylinder.from_line_radius(Line(start_pt, end_pt), self.screw_diameters[2]/2))

        # Smaller diameter cylinder / Pull Screw Thread
        start_pt = self.center_line.point_from_start(self.screw_lengths[2]-OVERSIZE)
        end_pt = self.center_line.point_from_start(self.screw_lengths[3]+OVERSIZE)
        features.append(Cylinder.from_line_radius(Line(start_pt, end_pt), self.screw_diameters[3]/2))

        # Final pull screw relief for extra long joint.
        if self.total_length >  self.screw_lengths[3]:
            start_pt = self.center_line.point_from_start(self.screw_lengths[1]-OVERSIZE)
            end_pt = self.center_line.point_from_start(self.screw_lengths[2])
            features.append(Cylinder.from_line_radius(Line(start_pt, end_pt), self.screw_diameters[2]/2))  

        return features

yck011522 avatar Sep 07 '21 13:09 yck011522

@yck011522 || @gonzalocasas this is a nice issue, @yck011522 closing it could be a good omen?

jf--- avatar Apr 08 '24 18:04 jf---

@jf--- @gonzalocasas

This is super old one, but I found the function rather practical after years of use in my own fork.

I can open a PR if someone can let me know whether this function is worth included in the core library.

yck011522 avatar Apr 09 '24 02:04 yck011522

+1

jf--- avatar Apr 09 '24 12:04 jf---

just so you know, there are:

  • Line.from_point_and_vector
  • Line.from_point_direction_length
  • Cylinder.from_line_and_radius
  • Vector.from_start_end

also, every line has a vector and a direction

tomvanmele avatar Apr 09 '24 14:04 tomvanmele

@yck011522 Tom's diplomatic & informative answer reads as a subtle crystal clear "close" when round off, don't you think? from_line_radius vs from_line_and_radius shouldn't keep this issue from keeping this open?

jf--- avatar Apr 10 '24 21:04 jf---

Agree. Either we close this, or we decide on what to include in a PR, move forward and close it as well :)

gonzalocasas avatar Apr 16 '24 14:04 gonzalocasas

I think the functions that I originally propose is more of a usability sugar that helps reduce user code, especially for beginners. Probably slightly inspired by the functions that grasshopper offer, and thus able to ease beginners' adoption of compas.

I think each of the functions I proposed above can probably discussed and voted for separately:

Line.point_from_start """Get a point along the line at the given distance from start towards end."""

  • I believe still highly relevant as illustrated in the drill hole example above
  • saves a lot of user code lines

Line.point_from_end

  • for completeness sake

Line.point_at(t) (existing function) """Construct a point at a specific location along the line."""

  • we need to clarify that 'specific location' does not meaning distance.

Line.flip """Flip the line in place"""

  • Quite helpful one liner, similar function offered by Grasshopper.

Line.flipped """Returns a new line that is flipped."""

  • same as above but offer a copy

Vector.from_line

  • probably quite redundant as Line.direction and Line.vector now covers it.
  • ignore

Cylinder.from_line_radius

  • As mentioned, now covered by Cylinder.from_line_and_radius. this wasn't available back then.
  • ignore

Can I suggest a quick vote on the functions, I can prepare the PR.

yck011522 avatar Apr 16 '24 15:04 yck011522

Besides the ignore ones, the rest look like a go in my opinion. @tomvanmele ?

gonzalocasas avatar Apr 16 '24 16:04 gonzalocasas

@jf--- | @tomvanmele ping

yck011522 avatar Apr 19 '24 02:04 yck011522

perhaps reverse and reversed instead of flip and flipped?

would also add the same for Vector (i do vector.scaled(-1) a lot)

tomvanmele avatar Apr 19 '24 09:04 tomvanmele

I'm slightly inclined to keep the terminology borrowed from Rhino / Grasshopper where

  • Vector (and other non tangible things, such as list) are reversed
  • Lines (and other tangible entity, such as curves, meshes and surfaces) are flipped.

yck011522 avatar Apr 24 '24 15:04 yck011522

Oh I just found that Vector already has Vector.invert() and Vector.inverted(). For some reason, I never knew this is there and I also do vector.scaled(-1) a lot. Should we consider add an aliases name of reversed to inverted? Or we should from now on remember that inverted is there.

yck011522 avatar Apr 24 '24 15:04 yck011522

great karma closing this PR, nice one @yck011522 !!

jf--- avatar Apr 27 '24 08:04 jf---