Analyzing spoke overlaps during rotation: Round 2

Question

Round 1: Analyzing spoke overlaps during rotation

I previously posted this question, but I've changed a lot of the code since then.

There was a mention of the previous being a bit mixed up, and things being too integrated, so I've abstracted away a lot of that functionality.

The purpose of this program is to draw two shapes with given counts of "spokes", and then analyze the rotation between them and how often the spokes would intersect / overlap / pass each other.

To start, I've defined a Shape type that has various components we'll need throughout the program:

Types.fs:

[<AutoOpen>]
module Spoke_Analyzer.Types

[<Struct>]
type Shape =
    { Spokes : int
      Angle : float
      PointSize : int
      PenWidth : float32
      Color : System.Drawing.Color }

Next, I moved a couple components to Math.fs, in particular:

• getXY: this will take the distance and angle and calculate what (X, Y) that is from the origin;
• getOverlaps: this will use the property that the number of total overlaps = the multiple of both spoke counts, then to establish the number of rotations to get all overlaps, it will determine if there are any coincident spokes (large % small = 0) and divide those out;

Additionally, I noticed that there were quite a few edge-cases and miscalculations in the getRotation (though, for the single use-case I needed it worked fine), so I went about it a different route and they seem to have disappeared.

Math.fs:

module Spoke_Analyzer.Math

let inline degToRad deg = deg * System.Math.PI / 180.
let inline radToDeg rad = rad * 180. / System.Math.PI
let inline angleMath distance angle fn = (distance * (angle |> degToRad |> fn)) |> int 

let inline getXY distance angle = cos |> angleMath distance angle, sin |> angleMath distance angle

let inline getOverlaps count1 count2 =
    let inline calculation smallCount largeCount =
        let totalOverlaps = (smallCount |> float) * (largeCount |> float)
        // If the small divides into large evenly, then there are `small` coincident spokes, and we'll divide our total
        // overlap count by that number.
        if largeCount % smallCount <> 0 then totalOverlaps
        else totalOverlaps / (smallCount |> float)

    if count1 > count2 then calculation count2 count1
    else calculation count1 count2 

Our Graphics.fs got a major overhaul:

• drawSpokes: this will draw the spokes for a given shape;
• saveImages: this will draw and save the images, using the drawSpokes function;
• makeGif: this will make the gif file / do the ffmpeg work;

Graphics.fs:

module Spoke_Analyzer.Graphics
open System
open System.Drawing
open Spoke_Analyzer

let drawLine (origin : Point) (g : Graphics) (pen : Pen) (start : Point, stop : Point) =
    g.DrawLine(pen, Point(start.X + origin.X, start.Y + origin.Y), Point(stop.X + origin.X, stop.Y + origin.Y))

let drawPoint (origin : Point) (g : Graphics) (brush : Brush) width (start : Point) =
    g.FillEllipse(brush, Rectangle(start.X + origin.X - (width / 2), start.Y + origin.Y - (width / 2), width, width))

let drawSpokes (origin : Point) (g : Graphics) distance (pen : Pen) (brush : Brush) shape offset =
    let drawLine = drawLine origin g pen
    let drawPoint = drawPoint origin g brush
    let drawSpoke num =
        (Point(0, 0),
         (shape.Angle * (num |> float) + offset) |> Spoke_Analyzer.Math.getXY distance |> Point)
        |> drawLine
    [|0..shape.Spokes|] |> Array.iter (drawSpoke)
    offset |> Spoke_Analyzer.Math.getXY distance |> Point |> drawPoint shape.PointSize

