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.

Sign up for GitHub

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

Jump to bottom

PAINTROID-291: Color changes cannot be undone or redone #1044

Merged
merged 2 commits into from
Jun 6, 2022
Merged

PAINTROID-291: Color changes cannot be undone or redone #1044

merged 2 commits into from
Jun 6, 2022

Conversation

JulianJautz
Copy link
Contributor

@JulianJautz JulianJautz commented Mar 9, 2022 •
edited
Loading

color changes weren't considered as commands and not added to CommandManager

  • implemented a ColorChangedCommand
  • ColorChangedCommand is added to CommandManager when color is selected in ColorPickerDialog
  • adapted undo/redo behaviour in line tool to work with ColorCommands

(https://jira.catrob.at/browse/PAINTROID-291)

Your checklist for this pull request

Please review the contributing guidelines and wiki pages of this repository.

  • Include the name of the Jira ticket in the PR’s title
  • Include a summary of the changes plus the relevant context
  • Choose the proper base branch (develop)
  • Confirm that the changes follow the project’s coding guidelines
  • Verify that the changes generate no compiler or linter warnings
  • Perform a self-review of the changes
  • Verify to commit no other files than the intentionally changed ones
  • Include reasonable and readable tests verifying the added or changed behavior
  • Confirm that new and existing unit tests pass locally
  • Check that the commits’ message style matches the project’s guideline
  • Stick to the project’s gitflow workflow
  • Verify that your changes do not have any conflicts with the base branch
  • After the PR, verify that all CI checks have passed
  • Post a message in the #paintroid Slack channel and ask for a code reviewer

@JulianJautz JulianJautz force-pushed the PAINTROID-291_undo_for_color_changes branch 2 times, most recently from 66b8024 to 3dc10b5 Compare March 10, 2022 21:36
illo1311
illo1311 requested changes Mar 23, 2022
View reviewed changes
Copy link
Contributor

@illo1311 illo1311 left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but please resolve the merge conflict :)

illo1311
illo1311 approved these changes Apr 26, 2022
View reviewed changes
Copy link
Contributor

@illo1311 illo1311 left a comment

Choose a reason for hiding this comment

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

lgtm

wslany
wslany requested changes May 7, 2022
View reviewed changes
Copy link
Member

@wslany wslany left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I found several crashes, as follows:

  • On Android 8 to Android 12, when the color is changed at least once, then when I save the image in the .catrobat-image format, there is an error while saving shown on the screen (this error only appears when there are color changes --- if the color stays the black one, then it works fine; also when it is a different format like png), as follows:
    image

The catrobat-image file is nevertheless written to the file system, but when I try to open that image using the "Load image" option of Paintroid, the app crashes with the following stack trace:

