Skip to content

Commit

Permalink
On lines listed, also diff line contents
Browse files Browse the repository at this point in the history
When buffer is cleared using `/buffer clear` and more lines are added,
line pointers may be reused. Before, if the app was disconnected during
these events, after reconnection it was possible for the lines to appear out
of order. This happened because normally we calculate diff using pointers
only, and if we had an older line with the same pointer as the newer one,
the adapter would reuse the old view holder, which might've been displaying
the wrong line.

This fixes the issue by also looking at line contents while diffing.

This solution is not ideal:

* While we can reliably tell whether the contents of two lines are the same,
  during the diffing we also want to know whether two lines represent the
  same thing, even if the contents have changed. As the pointer can be
  reused, sometimes `areItemsTheSame` will return a false positive. However,
  with this change the negative effect of this shouldn't go beyond wrong
  animation being applied.

* Read marker may appear in a wrong place. We save the pointer to the last
  read line locally, and if the pointer is reused, there's no way for us to
  tell that the line is wrong.

Fixes #577
  • Loading branch information
oakkitten committed Feb 23, 2024
1 parent add9984 commit 8a4c9d6
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 14 deletions.
Expand Up @@ -31,29 +31,28 @@ import com.ubergeek42.WeechatAndroid.Weechat
import com.ubergeek42.WeechatAndroid.copypaste.showCopyDialog
import com.ubergeek42.WeechatAndroid.relay.Buffer
import com.ubergeek42.WeechatAndroid.relay.BufferEye
import com.ubergeek42.WeechatAndroid.relay.HeaderLine
import com.ubergeek42.WeechatAndroid.relay.Line
import com.ubergeek42.WeechatAndroid.relay.LineSpec
import com.ubergeek42.WeechatAndroid.relay.Lines
import com.ubergeek42.WeechatAndroid.relay.MarkerLine
import com.ubergeek42.WeechatAndroid.relay.SquiggleLine
import com.ubergeek42.WeechatAndroid.relay.HeaderLine
import com.ubergeek42.WeechatAndroid.search.Search
import com.ubergeek42.WeechatAndroid.service.P
import com.ubergeek42.WeechatAndroid.upload.i
import com.ubergeek42.WeechatAndroid.upload.main
import com.ubergeek42.WeechatAndroid.views.AnimatedRecyclerView
import com.ubergeek42.WeechatAndroid.views.Animation
import com.ubergeek42.WeechatAndroid.utils.Toaster
import com.ubergeek42.WeechatAndroid.utils.forEachReversedIndexed
import com.ubergeek42.WeechatAndroid.utils.isAnyOf
import com.ubergeek42.WeechatAndroid.utils.ulet
import com.ubergeek42.WeechatAndroid.views.AnimatedRecyclerView
import com.ubergeek42.WeechatAndroid.views.Animation
import com.ubergeek42.WeechatAndroid.views.LineView
import com.ubergeek42.WeechatAndroid.views.solidColor
import com.ubergeek42.WeechatAndroid.views.updateMargins
import com.ubergeek42.cats.Kitty
import com.ubergeek42.cats.Root
import com.ubergeek42.weechat.ColorScheme
import java.util.*


class ChatLinesAdapter @MainThread constructor(
Expand Down Expand Up @@ -218,13 +217,17 @@ class ChatLinesAdapter @MainThread constructor(
private var updateStep = 0
private var style = 0

@AnyThread @Synchronized private fun onLinesChanged(animation: Animation) = ulet(buffer) { buffer ->
@AnyThread @Synchronized private fun onLinesChanged(
animation: Animation = Animation.Default,
diffLineContents: Boolean = false
) = ulet(buffer) { buffer ->
val thisUpdateStep = synchronized (updateLock) { ++updateStep }

val newLines = buffer.getLinesCopy()
val newStyle = buffer.style // todo synchronization?

val diffResult = DiffUtil.calculateDiff(DiffCallback(lines, newLines, style == newStyle), false)
val diffCallback = DiffCallback(lines, newLines, style == newStyle, diffLineContents)
val diffResult = DiffUtil.calculateDiff(diffCallback, false)

Weechat.runOnMainThreadASAP {
synchronized (updateLock) {
Expand Down Expand Up @@ -253,27 +256,27 @@ class ChatLinesAdapter @MainThread constructor(
////////////////////////////////////////////////////////////////////////////////////////////////

@MainThread @Synchronized override fun onGlobalPreferencesChanged(numberChanged: Boolean) {
onLinesChanged(Animation.Default)
onLinesChanged()
}

@WorkerThread override fun onLinesListed() {
onLinesChanged(Animation.NewLinesFetched)
onLinesChanged(Animation.NewLinesFetched, diffLineContents = true)
}

@AnyThread override fun onLineAdded() {
onLinesChanged(Animation.LastLineAdded)
}

@WorkerThread override fun onTitleChanged() {
onLinesChanged(Animation.Default)
onLinesChanged()
}

@WorkerThread override fun onBufferClosed() {}

////////////////////////////////////////////////////////////////////////////////////////////////

@MainThread fun loadLinesWithoutAnimation() {
onLinesChanged(Animation.None)
onLinesChanged(Animation.None, diffLineContents = true)
}

@MainThread @Synchronized fun loadLinesSilently() = ulet(buffer) { buffer ->
Expand Down Expand Up @@ -323,6 +326,7 @@ class ChatLinesAdapter @MainThread constructor(
private val oldLines: List<Line>,
private val newLines: List<Line>,
private val sameStyle: Boolean,
private val diffLineContents: Boolean,
) : DiffUtil.Callback() {
override fun getOldListSize() = oldLines.size
override fun getNewListSize() = newLines.size
Expand All @@ -332,10 +336,16 @@ class ChatLinesAdapter @MainThread constructor(
}

override fun areContentsTheSame(oldItemPosition: Int, newItemPosition: Int): Boolean {
if (newItemPosition == 0) {
return oldLines[oldItemPosition] === newLines[newItemPosition]
return if (newItemPosition == 0) {
// Header line, always present
oldLines[oldItemPosition] === newLines[newItemPosition]
} else if (!sameStyle) {
false
} else if (diffLineContents) {
oldLines[oldItemPosition].visuallyEqualsTo(newLines[newItemPosition])
} else {
true
}
return sameStyle
}
}
}
Expand Down
10 changes: 9 additions & 1 deletion app/src/main/java/com/ubergeek42/WeechatAndroid/relay/Line.kt
Expand Up @@ -21,7 +21,7 @@ import com.ubergeek42.weechat.Color
import com.ubergeek42.weechat.ColorScheme


open class Line constructor(
open class Line(
@JvmField val pointer: Long,
@JvmField val type: LineSpec.Type,
@JvmField val timestamp: Long,
Expand Down Expand Up @@ -103,5 +103,13 @@ open class Line constructor(
val timestampedIrcLikeString: String get() =
timestampString?.let { timestamp -> "$timestamp $ircLikeString" } ?: ircLikeString

fun visuallyEqualsTo(other: Line) =
type == other.type &&
timestamp == other.timestamp &&
rawPrefix == other.rawPrefix &&
rawMessage == other.rawMessage &&
isHighlighted == other.isHighlighted &&
displayAs == other.displayAs

override fun toString() = "Line(${pointer.as0x}): $ircLikeString)"
}

0 comments on commit 8a4c9d6

Please sign in to comment.