Skip to content

Commit

Permalink
Merge pull request Catrobat#1261 from khalid-nasralla/PAINTROID-542
Browse files Browse the repository at this point in the history
PAINTROID-542 Fix Fill-Tool creating checkered pattern on erased parts
  • Loading branch information
SebastianGrief authored Jun 21, 2023
2 parents ff695b1 + bc1a47c commit a044cad
Show file tree
Hide file tree
Showing 11 changed files with 298 additions and 134 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,25 @@ package org.catrobat.paintroid.test.espresso.tools

import android.graphics.Color
import android.net.Uri
import androidx.test.espresso.Espresso
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.IdlingRegistry
import androidx.test.espresso.action.ViewActions
import androidx.test.espresso.assertion.ViewAssertions
import androidx.test.espresso.action.ViewActions.replaceText
import androidx.test.espresso.action.ViewActions.closeSoftKeyboard
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.idling.CountingIdlingResource
import androidx.test.espresso.matcher.ViewMatchers
import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.isRoot
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.rule.ActivityTestRule
import org.catrobat.paintroid.MainActivity
import org.catrobat.paintroid.R
import org.catrobat.paintroid.test.espresso.util.BitmapLocationProvider
import org.catrobat.paintroid.test.espresso.util.DrawingSurfaceLocationProvider
import org.catrobat.paintroid.test.espresso.util.UiInteractions
import org.catrobat.paintroid.test.espresso.util.UiMatcher
import org.catrobat.paintroid.test.espresso.util.UiInteractions.swipeAccurate
import org.catrobat.paintroid.test.espresso.util.UiInteractions.touchAt
import org.catrobat.paintroid.test.espresso.util.UiInteractions.waitFor
import org.catrobat.paintroid.test.espresso.util.UiMatcher.withProgress
import org.catrobat.paintroid.test.espresso.util.wrappers.DrawingSurfaceInteraction.Companion.onDrawingSurfaceView
import org.catrobat.paintroid.test.espresso.util.wrappers.ToolBarViewInteraction.Companion.onToolBarView
import org.catrobat.paintroid.test.espresso.util.wrappers.ToolPropertiesInteraction.Companion.onToolProperties
Expand All @@ -45,13 +49,12 @@ import org.catrobat.paintroid.tools.ToolType
import org.catrobat.paintroid.tools.implementation.DEFAULT_TOLERANCE_IN_PERCENT
import org.catrobat.paintroid.tools.implementation.FillTool
import org.catrobat.paintroid.ui.Perspective
import org.junit.Rule
import org.junit.After
import org.junit.runner.RunWith
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.Assert
import org.junit.Ignore
import org.junit.runner.RunWith
import java.io.File

