From c11ea868fe88ad2dcd142c4b07a74ea14a054519 Mon Sep 17 00:00:00 2001 From: dece Date: Fri, 10 Dec 2021 23:50:18 +0100 Subject: [PATCH] MainActivity: rework layout to avoid lag ScrollView + RecyclerView lead to very slow loading times for the recycler, like a few seconds on the biggest Medusae pages. Turns out it binds every ViewHolder instantly, losing all the recycling behavior! Following some guidelines on StackOverflow fixed this, because official docs could not. --- app/build.gradle | 2 + .../lowrespalmtree/comet/ContentRecycler.kt | 30 ++++- .../java/dev/lowrespalmtree/comet/Gemtext.kt | 1 + .../dev/lowrespalmtree/comet/MainActivity.kt | 105 ++++++++++++++---- .../java/dev/lowrespalmtree/comet/Request.kt | 1 + .../java/dev/lowrespalmtree/comet/Response.kt | 1 + app/src/main/res/layout/activity_main.xml | 38 +++++-- app/src/main/res/layout/gemtext_empty.xml | 4 +- 8 files changed, 144 insertions(+), 38 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index cf38a41..68ef747 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -25,6 +25,7 @@ android { release { minifyEnabled false proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro' + signingConfig signingConfigs.debug } } compileOptions { @@ -42,6 +43,7 @@ dependencies { implementation 'com.google.android.material:material:1.4.0' implementation 'androidx.constraintlayout:constraintlayout:2.1.2' implementation "androidx.lifecycle:lifecycle-viewmodel-ktx:2.4.0" + implementation 'androidx.legacy:legacy-support-v4:1.0.0' testImplementation 'junit:junit:4.13.2' androidTestImplementation 'androidx.test.ext:junit:1.1.3' androidTestImplementation 'androidx.test.espresso:espresso-core:3.4.0' diff --git a/app/src/main/java/dev/lowrespalmtree/comet/ContentRecycler.kt b/app/src/main/java/dev/lowrespalmtree/comet/ContentRecycler.kt index e500c26..e64b97f 100644 --- a/app/src/main/java/dev/lowrespalmtree/comet/ContentRecycler.kt +++ b/app/src/main/java/dev/lowrespalmtree/comet/ContentRecycler.kt @@ -1,5 +1,6 @@ package dev.lowrespalmtree.comet +import android.annotation.SuppressLint import android.text.SpannableString import android.text.style.UnderlineSpan import android.util.Log @@ -12,6 +13,8 @@ import dev.lowrespalmtree.comet.databinding.* class ContentAdapter(private var content: List, private val listener: ContentAdapterListen) : RecyclerView.Adapter() { + private var lastLineCount = 0 + interface ContentAdapterListen { fun onLinkClick(url: String) } @@ -35,7 +38,6 @@ class ContentAdapter(private var content: List, private val listener: Cont } override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ContentViewHolder { - Log.d(TAG, "onCreateViewHolder: type $viewType") return LayoutInflater.from(parent.context).let { when (viewType) { TYPE_EMPTY -> ContentViewHolder.Empty(GemtextEmptyBinding.inflate(it)) @@ -54,7 +56,6 @@ class ContentAdapter(private var content: List, private val listener: Cont } override fun onBindViewHolder(holder: ContentViewHolder, position: Int) { - Log.d(TAG, "onBindViewHolder: position $position") val line = content[position] when (holder) { is ContentViewHolder.Paragraph -> holder.binding.textView.text = @@ -75,9 +76,28 @@ class ContentAdapter(private var content: List, private val listener: Cont override fun getItemCount(): Int = content.size - fun setContent(content: List) { - this.content = content - notifyDataSetChanged() + /** + * Replace the content rendered by the recycler. + * + * The new content list may or may not be the same object as the previous one, we don't + * assume anything. The assumptions this function do however are: + * - If the new content is empty, we are about to load new content, so clear the recycler. + * - If it's longer than before, we received new streamed content, so *append* data. + * - If it's shorter or the same size than before, we do not notify anything and let the caller + * manage the changes itself. + */ + @SuppressLint("NotifyDataSetChanged") + fun setContent(newContent: List) { + content = newContent.toList() // Shallow copy to avoid concurrent update issues. + if (content.isEmpty()) { + Log.d(TAG, "setContent: empty content") + notifyDataSetChanged() + } else if (content.size > lastLineCount) { + val numAdded = content.size - lastLineCount + Log.d(TAG, "setContent: added $numAdded items") + notifyItemRangeInserted(lastLineCount, numAdded) + } + lastLineCount = content.size } sealed class ContentViewHolder(view: View) : RecyclerView.ViewHolder(view) { diff --git a/app/src/main/java/dev/lowrespalmtree/comet/Gemtext.kt b/app/src/main/java/dev/lowrespalmtree/comet/Gemtext.kt index 85f987e..ca5c557 100644 --- a/app/src/main/java/dev/lowrespalmtree/comet/Gemtext.kt +++ b/app/src/main/java/dev/lowrespalmtree/comet/Gemtext.kt @@ -41,6 +41,7 @@ fun parseData( channel.send(line) } } + channel.close() } return channel } diff --git a/app/src/main/java/dev/lowrespalmtree/comet/MainActivity.kt b/app/src/main/java/dev/lowrespalmtree/comet/MainActivity.kt index d01636f..a7c4306 100644 --- a/app/src/main/java/dev/lowrespalmtree/comet/MainActivity.kt +++ b/app/src/main/java/dev/lowrespalmtree/comet/MainActivity.kt @@ -2,6 +2,7 @@ package dev.lowrespalmtree.comet import android.annotation.SuppressLint import android.app.Activity +import android.content.ActivityNotFoundException import android.content.Intent import android.content.Intent.ACTION_VIEW import android.net.Uri @@ -18,6 +19,7 @@ import androidx.lifecycle.viewModelScope import androidx.recyclerview.widget.LinearLayoutManager import dev.lowrespalmtree.comet.databinding.ActivityMainBinding import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.launch import java.net.UnknownHostException import java.nio.charset.Charset @@ -27,6 +29,8 @@ class MainActivity : AppCompatActivity(), ContentAdapter.ContentAdapterListen { private lateinit var pageViewModel: PageViewModel private lateinit var adapter: ContentAdapter + private val currentUrl get() = binding.addressBar.text + @SuppressLint("SetTextI18n") override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -50,8 +54,11 @@ class MainActivity : AppCompatActivity(), ContentAdapter.ContentAdapterListen { } } - pageViewModel.linesLiveData.observe(this, { adapter.setContent(it) }) - pageViewModel.alertLiveData.observe(this, { alert(it) }) + binding.contentSwipeLayout.setOnRefreshListener { openUrl(currentUrl.toString()) } + + pageViewModel.state.observe(this, { updateState(it) }) + pageViewModel.lines.observe(this, { updateLines(it) }) + pageViewModel.alert.observe(this, { alert(it) }) } override fun onLinkClick(url: String) { @@ -63,29 +70,68 @@ class MainActivity : AppCompatActivity(), ContentAdapter.ContentAdapterListen { var uri = Uri.parse(url) if (!uri.isAbsolute) { uri = if (!base.isNullOrEmpty()) joinUrls(base, url) else toGeminiUri(uri) - Log.d(TAG, "openUrl: '$url' - '$base' - '$uri'") - Log.d(TAG, "openUrl: ${uri.authority} - ${uri.path} - ${uri.query}") - binding.addressBar.setText(uri.toString()) } when (uri.scheme) { - "gemini" -> pageViewModel.sendGeminiRequest(uri) - else -> startActivity(Intent(ACTION_VIEW, uri)) + "gemini" -> { + binding.addressBar.setText(uri.toString()) + pageViewModel.sendGeminiRequest(uri) + } + else -> openUnknownScheme(uri) } } - private fun alert(message: String) { - AlertDialog.Builder(this) - .setTitle(R.string.alert_title) + private fun updateState(state: PageViewModel.State) { + Log.d(TAG, "updateState: $state") + when (state) { + PageViewModel.State.IDLE -> { + binding.contentProgressBar.hide() + binding.contentSwipeLayout.isRefreshing = false + } + PageViewModel.State.CONNECTING -> { + binding.contentProgressBar.show() + } + PageViewModel.State.RECEIVING -> { + binding.contentSwipeLayout.isRefreshing = false + } + } + } + + private fun updateLines(lines: List) { + Log.d(TAG, "updateLines: ${lines.size} lines") + adapter.setContent(lines) + } + + private fun alert(message: String, title: String? = null) { + val builder = AlertDialog.Builder(this) + if (title != null) + builder.setTitle(title) + else + builder.setTitle(title ?: R.string.alert_title) + builder .setMessage(message) .create() .show() } + private fun openUnknownScheme(uri: Uri) { + try { + startActivity(Intent(ACTION_VIEW, uri)) + } catch (e: ActivityNotFoundException) { + alert("Can't open this URL.") + } + } + class PageViewModel : ViewModel() { - private var lines = ArrayList() - val linesLiveData: MutableLiveData> by lazy { MutableLiveData>() } - val alertLiveData: MutableLiveData by lazy { MutableLiveData() } + private var requestJob: Job? = null + val state: MutableLiveData by lazy { MutableLiveData(State.IDLE) } + private var linesList = ArrayList() + val lines: MutableLiveData> by lazy { MutableLiveData>() } + val alert: MutableLiveData by lazy { MutableLiveData() } + + enum class State { + IDLE, CONNECTING, RECEIVING + } /** * Perform a request against this URI. @@ -93,42 +139,57 @@ class MainActivity : AppCompatActivity(), ContentAdapter.ContentAdapterListen { * The URI must be valid, absolute and with a gemini scheme. */ fun sendGeminiRequest(uri: Uri) { - Log.d(TAG, "sendRequest: $uri") - viewModelScope.launch(Dispatchers.IO) { + Log.d(TAG, "sendRequest: URI \"$uri\"") + state.postValue(State.CONNECTING) + requestJob?.apply { if (isActive) cancel() } + requestJob = viewModelScope.launch(Dispatchers.IO) { val response = try { val request = Request(uri) val socket = request.connect() val channel = request.proceed(socket, this) Response.from(channel, viewModelScope) } catch (e: UnknownHostException) { - alertLiveData.postValue("Unknown host \"${uri.authority}\".") + signalError("Unknown host \"${uri.authority}\".") return@launch } catch (e: Exception) { Log.e(TAG, "sendGeminiRequest coroutine: ${e.stackTraceToString()}") - alertLiveData.postValue("Oops! Whatever we tried to do failed!") + signalError("Oops! Whatever we tried to do failed!") return@launch } if (response == null) { - alertLiveData.postValue("Can't parse server response.") + signalError("Can't parse server response.") return@launch } Log.i(TAG, "sendRequest: got ${response.code} with meta \"${response.meta}\"") when (response.code) { Response.Code.SUCCESS -> handleRequestSuccess(response) - else -> alertLiveData.postValue("Can't handle code ${response.code}.") + else -> signalError("Can't handle code ${response.code}.") } } + } + private fun signalError(message: String) { + alert.postValue(message) + state.postValue(State.IDLE) } private suspend fun handleRequestSuccess(response: Response) { - lines.clear() + state.postValue(State.RECEIVING) + linesList.clear() + lines.postValue(linesList) val charset = Charset.defaultCharset() + var lastUpdate = System.currentTimeMillis() for (line in parseData(response.data, charset, viewModelScope)) { - lines.add(line) - linesLiveData.postValue(lines) + linesList.add(line) + val time = System.currentTimeMillis() + if (time - lastUpdate >= 100) { // Throttle to 100ms the recycler view updates… + lines.postValue(linesList) + lastUpdate = time + } } + lines.postValue(linesList) + state.postValue(State.IDLE) } } diff --git a/app/src/main/java/dev/lowrespalmtree/comet/Request.kt b/app/src/main/java/dev/lowrespalmtree/comet/Request.kt index 2b6885d..ee7dfe8 100644 --- a/app/src/main/java/dev/lowrespalmtree/comet/Request.kt +++ b/app/src/main/java/dev/lowrespalmtree/comet/Request.kt @@ -50,6 +50,7 @@ class Request(private val uri: Uri) { } } Log.d(TAG, "proceed coroutine: reading completed") + channel.close() } return channel } diff --git a/app/src/main/java/dev/lowrespalmtree/comet/Response.kt b/app/src/main/java/dev/lowrespalmtree/comet/Response.kt index 4e9b8b9..5b84dc2 100644 --- a/app/src/main/java/dev/lowrespalmtree/comet/Response.kt +++ b/app/src/main/java/dev/lowrespalmtree/comet/Response.kt @@ -70,6 +70,7 @@ class Response(val code: Code, val meta: String, val data: Channel) { } // Forward all incoming data to the Response channel. channel.consumeEach { responseChannel.send(it) } + responseChannel.close() } // Return the response here; this stops consuming the channel from this for-loop so // that the coroutine above can take care of it. diff --git a/app/src/main/res/layout/activity_main.xml b/app/src/main/res/layout/activity_main.xml index 219a496..b83413e 100644 --- a/app/src/main/res/layout/activity_main.xml +++ b/app/src/main/res/layout/activity_main.xml @@ -24,28 +24,48 @@ android:layout_width="match_parent" android:layout_height="match_parent" android:layout_margin="4dp" - android:ems="10" android:hint="@string/url" android:imeOptions="actionDone|actionGo" android:importantForAutofill="no" android:inputType="textUri" - android:text="" /> + android:text="" + tools:ignore="TextContrastCheck" /> - + app:layout_behavior="@string/appbar_scrolling_view_behavior"> - - + app:layout_behavior="@string/appbar_scrolling_view_behavior"> + + + + + + + + \ No newline at end of file diff --git a/app/src/main/res/layout/gemtext_empty.xml b/app/src/main/res/layout/gemtext_empty.xml index 36ef82a..b6998cf 100644 --- a/app/src/main/res/layout/gemtext_empty.xml +++ b/app/src/main/res/layout/gemtext_empty.xml @@ -2,5 +2,5 @@ - \ No newline at end of file + android:background="@color/black" + android:orientation="vertical"> \ No newline at end of file