05-07 18:06:03.916 E/AndroidRuntime( 1746): FATAL EXCEPTION: DefaultDispatcher-worker-3
05-07 18:06:03.916 E/AndroidRuntime( 1746): FATAL EXCEPTION: DefaultDispatcher-worker-3
05-07 18:06:03.916 E/AndroidRuntime( 1746): Process: org.catrobat.paintroid.independent_20220506_1943, PID: 1746
05-07 18:06:03.916 E/AndroidRuntime( 1746): com.esotericsoftware.kryo.KryoException: Buffer underflow.
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.io.Input.require(Input.java:219)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.io.Input.readVarInt(Input.java:407)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.util.DefaultClassResolver.readClass(DefaultClassResolver.java:145)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readClass(Kryo.java:758)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:869)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.readCurrentVersion(CommandManagerModelSerializer.kt:47)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.readCurrentVersion(CommandManagerModelSerializer.kt:27)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV3(VersionSerializer.kt:53)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV2(VersionSerializer.kt:50)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV1(VersionSerializer.kt:47)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.handleVersions(VersionSerializer.kt:37)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.read(CommandManagerModelSerializer.kt:39)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.read(CommandManagerModelSerializer.kt:27)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readObject(Kryo.java:777)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandSerializationUtilities.readFromFile(CommandSerializationUtilities.kt:194)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage.getBitmapReturnValue(LoadImage.kt:65)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage.access$getBitmapReturnValue(LoadImage.kt:37)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage$execute$1.invokeSuspend(LoadImage.kt:99)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
05-07 18:06:03.916 E/AndroidRuntime( 1746): Process: org.catrobat.paintroid.independent_20220506_1943, PID: 1746
05-07 18:06:03.916 E/AndroidRuntime( 1746): com.esotericsoftware.kryo.KryoException: Buffer underflow.
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.io.Input.require(Input.java:219)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.io.Input.readVarInt(Input.java:407)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.util.DefaultClassResolver.readClass(DefaultClassResolver.java:145)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readClass(Kryo.java:758)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:869)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.readCurrentVersion(CommandManagerModelSerializer.kt:47)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.readCurrentVersion(CommandManagerModelSerializer.kt:27)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV3(VersionSerializer.kt:53)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV2(VersionSerializer.kt:50)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV1(VersionSerializer.kt:47)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.handleVersions(VersionSerializer.kt:37)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.read(CommandManagerModelSerializer.kt:39)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.read(CommandManagerModelSerializer.kt:27)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readObject(Kryo.java:777)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandSerializationUtilities.readFromFile(CommandSerializationUtilities.kt:194)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage.getBitmapReturnValue(LoadImage.kt:65)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage.access$getBitmapReturnValue(LoadImage.kt:37)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage$execute$1.invokeSuspend(LoadImage.kt:99)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
05-07 18:06:03.916 E/AndroidRuntime( 1746): Process: org.catrobat.paintroid.independent_20220506_1943, PID: 1746
05-07 18:06:03.916 E/AndroidRuntime( 1746): com.esotericsoftware.kryo.KryoException: Buffer underflow.
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.io.Input.require(Input.java:219)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.io.Input.readVarInt(Input.java:407)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.util.DefaultClassResolver.readClass(DefaultClassResolver.java:145)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readClass(Kryo.java:758)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:869)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.readCurrentVersion(CommandManagerModelSerializer.kt:47)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.readCurrentVersion(CommandManagerModelSerializer.kt:27)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV3(VersionSerializer.kt:53)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV2(VersionSerializer.kt:50)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV1(VersionSerializer.kt:47)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.handleVersions(VersionSerializer.kt:37)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.read(CommandManagerModelSerializer.kt:39)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.read(CommandManagerModelSerializer.kt:27)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readObject(Kryo.java:777)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandSerializationUtilities.readFromFile(CommandSerializationUtilities.kt:194)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage.getBitmapReturnValue(LoadImage.kt:65)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage.access$getBitmapReturnValue(LoadImage.kt:37)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage$execute$1.invokeSuspend(LoadImage.kt:99)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
05-07 18:06:03.916 E/AndroidRuntime( 1746): Process: org.catrobat.paintroid.independent_20220506_1943, PID: 1746
05-07 18:06:03.916 E/AndroidRuntime( 1746): com.esotericsoftware.kryo.KryoException: Buffer underflow.
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.io.Input.require(Input.java:219)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.io.Input.readVarInt(Input.java:407)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.util.DefaultClassResolver.readClass(DefaultClassResolver.java:145)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readClass(Kryo.java:758)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:869)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.readCurrentVersion(CommandManagerModelSerializer.kt:47)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.readCurrentVersion(CommandManagerModelSerializer.kt:27)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV3(VersionSerializer.kt:53)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV2(VersionSerializer.kt:50)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.readV1(VersionSerializer.kt:47)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.VersionSerializer.handleVersions(VersionSerializer.kt:37)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.read(CommandManagerModelSerializer.kt:39)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandManagerModelSerializer.read(CommandManagerModelSerializer.kt:27)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at com.esotericsoftware.kryo.Kryo.readObject(Kryo.java:777)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.command.serialization.CommandSerializationUtilities.readFromFile(CommandSerializationUtilities.kt:194)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage.getBitmapReturnValue(LoadImage.kt:65)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage.access$getBitmapReturnValue(LoadImage.kt:37)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at org.catrobat.paintroid.iotasks.LoadImage$execute$1.invokeSuspend(LoadImage.kt:99)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
05-07 18:06:03.916 E/AndroidRuntime( 1746): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
  • On Android 6, 7 and 7.1, the behavior is different: when I tap on the color button, then select a color, and tap on "Apply" in the color chooser, then the app immediately crashes with the following stack trace:
