Shapes 7

[mjkalyan]

Please check to make sure Bezier line support is added desirably. Otherwise, this should be good to merge.

[mjkalyan]

It's probably better to just add control point slots %control-point-1 & %control-point-2 to line-2d. If both are given, we draw a cubic curve, if one is given we draw a quadratic curve, if none are given we draw a line.

[mjkalyan]

It's probably better to just add control point slots %control-point-1 & %control-point-2 to line-2d. If both are given, we draw a cubic curve, if one is given we draw a quadratic curve, if none are given we draw a line.

But an issue with this approach is we don't get the nice automatic cubic Bezier that draw-line-bezier gives us. So maybe the %bezier slot could specify one of :auto, :quad, or :cubic. :auto for draw-line-bezier, :quad for draw-line-bezier-quad using %control-point-1, and :cubic as you'd expect.

[shelvick]

So maybe the %bezier slot could specify one of :auto, :quad, or :cubic. :auto for draw-line-bezier, :quad for draw-line-bezier-quad using %control-point-1, and :cubic as you'd expect.

I like this. And NIL of course for a straight line. That encompasses all of the 2D line types except for line strips, which should probably be a separate class (in a future PR, not this one).

[shelvick]

Actually... couldn't we just keep it a boolean? NIL = straight line T with no control points = auto bezier T with one point = bezier quad T with two points = bezier cubic

[mjkalyan]

This (https://github.com/defun-games/claylib/pull/23/commits/4a5829c46e3cd1417437afe0b454322bd52c3e0a) works, but I'd like to make constructing the args list conditionally with some repeated args more elegant. I tried this solution, which also works, but is a tiny bit slower and not much prettier:

(defmethod draw-object ((obj line-2d))
  (let* ((bezier (bezier obj))
         (pt (control-pt obj))
         (pt2 (control-pt2 obj))
         (first-args (list (c-struct (start obj))
                           (c-struct (end obj))))
         (last-args (list (thickness obj)
                          (c-struct (color obj))))
         (fn (cond ((and pt2 pt bezier) (progn
                                          (setf last-args (cons (c-struct pt)
                                                                (cons (c-struct pt2) last-args)))
                                          #'claylib/ll:draw-line-bezier-cubic))
                   ((and pt bezier)     (progn
                                          (setf last-args (cons (c-struct pt) last-args))
                                          #'claylib/ll:draw-line-bezier-quad))
                   (bezier              #'claylib/ll:draw-line-bezier)
                   (t                   #'claylib/ll:draw-line-ex))))
    (apply fn (nconc first-args last-args))))

I'd rather not sacrifice any performance in a draw method... Any suggestions are appreciated.

[shelvick]

First of all, how much slower is "a tiny bit" and how did you measure this? Enough to be a notable concern or just premature optimization?

A few suggestions for elegance off the top of my head:

• A c-structs function to get the structs of multiple things at once
• Use a macrolet or just a macro
• Separate the cond part into its own flet or function

[mjkalyan]

Just premature optimization. Both solutions should have the same time complexity, but the one pasted above is 10-30 thousand cycles slower (as measured by wrapping draw-object's body in a time call) depending on the method (line/auto/quad/cubic).

I will fix this soon, thanks for the input!

[mjkalyan]

I think this'll have to do. From the most recent commit message:

"This is close to as good as we can get performance and readability wise, as far as I can tell. The crux of the issue is we can't get around listing the functions and arguments to pick from without sacrificing performance. Any attempt to dynamically build the args is slower than just picking from a known execution path."

[shelvick]

Nah, I think this is good actually. Was going to suggest a macro of some kind to remove repetition but you did a pretty good job of that; I don't think it's needed.