let saveImages imageWidth imageDir shape1 shape2 rotationOffset offset angleDifference totalRotations = 
    let drawImage (bmp : Bitmap) (clearColor : Color) i =
        use pen1 = new Pen(shape1.Color, shape1.PenWidth)
        use brush1 = new SolidBrush(shape1.Color)
        use pen2 = new Pen(shape2.Color, shape2.PenWidth)
        use brush2 = new SolidBrush(shape2.Color)            

        use g = bmp |> Graphics.FromImage
        g.SmoothingMode <- Drawing2D.SmoothingMode.AntiAlias
        g.Clear(clearColor)
        let drawSpokes = drawSpokes (Point(bmp.Width / 2, bmp.Height / 2)) g (((imageWidth - ((max shape1.PointSize shape2.PointSize) / 2 + 2)) / 2) |> float)
        drawSpokes pen1 brush1 shape1 rotationOffset
        drawSpokes pen2 brush2 shape2 (shape2.Angle * offset + rotationOffset + angleDifference * (i |> float))
        ()

    let totalRotations = if totalRotations |> Double.IsInfinity then 1 else totalRotations |> int
    use bmp = new Bitmap(imageWidth, imageWidth)
    [|0..totalRotations - 1|]
    |> Array.iter (fun i ->
        drawImage bmp (Color.FromArgb(255, 32, 32, 32)) i
        bmp.Save(sprintf "%s/rot_%i.png" imageDir i, Imaging.ImageFormat.Png))

let makeGif fps imageDir : string option -> unit =    
    function
    | Some ffmpeg ->
        let ffmpeg =
            if ffmpeg.EndsWith("ffmpeg") = false && ffmpeg.EndsWith("ffmpeg.exe") = false then
                System.IO.Path.Combine(ffmpeg, "ffmpeg")
            else ffmpeg

        printfn "Running ffmpeg..."
        System
            .Diagnostics
            .Process
            .Start(ffmpeg, sprintf "-framerate %i -f image2 -i %s/rot_%%d.png -c:v libx264 -crf 0 -r %i -preset ultrafast -tune stillimage %s/temp.avi" fps imageDir fps imageDir)
            .WaitForExit()
        System
            .Diagnostics
            .Process
            .Start(ffmpeg, sprintf "-i %s/temp.avi -pix_fmt rgb24 %s/_final.gif" imageDir imageDir)
            .WaitForExit()
        printfn "Images converted to gif."
        printfn ""
    | _ -> ()

The only changes to Input.fs were some formatting:

Input.fs:

module Spoke_Analyzer.Input
open System

let rec getInput convert validate prompt =
    printf "%s" prompt
    let input = () |> Console.ReadLine
    if input |> validate then
        input |> convert
    else
        printfn "Invalid, please try again."
        getInput convert validate prompt
let getInputInt =
    getInput
        Int32.Parse
        (Int32.TryParse >> function | true, f when f > 0 -> true | _ -> false)
let getInputIntOption =
    getInput
        (function | "" -> None | s -> s |> Int32.Parse |> Some)
        (function | "" -> true | s -> s |> Int32.TryParse |> function | true, f when f > 0 -> true | _ -> false)
let getInputDoubleOption =
    getInput
        (function | "" -> None | s -> s |> Double.Parse |> Some)
        (function | "" -> true | s -> s |> Double.TryParse |> function | true, f when f >= 0. && f <= 1. -> true | _ -> false)
let getInputDouble =
    getInput
        Double.Parse
        (Double.TryParse >> function | true, f when f >= 0. && f <= 1. -> true | _ -> false)
let getInputFileOption (file : string) =
    getInput
        (function | "" -> None | s -> Some s)
        (function
         | "" -> true
         | s ->
             if Uri.IsWellFormedUriString(sprintf "file:///%s" (s.Replace('\\', '/')), UriKind.RelativeOrAbsolute) then
                let test = 
                    if s.EndsWith(file) = false && s.EndsWith(sprintf "%s.exe" file) = false then
                        let t = System.IO.Path.Combine(s, file)
                        if System.IO.File.Exists(t) then t
                        else System.IO.Path.Combine(s, sprintf "%s.exe" file)
                    else s
                if System.IO.File.Exists(test) then
                    true
                else false
             else false)

We also added an IO.fs to separate directory concerns out:

IO.fs:

module Spoke_Analyzer.IO

let createDirIfNotExists dir =
    if dir |> System.IO.Directory.Exists then dir |> System.IO.DirectoryInfo
    else dir |> System.IO.Directory.CreateDirectory

let clearDir = System.IO.Directory.GetFiles >> Array.iter System.IO.File.Delete

And finally, our Program.fs got a lot tighter:

Program.fs:

open System
open System.Drawing
open Spoke_Analyzer
open Spoke_Analyzer