@RunWith(AndroidJUnit4::class)
Expand Down Expand Up @@ -89,7 +92,7 @@ class FillToolIntegrationTest {
onToolProperties()
.checkMatchesColor(Color.BLACK)
onDrawingSurfaceView()
.perform(UiInteractions.touchAt(DrawingSurfaceLocationProvider.MIDDLE))
.perform(touchAt(DrawingSurfaceLocationProvider.MIDDLE))
onDrawingSurfaceView()
.checkPixelColor(Color.BLACK, BitmapLocationProvider.MIDDLE)
mainActivity.model.savedPictureUri = null
Expand All @@ -100,7 +103,7 @@ class FillToolIntegrationTest {
onToolProperties()
.checkMatchesColor(Color.BLACK)
onDrawingSurfaceView()
.perform(UiInteractions.touchAt(DrawingSurfaceLocationProvider.MIDDLE))
.perform(touchAt(DrawingSurfaceLocationProvider.MIDDLE))
onDrawingSurfaceView()
.checkPixelColor(Color.BLACK, BitmapLocationProvider.MIDDLE)
}
Expand All @@ -111,7 +114,7 @@ class FillToolIntegrationTest {
onToolProperties()
.checkMatchesColor(Color.BLACK)
onDrawingSurfaceView()
.perform(UiInteractions.touchAt(DrawingSurfaceLocationProvider.OUTSIDE_MIDDLE_RIGHT))
.perform(touchAt(DrawingSurfaceLocationProvider.OUTSIDE_MIDDLE_RIGHT))
onDrawingSurfaceView()
.checkPixelColor(Color.TRANSPARENT, BitmapLocationProvider.MIDDLE)
}
Expand All @@ -123,16 +126,16 @@ class FillToolIntegrationTest {
onToolProperties()
.checkMatchesColor(Color.BLACK)
onDrawingSurfaceView()
.perform(UiInteractions.swipeAccurate(DrawingSurfaceLocationProvider.HALFWAY_TOP_MIDDLE, DrawingSurfaceLocationProvider.HALFWAY_RIGHT_MIDDLE))
.perform(UiInteractions.swipeAccurate(DrawingSurfaceLocationProvider.HALFWAY_RIGHT_MIDDLE, DrawingSurfaceLocationProvider.HALFWAY_BOTTOM_MIDDLE))
.perform(UiInteractions.swipeAccurate(DrawingSurfaceLocationProvider.HALFWAY_BOTTOM_MIDDLE, DrawingSurfaceLocationProvider.HALFWAY_LEFT_MIDDLE))
.perform(UiInteractions.swipeAccurate(DrawingSurfaceLocationProvider.HALFWAY_LEFT_MIDDLE, DrawingSurfaceLocationProvider.HALFWAY_TOP_MIDDLE))
.perform(swipeAccurate(DrawingSurfaceLocationProvider.HALFWAY_TOP_MIDDLE, DrawingSurfaceLocationProvider.HALFWAY_RIGHT_MIDDLE))
.perform(swipeAccurate(DrawingSurfaceLocationProvider.HALFWAY_RIGHT_MIDDLE, DrawingSurfaceLocationProvider.HALFWAY_BOTTOM_MIDDLE))
.perform(swipeAccurate(DrawingSurfaceLocationProvider.HALFWAY_BOTTOM_MIDDLE, DrawingSurfaceLocationProvider.HALFWAY_LEFT_MIDDLE))
.perform(swipeAccurate(DrawingSurfaceLocationProvider.HALFWAY_LEFT_MIDDLE, DrawingSurfaceLocationProvider.HALFWAY_TOP_MIDDLE))
onToolBarView()
.performSelectTool(ToolType.FILL)
onToolProperties()
.setColorResource(R.color.pocketpaint_color_picker_green1)
onDrawingSurfaceView()
.perform(UiInteractions.touchAt(DrawingSurfaceLocationProvider.MIDDLE))
.perform(touchAt(DrawingSurfaceLocationProvider.MIDDLE))
onDrawingSurfaceView()
.checkPixelColorResource(R.color.pocketpaint_color_picker_green1, BitmapLocationProvider.MIDDLE)
.checkPixelColor(Color.TRANSPARENT, BitmapLocationProvider.MIDDLE_RIGHT)
Expand All @@ -148,15 +151,19 @@ class FillToolIntegrationTest {
fillTool.colorTolerance.toDouble(),
TOLERANCE_DELTA
)
val colorToleranceInput = Espresso.onView(withId(R.id.pocketpaint_fill_tool_dialog_color_tolerance_input))
val colorToleranceSeekBar = Espresso.onView(withId(R.id.pocketpaint_color_tolerance_seek_bar))
val colorToleranceInput = onView(withId(R.id.pocketpaint_fill_tool_dialog_color_tolerance_input))
val colorToleranceSeekBar = onView(withId(R.id.pocketpaint_color_tolerance_seek_bar))
val testToleranceText = "100"
colorToleranceInput.check(ViewAssertions.matches(ViewMatchers.withText(
colorToleranceInput.check(
matches(
withText(
DEFAULT_TOLERANCE_IN_PERCENT.toString()
)))
colorToleranceInput.perform(ViewActions.replaceText(testToleranceText), ViewActions.closeSoftKeyboard())
colorToleranceInput.check(ViewAssertions.matches(ViewMatchers.withText(testToleranceText)))
colorToleranceSeekBar.check(ViewAssertions.matches(UiMatcher.withProgress(testToleranceText.toInt())))
)
)
)
colorToleranceInput.perform(replaceText(testToleranceText), closeSoftKeyboard())
colorToleranceInput.check(matches(withText(testToleranceText)))
colorToleranceSeekBar.check(matches(withProgress(testToleranceText.toInt())))
val expectedAbsoluteTolerance = fillTool.getToleranceAbsoluteValue(100)
Assert.assertEquals("Wrong fill tool member value for color tolerance", expectedAbsoluteTolerance.toDouble(), fillTool.colorTolerance.toDouble(), TOLERANCE_DELTA)

Expand All @@ -168,10 +175,10 @@ class FillToolIntegrationTest {
@Test
fun testFillToolDialogAfterToolSwitch() {
val fillTool = toolReference!!.tool as FillTool?
val colorToleranceInput = Espresso.onView(withId(R.id.pocketpaint_fill_tool_dialog_color_tolerance_input))
val colorToleranceSeekBar = Espresso.onView(withId(R.id.pocketpaint_color_tolerance_seek_bar))
val colorToleranceInput = onView(withId(R.id.pocketpaint_fill_tool_dialog_color_tolerance_input))
val colorToleranceSeekBar = onView(withId(R.id.pocketpaint_color_tolerance_seek_bar))
val toleranceInPercent = 50
colorToleranceInput.perform(ViewActions.replaceText(toleranceInPercent.toString()))
colorToleranceInput.perform(replaceText(toleranceInPercent.toString()))
val expectedAbsoluteTolerance = fillTool!!.getToleranceAbsoluteValue(toleranceInPercent)
Assert.assertEquals("Wrong fill tool member value for color tolerance", expectedAbsoluteTolerance.toDouble(), fillTool.colorTolerance.toDouble(), TOLERANCE_DELTA)

Expand All @@ -182,34 +189,35 @@ class FillToolIntegrationTest {
.performSelectTool(ToolType.BRUSH)
onToolBarView()
.performSelectTool(ToolType.FILL)
colorToleranceInput.check(ViewAssertions.matches(ViewMatchers.withText(
colorToleranceInput.check(
matches(
withText(
DEFAULT_TOLERANCE_IN_PERCENT.toString()
)))
colorToleranceSeekBar.check(ViewAssertions.matches(UiMatcher.withProgress(DEFAULT_TOLERANCE_IN_PERCENT)))
)
)
)
colorToleranceSeekBar.check(matches(withProgress(DEFAULT_TOLERANCE_IN_PERCENT)))
}

@Ignore("Fails on Jenkins, trying out if everything works without this test or if error is due to a bug on Jenkins")
@Test
fun testFillToolUndoRedoWithTolerance() {
onToolBarView()
.performSelectTool(ToolType.BRUSH)
onDrawingSurfaceView()
.checkPixelColor(Color.TRANSPARENT, BitmapLocationProvider.MIDDLE)
.perform(UiInteractions.touchAt(DrawingSurfaceLocationProvider.MIDDLE))
.perform(touchAt(DrawingSurfaceLocationProvider.MIDDLE))
onDrawingSurfaceView()
.checkPixelColor(Color.BLACK, BitmapLocationProvider.MIDDLE)
onToolProperties()
.setColorResource(R.color.pocketpaint_color_picker_brown2)
.checkMatchesColorResource(R.color.pocketpaint_color_picker_brown2)
onToolBarView()
.performSelectTool(ToolType.FILL)
.performOpenToolOptionsView()
Espresso.onView(withId(R.id.pocketpaint_fill_tool_dialog_color_tolerance_input))
.perform(ViewActions.replaceText(100.toString()))
onView(withId(R.id.pocketpaint_fill_tool_dialog_color_tolerance_input)).perform(replaceText("100"))
onToolBarView()
.performCloseToolOptionsView()
onDrawingSurfaceView()
.perform(UiInteractions.touchAt(DrawingSurfaceLocationProvider.MIDDLE))
.perform(touchAt(DrawingSurfaceLocationProvider.MIDDLE))
onDrawingSurfaceView()
.checkPixelColorResource(R.color.pocketpaint_color_picker_brown2, BitmapLocationProvider.MIDDLE)
.checkPixelColorResource(R.color.pocketpaint_color_picker_brown2, BitmapLocationProvider.HALFWAY_RIGHT_MIDDLE)
Expand All @@ -225,6 +233,38 @@ class FillToolIntegrationTest {
.checkPixelColorResource(R.color.pocketpaint_color_picker_brown2, BitmapLocationProvider.HALFWAY_RIGHT_MIDDLE)
}

@Test
fun testFillBitmapAndEraseASpotAndFillTheErasedSpotAgain() {
onToolProperties()
.checkMatchesColor(Color.BLACK)
onDrawingSurfaceView()
.perform(touchAt(DrawingSurfaceLocationProvider.MIDDLE))
onDrawingSurfaceView()
.checkPixelColor(Color.BLACK, BitmapLocationProvider.MIDDLE)
onToolBarView()
.performSelectTool(ToolType.ERASER)
onDrawingSurfaceView()
.perform(
swipeAccurate(
DrawingSurfaceLocationProvider.HALFWAY_TOP_MIDDLE,
DrawingSurfaceLocationProvider.HALFWAY_BOTTOM_MIDDLE
)
)
onView(isRoot()).perform(waitFor(1000))
onDrawingSurfaceView()
.checkPixelColor(Color.TRANSPARENT, BitmapLocationProvider.MIDDLE)
onView(isRoot()).perform(waitFor(1000))
onToolBarView()
.performSelectTool(ToolType.FILL)
onToolProperties()
.checkMatchesColor(Color.BLACK)
onDrawingSurfaceView()
.perform(touchAt(DrawingSurfaceLocationProvider.HALFWAY_BOTTOM_MIDDLE))
onView(isRoot()).perform(waitFor(1000))
onDrawingSurfaceView()
.checkPixelColor(Color.BLACK, BitmapLocationProvider.MIDDLE)
}

companion object {
private const val TOLERANCE_DELTA = 0.05
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class FillAlgorithmTest {
assertEquals("Wrong array size", height, algorithmPixels.size)
assertEquals("Wrong array size", width, algorithmPixels[0].size)
val algorithmTargetColor = fillAlgorithm.targetColor
val algorithmReplacementColor = fillAlgorithm.replacementColor
val algorithmReplacementColor = fillAlgorithm.colorToBeReplaced
val algorithmColorTolerance = fillAlgorithm.colorToleranceThresholdSquared.toFloat()
assertEquals(
"Wrong target color",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ class FillCommand(private val fillAlgorithmFactory: FillAlgorithmFactory, clicke
override fun run(canvas: Canvas, layerModel: LayerContracts.Model) {
val currentLayer = layerModel.currentLayer
currentLayer ?: return
currentLayer.bitmap?.let { bitmap ->
val replacementColor = bitmap.getPixel(clickedPixel.x, clickedPixel.y)
currentLayer.bitmap.let { bitmap ->
val colorToBeReplaced = bitmap.getPixel(clickedPixel.x, clickedPixel.y)
val fillAlgorithm = fillAlgorithmFactory.createFillAlgorithm()
fillAlgorithm.setParameters(
bitmap,
clickedPixel,
paint.color,
replacementColor,
colorTolerance
bitmap,
clickedPixel,
paint.color,
colorToBeReplaced,
colorTolerance
)
fillAlgorithm.performFilling()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,8 @@ interface MainActivityContracts {

fun showCurrentTool(toolType: ToolType?)

fun enableColorItemView(show: Boolean)

fun setColorButtonColor(@ColorInt color: Int)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import org.catrobat.paintroid.tools.ToolReference
import org.catrobat.paintroid.tools.ToolType
import org.catrobat.paintroid.tools.Workspace
import org.catrobat.paintroid.tools.implementation.ClippingTool
import org.catrobat.paintroid.tools.implementation.EraserTool
import org.catrobat.paintroid.tools.implementation.ImportTool
import org.catrobat.paintroid.tools.implementation.LineTool
import org.catrobat.paintroid.tools.implementation.SprayTool
Expand Down Expand Up @@ -130,6 +131,9 @@ class DefaultToolController(
private fun switchTool(tool: Tool) {
val currentTool = toolReference.tool
val currentToolType = currentTool?.toolType
if (currentToolType == ToolType.ERASER) {
(currentTool as EraserTool).setSavedColor()
}
currentToolType?.let { hidePlusIfShown(it) }

if (currentToolType == tool.toolType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class JavaFillAlgorithm : FillAlgorithm {
var targetColor = 0

@VisibleForTesting
var replacementColor = 0
var colorToBeReplaced = 0

@VisibleForTesting
var colorToleranceThresholdSquared = 0
Expand Down Expand Up @@ -70,7 +70,7 @@ class JavaFillAlgorithm : FillAlgorithm {
filledPixels = Array(bitmap.height) { BooleanArray(bitmap.width) }
this.clickedPixel = clickedPixel
this.targetColor = targetColor
this.replacementColor = replacementColor
this.colorToBeReplaced = replacementColor
colorToleranceThresholdSquared = square(colorToleranceThreshold.toInt())
considerTolerance = colorToleranceThreshold > 0
}
Expand All @@ -97,9 +97,9 @@ class JavaFillAlgorithm : FillAlgorithm {

private fun shouldCellBeFilled(row: Int, col: Int): Boolean =
!filledPixels[row][col] && (
pixels[row][col] == replacementColor || considerTolerance && isPixelWithinColorTolerance(
pixels[row][col] == colorToBeReplaced || considerTolerance && isPixelWithinColorTolerance(
pixels[row][col],
replacementColor
colorToBeReplaced
)
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class DefaultToolFactory(mainActivity: MainActivity) : ToolFactory {
workspace,
idlingResource,
commandManager,
mainActivity.bottomNavigationViewHolder,
DRAW_TIME_INIT
)
ToolType.LINE -> LineTool(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,18 @@ import org.catrobat.paintroid.tools.ToolType
import org.catrobat.paintroid.tools.Workspace
import org.catrobat.paintroid.tools.options.BrushToolOptionsView
import org.catrobat.paintroid.tools.options.ToolOptionsViewController
import org.catrobat.paintroid.ui.viewholder.BottomNavigationViewHolder

class EraserTool(
brushToolOptionsView: BrushToolOptionsView,
contextCallback: ContextCallback,
toolOptionsViewController: ToolOptionsViewController,
toolPaint: ToolPaint,
workspace: Workspace,
idlingResource: CountingIdlingResource,
commandManager: CommandManager,
drawTime: Long
brushToolOptionsView: BrushToolOptionsView,
contextCallback: ContextCallback,
toolOptionsViewController: ToolOptionsViewController,
toolPaint: ToolPaint,
workspace: Workspace,
idlingResource: CountingIdlingResource,
commandManager: CommandManager,
bottomNavigationViewHolder: BottomNavigationViewHolder,
drawTime: Long
) : BrushTool(
brushToolOptionsView,
contextCallback,
Expand All @@ -48,10 +50,22 @@ class EraserTool(
commandManager,
drawTime
) {

private var savedColor: Int
private var bottomNavigationViewHolder: BottomNavigationViewHolder

init {
this.bottomNavigationViewHolder = bottomNavigationViewHolder
bottomNavigationViewHolder.enableColorItemView(false)
bottomNavigationViewHolder.setColorButtonColor(Color.TRANSPARENT)
savedColor = toolPaint.color
toolPaint.color = Color.TRANSPARENT
brushToolOptionsView.setCurrentPaint(toolPaint.paint)
}
override val previewPaint: Paint
get() = Paint().apply {
set(super.previewPaint)
color = Color.BLACK
color = Color.TRANSPARENT
shader = toolPaint.checkeredShader
}

Expand All @@ -64,4 +78,11 @@ class EraserTool(

override val toolType: ToolType
get() = ToolType.ERASER

fun setSavedColor() {
bottomNavigationViewHolder.enableColorItemView(true)
bottomNavigationViewHolder.setColorButtonColor(savedColor)
toolPaint.color = savedColor
brushToolOptionsView.setCurrentPaint(toolPaint.paint)
}
}
Loading

0 comments on commit a044cad

Please sign in to comment.