[부나] 뷰 챌린지 미션 2단계 제출합니다. #32
Conversation
| fun Paint.softPainter( | ||
| paletteColor: PaletteColor = PaletteColor.RED, | ||
| paintStyle: Paint.Style = Paint.Style.STROKE, | ||
| porterDuffMode: PorterDuff.Mode = PorterDuff.Mode.SRC_OVER, |
There was a problem hiding this comment.
PorterDuff.Mode.SRC_OVER 는 어떤 옵션인가요??
There was a problem hiding this comment.
기존에 그려져 있는 리소스 위에 작성하겠다는 의미입니다 : )
| fun onActionUp(x: Float, y: Float) { | ||
| currentPainter = currentPainter.extract() | ||
| } |
There was a problem hiding this comment.
언젠가 onActionUp 되었을 때 좌표가 필요한 경우가 있을 것 같아서 지우지 않았는데,
생각해보니 필요한 시점에 추가하는 게 낫겠네요 👍
| private val path: Path = Path(), | ||
| val paint: Paint = Paint().softPainter(), |
There was a problem hiding this comment.
path는 private으로 변경되었는데 paint는 public인 이유가 있나요??
There was a problem hiding this comment.
IDE에서 따로 알려주지 않아서 몰랐네요!
private으로 변경하겠습니다 😃
| override fun extract(): Painter = copy( | ||
| path = Path(), | ||
| paint = Paint(paint), | ||
| ) |
There was a problem hiding this comment.
제 코드는 onActionDown되었을 때 리스트에 추가하고,
onActionUp 되었을 때 새로운 Painter로 변경해주는 방식으로 Undo 리스트를 관리하고 있습니다.
그래서 각 Painter별로 새로운 객체를 생성하는 기능이 필요해서 해당 코드를 구현하였습니다.
단순 data class이면 copy()를 사용하면 되는데, Painter라는 추상 클래스를 구현하다보니 외부에서는 copy()를 사용할 수가 없습니다.. 🥲
해시가 알고 있는 좋은 방법이 있을까요?
There was a problem hiding this comment.
아아 extract()로 리셋하지 않으면 색상이나 두께를 변경하지 않고 여러 개를 그렸을때 문제가 있겠네요..! 저도 비슷한 방식으로 copy하고 있어요. 다만 저는 아래 코드처럼 구현해서 색상이나 두께 변경 시에도 같은 메서드 사용하고 있습니다! 더 좋은 방법이 있는지는 아직 잘 모르겠네요..
override fun copy(color: Int?, width: Float?): Brush {
val newPaint = Paint(paint).apply {
this.color = color ?: paint.color
this.strokeWidth = width ?: paint.strokeWidth
}
return LineBrush(Path(), newPaint)
}There was a problem hiding this comment.
위 코드를 적용해보고자 하였는데 제 코드는 생성자에 필요한 인자가 각각 달라서 적용이 힘들겠네요ㅠㅠ!
올려주신 코드도 좋아 보이네요~
| override fun draw(canvas: Canvas) { | ||
| canvas.drawArc(rect, 0F, 360F, true, paint) | ||
| } |
There was a problem hiding this comment.
다시 생각해보니 원의 360도 전체를 그릴거라면 drawOval() 을 사용해도 똑같네요 ^_^
인자도 훨씬 적어지구요!
| private val DEFAULT_PAINT: Paint | ||
| get() = Paint().softPainter(paintStyle = Paint.Style.FILL) |
There was a problem hiding this comment.
CirclePainter와 RectanglePainter도 softPainter 여야 하나요?
There was a problem hiding this comment.
넵! softPainter의 내부를 확인해보면 antiAlias가 true로 설정되어 매끄럽게 그려주고 있습니다.
물론 그 외에 필요하지 않은 속성들도 있기는 하지만 도형을 그릴 때 문제가 되지 않는 속성이라 그대로 유지하였습니다.
There was a problem hiding this comment.
antiAlias가 도형에도 영향을 주는 옵션이군요! 새로 알아갑니다 👍
| fun changePainter( | ||
| paletteMode: PaletteMode, | ||
| paletteShape: PaletteShape = PaletteShape.values().first(), | ||
| ): Painter { | ||
| return when (paletteMode) { | ||
| PaletteMode.BRUSH -> BrushPainter() | ||
| PaletteMode.SHAPE -> getShapePainter(paletteShape) | ||
| PaletteMode.ERASER -> PathEraserPainter() | ||
| } | ||
| } | ||
|
|
||
| private fun getShapePainter( | ||
| paletteShape: PaletteShape, | ||
| ): Painter = when (paletteShape) { | ||
| PaletteShape.RECTANGLE -> RectanglePainter(paletteShape) | ||
| PaletteShape.CIRCLE -> CirclePainter(paletteShape) | ||
| } |
There was a problem hiding this comment.
PainterHistory 내부에서 currentPainter를 변경하는 함수로 만들어도 될 것 같은데 Painter 인터페이스 내부에 구현하신 이유가 궁금합니다!
There was a problem hiding this comment.
현재 네이밍이 changePainter로 되어 있고 정적이지는 않지만, 기능만 보면 사실 정적 팩토리 함수와 동일합니다.
그래서 필요한 인자를 전달해주면 Painter 객체를 생성해주는 역할을 Painter 자체에 부여하였습니다.
There was a problem hiding this comment.
인터페이스에 구현 메서드를 둔 것이 조금 어색하다 생각했는데 추상 클래스로 변경됐네요! 팩토리 함수라면 하위클래스가 재정의 할 일도 없으니 companion object에 두는 것은 어떤가요??
There was a problem hiding this comment.
상태를 가지고 있지 않기 때문에 interface로 하였다가, paint라는 상태를 가지면서 추상클래스로 변경하였습니다~
팩토리 함수라면 하위클래스가 재정의 할 일도 없으니 companion object에 두는 것은 어떤가요??
코멘트만 달아놓고 아직 수정하지 않았었네요!
반영하겠습니다 👍
// Painter.kt
companion object {
fun changePainter(
paletteMode: PaletteMode,
painterState: PainterState,
) { ... }| var selectedPaintThickness: Float = THICKNESS_SIZE_UNIT | ||
| var selectedPaletteColor: PaletteColor = PaletteColor.values().first() |
There was a problem hiding this comment.
private set으로 두면 좀 더 안전할 것 같네요!
| import woowacourse.paint.view.palette.color.PaletteColor | ||
|
|
||
| fun Paint.softPainter( | ||
| paletteColor: PaletteColor = PaletteColor.RED, |
There was a problem hiding this comment.
팔레트 색상이나 굵기가 유지되면 사용자 입장에서 더 자연스러울 것 같아요! 현재는 지우개나 브러시를 선택했을 때 슬라이더 위치와 실제 그려지는 두께가 다르고, 버튼을 누를 때마다 색상이 빨간색으로 초기화되어 불편합니다.
이건 요구사항은 아니니까 자유롭게 반영해주세요! (어디에 코멘트 다는게 좋을지 모르겠어서 아무데나 남깁니다..ㅋㅋ)
There was a problem hiding this comment.
아무래도 Painter의 changePainter() 메서드에서 매번 새로운 Painter를 반환해주니 생기는 문제인 것 같습니다 🥲
현재 PainterHistory에서 두께, 도형, 색상에 대한 상태를 가지고 있도록 하였고, 브러시 변경시 Painter.changePainter()의 인자로 이전의 상태를 전달하도록 변경하였습니다.
|
안녕하세요 해시! |
| fun undo() { | ||
| val redo = undoes.removeLastOrNull() ?: return | ||
| redoes.add(redo) | ||
| } | ||
|
|
||
| fun redo() { | ||
| val undo = redoes.removeLastOrNull() ?: return | ||
| undoes.add(undo) | ||
| } |
There was a problem hiding this comment.
이건 정책 차이라고 생각해서 꼭 반영해야하는 부분은 아니라고 생각하는데..
일반적으로 노트나 그림판 앱을 보면 undo를 실행하고 다른 그림을 추가하면 redoes가 clear() 됩니다!
There was a problem hiding this comment.
맞습니다!
하지만 저는 개인적으로 이러한 undo/redo 방식에 불편함을 느끼고 있었기에,
UNDO를 했을 때 clear 하지 않고 기존 데이터를 유지하는 정책을 선택했습니다!
안녕하세요 해시! ☀️
만족한 기능 요구사항은 다음과 같습니다.
기능 요구 사항
그림판 앱에 기능을 추가한다.
선택 요구 사항
또한, 도형, 지우개 등이 추가되면서 기존에 PathPainter 클래스를 그대로 사용하는 대신, 모두 Painter interface를 구현하도록 변경하였습니다.
리뷰 잘 부탁드립니다 😃