05-07 18:12:13.450 E/ClipboardServiceEx(16083): clipEx is android.sec.clipboard.ClipboardExManager@e116d5e
05-07 18:12:13.450 E/ClipboardServiceEx(16083): clipEx has text data : false
05-07 18:12:19.650 E/ViewRootImpl(16083): sendUserActionEvent() mView == null
05-07 18:12:44.800 E/AndroidRuntime(16083): FATAL EXCEPTION: DefaultDispatcher-worker-2
05-07 18:12:44.800 E/AndroidRuntime(16083): Process: org.catrobat.paintroid.independent_20220506_1943, PID: 16083
05-07 18:12:44.800 E/AndroidRuntime(16083): android.view.ViewRootImpl$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.ViewRootImpl.checkThread(ViewRootImpl.java:8134)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.ViewRootImpl.invalidateChildInParent(ViewRootImpl.java:1257)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.ViewGroup.invalidateChild(ViewGroup.java:5428)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.View.invalidateInternal(View.java:13927)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.View.invalidate(View.java:13891)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.View.invalidate(View.java:13875)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.ui.tools.DefaultBrushToolOptionsView.invalidate(DefaultBrushToolOptionsView.kt:143)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.tools.implementation.BrushTool.changePaintColor(BrushTool.kt:158)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.command.implementation.ColorChangedCommand.run(ColorChangedCommand.kt:38)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.command.implementation.DefaultCommandManager.executeCommand(DefaultCommandManager.kt:85)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.command.implementation.DefaultCommandManager.addCommand(DefaultCommandManager.kt:77)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.command.implementation.AsyncCommandManager$addCommand$1.invokeSuspend(AsyncCommandManager.kt:65)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
05-07 18:12:44.800 E/ViewRootImpl(16083): sendUserActionEvent() mView == null
05-07 18:11:59.240 E/Zygote  (16083): v2
05-07 18:11:59.250 E/Zygote  (16083): accessInfo : 0
05-07 18:11:59.980 E/MotionRecognitionManager(16083): mSContextService = android.hardware.scontext.ISContextService$Stub$Proxy@bf1e03b
05-07 18:11:59.980 E/MotionRecognitionManager(16083): motionService = com.samsung.android.motion.IMotionRecognitionService$Stub$Proxy@f0a358
05-07 18:11:59.980 E/MotionRecognitionManager(16083): motionService = com.samsung.android.motion.IMotionRecognitionService$Stub$Proxy@f0a358
05-07 18:12:00.470 E/ClipboardServiceEx(16083): clipEx is android.sec.clipboard.ClipboardExManager@e116d5e
05-07 18:12:00.470 E/ClipboardServiceEx(16083): clipEx has text data : false
05-07 18:12:13.450 E/ClipboardServiceEx(16083): clipEx is android.sec.clipboard.ClipboardExManager@e116d5e
05-07 18:12:13.450 E/ClipboardServiceEx(16083): clipEx has text data : false
05-07 18:12:19.650 E/ViewRootImpl(16083): sendUserActionEvent() mView == null
05-07 18:12:44.800 E/AndroidRuntime(16083): FATAL EXCEPTION: DefaultDispatcher-worker-2
05-07 18:12:44.800 E/AndroidRuntime(16083): Process: org.catrobat.paintroid.independent_20220506_1943, PID: 16083
05-07 18:12:44.800 E/AndroidRuntime(16083): android.view.ViewRootImpl$CalledFromWrongThreadException: Only the original thread that created a view hierarchy can touch its views.
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.ViewRootImpl.checkThread(ViewRootImpl.java:8134)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.ViewRootImpl.invalidateChildInParent(ViewRootImpl.java:1257)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.ViewGroup.invalidateChild(ViewGroup.java:5428)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.View.invalidateInternal(View.java:13927)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.View.invalidate(View.java:13891)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at android.view.View.invalidate(View.java:13875)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.ui.tools.DefaultBrushToolOptionsView.invalidate(DefaultBrushToolOptionsView.kt:143)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.tools.implementation.BrushTool.changePaintColor(BrushTool.kt:158)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.command.implementation.ColorChangedCommand.run(ColorChangedCommand.kt:38)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.command.implementation.DefaultCommandManager.executeCommand(DefaultCommandManager.kt:85)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.command.implementation.DefaultCommandManager.addCommand(DefaultCommandManager.kt:77)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at org.catrobat.paintroid.command.implementation.AsyncCommandManager$addCommand$1.invokeSuspend(AsyncCommandManager.kt:65)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
05-07 18:12:44.800 E/AndroidRuntime(16083): 	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
05-07 18:12:44.800 E/ViewRootImpl(16083): sendUserActionEvent() mView == null