[<Literal>]
let FULL_CIRCLE = 360.
[<Literal>]
let ROTATION_OFFSET = -90. // -FULL_CIRCLE / 4.
[<Literal>]
let IMAGE_DIR = "temp/images"

[<EntryPoint>]
let main argv =
    let getShape pointSize penWidth color i =
        let spokes = i |> sprintf "Enter the number of spokes for shape %i (whole number > 0): " |> Input.getInputInt
        { Spokes = spokes; Angle = FULL_CIRCLE / (spokes |> float); PointSize = pointSize; PenWidth = penWidth; Color = color }

    let imageWidth = Input.getInputIntOption "Enter the image dimension (whole number > 0) [160]: " |> Option.defaultValue 160
    let shape1 = getShape 8 2.5f (Color.FromArgb(255, 224, 32, 32)) 1
    let shape2 = getShape 6 1.5f (Color.FromArgb(255, 32, 224, 32)) 2
    let offset = Input.getInputDoubleOption "Enter the radial offset in percentage (0.0 - 1.0) [0]: " |> Option.defaultValue 0.
    let ffmpeg = Input.getInputFileOption "ffmpeg" "Enter the location of ffmpeg (if available) []: "
    let fps = ffmpeg |> Option.bind (fun s -> Input.getInputIntOption "Enter the fps of the output (whole number > 0) [24]: ") |> Option.defaultValue 24    
    printfn ""

    let totalRotations = Spoke_Analyzer.Math.getOverlaps shape1.Spokes shape2.Spokes
    let angleDifference = FULL_CIRCLE / totalRotations

    IMAGE_DIR |> Spoke_Analyzer.IO.createDirIfNotExists |> ignore
    IMAGE_DIR |> Spoke_Analyzer.IO.clearDir |> ignore
    Graphics.saveImages imageWidth IMAGE_DIR shape1 shape2 ROTATION_OFFSET offset angleDifference totalRotations
    printfn "Images saved."    
    Graphics.makeGif fps IMAGE_DIR ffmpeg

    printfn "Shape 1 Angle (%i spokes): %f° / %f rads" shape1.Spokes shape1.Angle (shape1.Angle |> Spoke_Analyzer.Math.degToRad)
    printfn "Shape 2 Angle (%i spokes): %f° / %f rads" shape2.Spokes shape2.Angle (shape2.Angle |> Spoke_Analyzer.Math.degToRad)
    printfn "Overlap Angle Difference: %f° / %f rads (%f rotations)" angleDifference (angleDifference |> Spoke_Analyzer.Math.degToRad) (angleDifference / FULL_CIRCLE)
    printfn "Overlaps per Rotation: %f" totalRotations

    0

I've no idea if LoC went down or not, nor do I really care, because this feels like a much more maintainable version.

As always, all suggestions welcome.

The project is on GitHub: https://github.com/EBrown8534/Spoke_Analyzer

Answer 1

In getOverlaps I think you should find the gcd(small, large) and divide with that instead of testing for large % small <> 0 (if I understand the project right).

E.g.: spokes 12 and 15 -> gcd = 3 -> 180 / 3 = 60 rotations -> rotation angle: 360 / 60 = 6

---

IMO you overdo the use of the |> operator a little:

let input = () |> Console.ReadLine
if input |> validate then

To me it is more clear and straight forward to write:

let input = Console.ReadLine()
if validate input then
  ...

---

All in all it's definitely better and cleaner than the first version, but I still think it could be more stringent in the workflow.

For instance:

The main function could be be split into:

let processInfo = promptForInput()

prepareOutputDirectory processInfo.OutputPath
processRotations processInfo

All the exiting stuff are hidden in Graphics.saveImages. I think I would "revert" the process/workflow, so that it is more clear what the algorithm and what the output are - for instance:

let processRotations processInfo =
    let rec processRotation info =
        if info.RotationIndex < info.Rotations then
            createImage info
            processRotation (rotate info)
        else
            postProcess info  // create gif, print info etc...

    processRotation processInfo

The function rotate info handles the rotation of one of the shapes and returns a new instance of processInfo that holds that new state state plus all the other (input) information

I hope this skeleton makes any sense...