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 lines may visually 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 b515db3
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 16 deletions.
Original file line number Diff line number Diff line change
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,24 @@ 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) {
val oldLine = oldLines[oldItemPosition]
val newLine = newLines[newItemPosition]

oldLine.type == newLine.type &&
oldLine.timestamp == newLine.timestamp &&
oldLine.rawPrefix == newLine.rawPrefix &&
oldLine.rawMessage == newLine.rawMessage &&
oldLine.isHighlighted == newLine.isHighlighted &&
oldLine.displayAs == newLine.displayAs
} else {
true
}
return sameStyle
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions app/src/main/java/com/ubergeek42/WeechatAndroid/relay/Line.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ 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,
private val rawPrefix: String,
private val rawMessage: String,
@JvmField val rawPrefix: String,
@JvmField val rawMessage: String,
@JvmField val nick: String?,
@JvmField val isVisible: Boolean,
@JvmField val isHighlighted: Boolean,
Expand Down

0 comments on commit b515db3

Please sign in to comment.