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

straight line #28

Merged
merged 1 commit into from
Nov 3, 2019
Merged

straight line #28

merged 1 commit into from
Nov 3, 2019

Conversation

dcooley
Copy link
Contributor

@dcooley dcooley commented Oct 14, 2019

I've had a go at implement a straight line for #4

I've made a public variable isStraight

/// sets whether the line drawn should be straight
public var isStraight = false

which, when set to true creates a straight line between the first touch point and the current point

lines.removeLast()
            
let newLine = Line(path: CGMutablePath(),
    brush: Brush(color: brush.color, width: brush.width, opacity: brush.opacity, blendMode: brush.blendMode))
newLine.path.addPath(createNewStraightPath())
lines.append(newLine)
drawingHistory = lines

Using the methods

    private func createNewStraightPath() -> CGMutablePath {
        let pt1 : CGPoint = firstPoint
        let pt2 : CGPoint = currentPoint
        let subPath = createStraightSubPath(pt1, mid2: pt2)
        let newPath = addSubPathToPath(subPath)
        return newPath
    }

    private func createStraightSubPath(_ mid1: CGPoint, mid2: CGPoint) -> CGMutablePath {
        let subpath : CGMutablePath = CGMutablePath()
        subpath.move(to: mid1)
        subpath.addLine(to: mid2)
        return subpath
    }

@LinusGeffarth
Copy link
Collaborator

Thanks for your contribution!
I will check it out the next days and merge if possible :)

@LinusGeffarth LinusGeffarth self-requested a review October 14, 2019 21:08
@LinusGeffarth LinusGeffarth merged commit 55d7459 into Awalz:master Nov 3, 2019
Copy link
Collaborator

@LinusGeffarth LinusGeffarth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@LinusGeffarth
Copy link
Collaborator

LinusGeffarth commented Nov 4, 2019

Hey, so I merged it, but as I tried it out, I saw the following result, when dragging in a circle:

IMG_26A3EB0B2D72-1

Hence, I marked it as an experimental feature, but will include it in the next release.
Would you like to investigate?

@dcooley
Copy link
Contributor Author

dcooley commented Nov 5, 2019

yeah that's weird. I'll see if I can track it down.

@dcooley
Copy link
Contributor Author

dcooley commented Dec 28, 2019

@LinusGeffarth I've finally managed to get around to this, and I think the solution is to put a setNeedsDisplay() inside the if shouldDrawStraight { condition here

        if shouldDrawStraight {
            lines.removeLast()
            setNeedsDisplay()   
            let newLine = Line(path: CGMutablePath(),
                               brush: Brush(color: brush.color, width: brush.width, opacity: brush.opacity, blendMode: brush.blendMode))
            newLine.path.addPath(createNewStraightPath())
            lines.append(newLine)
            drawingHistory = lines
        } else {
            let newPath = createNewPath()
            if let currentPath = lines.last {
                currentPath.path.addPath(newPath)
            }
        }
    }

Would you like me to make another PR, or are you happy to test and implement this yourself?


But I am also sometimes getting an error thrown in lines is empty, so I'll try and track that down too.

@LinusGeffarth
Copy link
Collaborator

Thanks. If you could open a PR, that'd be appreciated.

@dcooley
Copy link
Contributor Author

dcooley commented Dec 29, 2019

done.

The other issue I was having was caused something else in my project, so I don't think there is anything else to be done.

@talaviram
Copy link
Contributor

Is this still considered experimental after invalidating (setNeedsDisplay())?
because the code still has a comment this is experimental.

@dcooley
Copy link
Contributor Author

dcooley commented Apr 13, 2020

For what it's worth, I've not had any issues in the 3 months of using this feature.

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

Successfully merging this pull request may close these issues.

3 participants