@JulianJautz JulianJautz force-pushed the PAINTROID-291_undo_for_color_changes branch from 134ef16 to 6b6283d Compare May 11, 2022 16:42
@JulianJautz
Copy link
Contributor Author

The issue for the crashes on versions 6, 7, 7.1 was that the changes have to be done by the UI-Thread and not by a background thread. This is done now.

The catrobat file saving crashed, because there was no Serializer for the ColorChangedCommand. As I need the main activity in the command I don't know how to serialize this. So I just excluded the ColorChangedCommands now from the file. Which means that after loading the file previous color changes cannot be undone.
I guess one option would be to add the context to the run method of every command? But maybe there is also a way to serialize the context?

@JulianJautz JulianJautz requested a review from wslany May 11, 2022 16:50
@JulianJautz JulianJautz force-pushed the PAINTROID-291_undo_for_color_changes branch 2 times, most recently from 4230a3e to 46a3588 Compare May 16, 2022 16:13
FromG
FromG requested changes May 19, 2022
View reviewed changes
...rc/androidTest/java/org/catrobat/paintroid/test/junit/command/DefaultCommandFactoryTest.java Outdated
Comment on lines 145 to 151
@Test
public void testCreateColorChangedCommand() {
ToolReference toolReference = new DefaultToolReference();
MockitoAnnotations.initMocks(this);
Command command = commandFactory.createColorChangedCommand(toolReference, bottomNavigationViewHolder, Color.WHITE);
assertThat(command, is(instanceOf(ColorChangedCommand.class)));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this test seems to fail.
createColorChangedCommand wants a context as second parameter.
Therefore jenkins can't run the ticket.

@JulianJautz JulianJautz force-pushed the PAINTROID-291_undo_for_color_changes branch from 46a3588 to c182042 Compare May 20, 2022 13:56
Julian Raphael Jautz added 2 commits May 20, 2022 17:04
PAINTROID-291: color changes cannot be un/redone
9afc5a8 
color changes weren't considered as commands and not added to CommandManager

* implemented a ColorChangedCommand

* ColorChangedCommand is added to CommandManager when color is selected in ColorPickerDialog

* adapted undo/redo behaviour in line tool to work with ColorCommands

* ColorChangedCommmand are not saved in catrobat file

*access to UI have to be done in main thread -> crashes on Android Versions 6, 7 and 7.1
Merge branch 'develop' of https://github.com/Catrobat/Paintroid into …
c74aa88 
…PAINTROID-291_undo_for_color_changes

� Conflicts:
�	Paintroid/src/main/java/org/catrobat/paintroid/command/implementation/DefaultCommandManager.kt
@JulianJautz JulianJautz force-pushed the PAINTROID-291_undo_for_color_changes branch from c182042 to c74aa88 Compare May 20, 2022 15:11
FromG
FromG approved these changes May 31, 2022
View reviewed changes
Copy link
Contributor

@FromG FromG left a comment

Choose a reason for hiding this comment

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

everything should work.

@wslany
Copy link
Member

wslany commented Jun 6, 2022

@AneiMakovec wrote:

I took a look at PAINTROID-291 and tested the code. I used multiple different tools while changing the colors and the undo and redo operations performed as expected for both changing the color with the color picker or with the pipette in the color picker. The only thing that I noticed was that when using the pipette tool (the standalone tool, not the one in color picker) the color changes were not remembered as a command. I don't know if this is really an issue, since the ticket description mentions only the color picker and its pipette. As for the tests, I ran them locally and they passed. I also checked the test code and it seems to test multiple use cases, but I didn't find any tests regarding the pipette in the color picker. I don't know if we can even test the pipette, so I'm not sure if this is an issue either.

Thanks Anei for doing the QA check. I think you are right, it would have been better if the color-checkpoints of the pipette tool were also included. Having said that, the fix covers a large amount of the original ticket, so it now works fine in most situations. Also, regarding tests for the pipette in the color picker, it is true but at the same time it is nearly impossible to test all possible scenarios, so I guess the current test coverage is good enough. Another consideration is that @JulianJautz who implemented the fix is currently not with the team and has no time to further optimize the implementation, so based on all this I am now merging it.

Thanks everyone who was involved in fixing and giving feedback and testing this PR!

@wslany wslany merged commit 5a2c023 into Catrobat:develop Jun 6, 2022
goschale added a commit to goschale/Paintroid that referenced this pull request Jul 5, 2022
@goschale
Squashed commit of the following:
dc28273 
commit bf26e71
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Mon Jul 4 13:26:03 2022 +0200

    added test and removed unnecessary stuff

commit 7b621ff
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Mon Jul 4 13:15:59 2022 +0200

    path correction

commit 84be6c2
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Mon Jul 4 13:05:56 2022 +0200

    updated to debian:bullsyeve, latest ruby version and fastlane

commit 0f83cef
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 24 10:59:10 2022 +0200

    dockerfile changed to docker hub download

commit c317049
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 24 10:51:02 2022 +0200

    update jenkinsfile.releaseAPK

commit 56bba15
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 24 10:38:48 2022 +0200

    logcat test

commit 0fc9f4f
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 13:23:15 2022 +0200

    root dir

commit ab9fc7f
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 13:06:47 2022 +0200

    touch file

commit 280c329
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 12:51:44 2022 +0200

    changed path, again

commit 9405962
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 12:12:40 2022 +0200

    corrected path

commit 9d32622
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 12:06:46 2022 +0200

    txt not text

commit da3f69a
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 11:01:18 2022 +0200

    logcat path reduced

commit c29aed2
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 10:43:06 2022 +0200

    changed log to post action

commit 1c7b343
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 10:41:44 2022 +0200

    failing tests removed

commit 6af0b73
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 10:23:17 2022 +0200

    path correction Paintroid directory

commit 5dd7aa7
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 10:22:08 2022 +0200

    file creation and move changed

commit 205af86
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 10:18:18 2022 +0200

    Revert "removed UI tests for better building"

    This reverts commit 93c7493.

commit 4068ddc
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 10:17:29 2022 +0200

    Revert "Revert "remove failing tests for pipeline test""

    This reverts commit 31d759c.

commit 31d759c
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 10:16:25 2022 +0200

    Revert "remove failing tests for pipeline test"

    This reverts commit fc18989.

commit 93c7493
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 09:52:40 2022 +0200

    removed UI tests for better building

commit bc950ff
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Thu Jun 23 09:04:10 2022 +0200

    corrected path

commit 07cea6e
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Wed Jun 22 12:37:06 2022 +0200

    removed failing test and changes path of logcat

commit 92f9a06
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Wed Jun 22 11:48:24 2022 +0200

    added logcat output

commit fc18989
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Tue Jun 14 11:24:01 2022 +0200

    remove failing tests for pipeline test

commit 62af1df
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Mon Jun 13 22:07:00 2022 +0200

    deactivated failing tests and version var for docker

commit 0481d83
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 10 15:13:34 2022 +0200

    changes emulator call for background execution

commit 4d4d534
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 10 12:28:06 2022 +0200

    a point too much

commit 1f3dd47
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 10 12:20:10 2022 +0200

    error for mainactivity changed

commit e2fc817
Merge: 3380702 240adfb
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 10 12:06:16 2022 +0200

    Merge remote-tracking branch 'tobstream/PAINTROID-221_migrate_away_from_jcenter' into DEVOPS-686

commit 3380702
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 10 12:02:02 2022 +0200

    tobstream add

commit 5da9bb4
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 10 11:48:23 2022 +0200

    JenkinsFile Backslashed

commit fc18641
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri Jun 10 11:34:52 2022 +0200

    dockerfile update and Jenkinsfile skip gradle plugin

commit 240adfb
Author: Tobias Stangl <tobias.stangl@student.tugraz.at>
Date:   Wed May 25 11:16:57 2022 +0200

    PAINTROID-221 migrate away from JCenter

commit 5a2c023
Author: Julian Jautz <63667997+JulianJautz@users.noreply.github.com>
Date:   Mon Jun 6 15:27:08 2022 +0200

    PAINTROID-291: color changes cannot be un/redone (Catrobat#1044)

    * color changes weren't considered as commands and not added to CommandManager
    * implemented a ColorChangedCommand
    * ColorChangedCommand is added to CommandManager when color is selected in ColorPickerDialog
    * adapted undo/redo behaviour in line tool to work with ColorCommands
    * ColorChangedCommmand are not saved in catrobat file
    *access to UI have to be done in main thread -> crashes on Android Versions 6, 7 and 7.1

    Co-authored-by: Julian Raphael Jautz <julian.jautz@student.tugraz.at>

commit dae5e52
Author: Wolfgang Slany <1747888+wslany@users.noreply.github.com>
Date:   Fri Jun 3 01:35:30 2022 +0200

    Version name and version code update for v2.9.1

commit 52af881
Merge: 17d8c9d f840b66
Author: Wolfgang Slany <1747888+wslany@users.noreply.github.com>
Date:   Fri Jun 3 01:29:33 2022 +0200

    Merge pull request Catrobat#1105 from Catrobat/l10n_colorpicker_crowdin_action

    New Colorpicker Crowdin translations by Github Action

commit 17d8c9d
Merge: 5b86b2f 49f264f
Author: Wolfgang Slany <1747888+wslany@users.noreply.github.com>
Date:   Fri Jun 3 01:26:00 2022 +0200

    Merge pull request Catrobat#1102 from Catrobat/l10n_crowdin_action

    New Crowdin translations by Github Action

commit 5b86b2f
Author: SebastianGrief <sebastian.grief@student.tugraz.at>
Date:   Fri Jun 3 00:53:32 2022 +0200

    PAINTROID-427: Very high frequency crash hotfix (Catrobat#1111)

    https://jira.catrob.at/browse/PAINTROID-427

    * PAINTROID-427 Very high frequency crash hotfix

commit 9a0d8f7
Merge: 537bd84 cd14f24
Author: Wolfgang Slany <1747888+wslany@users.noreply.github.com>
Date:   Thu Jun 2 12:53:45 2022 +0200

    Merge pull request Catrobat#1087 from Electronix1337/PAINTROID-415_keep_zoom_level_and_placement_after_applying_transform_tool

    PAINTROID-415 Keep zoom level and placement after applying the Transform tool

commit cd14f24
Author: Michael Tiefnig <michael.tiefnig@student.tugraz.at>
Date:   Mon May 9 14:35:29 2022 +0200

    PAINTROID-415

    Fixed bug where zoom level and placement were reset after clicking checkmark when using Transformtool. Now zoom level and placement stays the same when transforming via checkmark.

commit 537bd84
Merge: b3a4287 4f32aa7
Author: Wolfgang Slany <1747888+wslany@users.noreply.github.com>
Date:   Thu Jun 2 10:23:22 2022 +0200

    Merge pull request Catrobat#1103 from ThorstenBandel/PAINTROID-423_Fix_paintroid_filenames

    PAINTROID-423 Fix paintroid filenames

commit f840b66
Author: Crowdin Bot <support+bot@crowdin.com>
Date:   Thu Jun 2 02:04:58 2022 +0000

    New Crowdin translations by Github Action

commit 49f264f
Author: Crowdin Bot <support+bot@crowdin.com>
Date:   Thu Jun 2 00:52:53 2022 +0000

    New Crowdin translations by Github Action

commit 4f32aa7
Author: Thorsten Bandel <thorsten.bandel@student.tugraz.at>
Date:   Mon May 23 15:18:17 2022 +0200

    PAINTROID-423 Fix paintroid filenames

commit b3a4287
Merge: 8abd3d6 268a175
Author: ThorstenB <37836636+ThorstenBandel@users.noreply.github.com>
Date:   Tue May 31 23:09:53 2022 +0200

    Merge pull request Catrobat#1109 from CodeChamp-SS/PAINTROID-426

    PAINTROID-426 fixed very high frequency crash in the Smudge Tool

commit 268a175
Author: Saarthak Seth <saarthakseth@gmail.com>
Date:   Wed Jun 1 00:10:12 2022 +0530

    PAINTROID-426 fixed crash in the Smudge Tool and removed the git status operator from all other places as well

commit 3275f52
Author: Alexander Gosch <a.gosch@student.tugraz.at>
Date:   Fri May 27 09:17:18 2022 +0200

    added android command lines tools to dockerfile

commit 934b89a
Author: Tobias Stangl <tobias.stangl@student.tugraz.at>
Date:   Wed May 25 11:16:57 2022 +0200

    PAINTROID-221 migrate away from JCenter

commit 8abd3d6
Merge: a733d52 a75a804
Author: Wolfgang Slany <1747888+wslany@users.noreply.github.com>
Date:   Sun May 22 10:35:04 2022 +0200

    Version name and version code update for v2.9.0

commit a75a804
Author: Wolfgang Slany <1747888+wslany@users.noreply.github.com>
Date:   Sun May 22 08:24:54 2022 +0200

    Version name and version code update for v2.9.0
SebastianGrief pushed a commit to SebastianGrief/Paintroid that referenced this pull request Sep 12, 2022
@JulianJautz @SebastianGrief
PAINTROID-291: color changes cannot be un/redone (Catrobat#1044)
5692153 
* color changes weren't considered as commands and not added to CommandManager
* implemented a ColorChangedCommand
* ColorChangedCommand is added to CommandManager when color is selected in ColorPickerDialog
* adapted undo/redo behaviour in line tool to work with ColorCommands
* ColorChangedCommmand are not saved in catrobat file
*access to UI have to be done in main thread -> crashes on Android Versions 6, 7 and 7.1

Co-authored-by: Julian Raphael Jautz <julian.jautz@student.tugraz.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FromG FromG FromG approved these changes

@illo1311 illo1311 illo1311 approved these changes

@wslany wslany wslany approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JulianJautz @wslany @illo1311 @FromG