From a711be153a59d063a5b518714452f1194db013df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C9=91rry=20Shiv=C9=91m?= Date: Sun, 1 Sep 2024 11:29:54 +0530 Subject: [PATCH] perf: Optimize reader layout by grouping texts together & handle one edge case (#205) --------- Signed-off-by: starry-shivam --- .idea/other.xml | 318 ------------------ .../java/com/starry/myne/di/MainModule.kt | 2 +- .../java/com/starry/myne/epub/EpubParser.kt | 64 +++- .../reader/composables/ReaderContent.kt | 56 +-- .../java/com/starry/myne/EpubParserTest.kt | 3 +- 5 files changed, 99 insertions(+), 344 deletions(-) delete mode 100644 .idea/other.xml diff --git a/.idea/other.xml b/.idea/other.xml deleted file mode 100644 index 94c96f63..00000000 --- a/.idea/other.xml +++ /dev/null @@ -1,318 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/app/src/main/java/com/starry/myne/di/MainModule.kt b/app/src/main/java/com/starry/myne/di/MainModule.kt index cb7b39a5..518985e1 100644 --- a/app/src/main/java/com/starry/myne/di/MainModule.kt +++ b/app/src/main/java/com/starry/myne/di/MainModule.kt @@ -64,7 +64,7 @@ class MainModule { @Singleton @Provides - fun provideEpubParser() = EpubParser() + fun provideEpubParser(@ApplicationContext context: Context) = EpubParser(context) @Provides @Singleton diff --git a/app/src/main/java/com/starry/myne/epub/EpubParser.kt b/app/src/main/java/com/starry/myne/epub/EpubParser.kt index 633748f1..1c0b0549 100644 --- a/app/src/main/java/com/starry/myne/epub/EpubParser.kt +++ b/app/src/main/java/com/starry/myne/epub/EpubParser.kt @@ -17,6 +17,7 @@ package com.starry.myne.epub +import android.content.Context import android.graphics.Bitmap import android.graphics.BitmapFactory import android.util.Log @@ -30,13 +31,14 @@ import org.w3c.dom.Node import java.io.File import java.io.FileInputStream import java.io.InputStream +import java.util.zip.ZipFile import java.util.zip.ZipInputStream /** * Parses an EPUB file and creates an [EpubBook] object. */ -class EpubParser { +class EpubParser(private val context: Context) { /** @@ -132,8 +134,39 @@ class EpubParser { private suspend fun parseAndCreateEbook( inputStream: InputStream, shouldUseToc: Boolean ): EpubBook = withContext(Dispatchers.IO) { - val files = getZipFiles(inputStream) - val document = createEpubDocument(files) + var files = getZipFilesFromStream(inputStream) + // In some rare cases, the ZipInputStream does not contains / fails to read all of the files + // required to parse the EPUB archive, even though the zip file itself contains them. + // In such cases, retry parsing the EPUB file by directly using the ZipFile API. + // Since ZipFile requires a file path, we need to create a temporary file from the input stream. + // + // This is a workaround for the issue where ZipInputStream fails to read all of the files. + // Reasons for this issue are unknown and may be related to how the EPUB file is compressed + // i.e. weather it is missing some metadata or file/folder entry in it's header or how the + // ZipInputStream reads the file. + // + // If someone knows the exact reason for this issue, or have dealt with it before, please + // let me know or feel free to create a PR with a better solution. + val document = try { + createEpubDocument(files) + } catch (exc: EpubParserException) { + if (exc.message == "META-INF/container.xml file missing" + || exc.message == ".opf file missing" + ) { + Log.e( + TAG, "Failed to parse EPUB file using ZipInputStream " + + "due to missing files required for parsing! " + + "Retrying using ZipFile by creating temporary file " + + "From input stream.", exc + ) + Log.w(TAG, "Resetting input stream position to beginning") + (inputStream as FileInputStream).channel.position(0) + files = getZipFilesFromFile(inputStream) + createEpubDocument(files) + } else { + throw exc // Re-throw the exception if it's not due to missing files. + } + } val metadataTitle = document.metadata.selectFirstChildTag("dc:title")?.textContent ?: "Unknown Title" @@ -201,7 +234,7 @@ class EpubParser { } // Get all of the files located in the EPUB archive. - private suspend fun getZipFiles( + private suspend fun getZipFilesFromStream( inputStream: InputStream ): Map = withContext(Dispatchers.IO) { ZipInputStream(inputStream).let { zipInputStream -> @@ -211,6 +244,29 @@ class EpubParser { } } + // Fallback method to handle the issue where ZipInputStream fails to read all of the files. + // This method creates a temporary file from the input stream and uses ZipFile API to read + // all of the files in the EPUB archive. + private fun getZipFilesFromFile(inputStream: InputStream): Map { + Log.w(TAG, "Copying input stream to a temporary file") + val tempFile = File(context.cacheDir, "_zip_temp.epub") + tempFile.outputStream().use { output -> inputStream.copyTo(output) } + Log.w(TAG, "Input stream copied to temporary file") + val epubFile = ZipFile(tempFile).use { zipFile -> + zipFile.entries().asSequence() + .filterNot { it.isDirectory } + .map { entry -> + val content = zipFile.getInputStream(entry).readBytes() + EpubFile(absPath = entry.name, data = content) + } + .associateBy { it.absPath } + } + Log.w(TAG, "EPUB file created from ZipFile") + tempFile.delete() + Log.w(TAG, "Temporary file deleted") + return epubFile + } + // Create an EpubDocument object from the EPUB files. @Throws(EpubParserException::class) private fun createEpubDocument(files: Map): EpubDocument { diff --git a/app/src/main/java/com/starry/myne/ui/screens/reader/composables/ReaderContent.kt b/app/src/main/java/com/starry/myne/ui/screens/reader/composables/ReaderContent.kt index 63e1cc69..c1d8b3d7 100644 --- a/app/src/main/java/com/starry/myne/ui/screens/reader/composables/ReaderContent.kt +++ b/app/src/main/java/com/starry/myne/ui/screens/reader/composables/ReaderContent.kt @@ -211,38 +211,54 @@ private fun ChapterLazyItemItem( ) Spacer(modifier = Modifier.height(12.dp)) - paragraphs.forEach { para -> + val accumulatedText = remember { StringBuilder() } + paragraphs.forEachIndexed { index, para -> val imgEntry = BookTextMapper.ImgEntry.fromXMLString(para) - when { - imgEntry == null -> { + + if (imgEntry == null) { + // Accumulate text paragraphs with two newlines as separators + accumulatedText.append(para).append("\n\n") + } else { + // If an image is encountered, display accumulated text first + if (accumulatedText.isNotEmpty()) { Text( - text = para, + text = accumulatedText.toString().trimEnd(), fontSize = fontSize.sp, lineHeight = 1.3.em, fontFamily = state.fontFamily.fontFamily, modifier = Modifier.padding(start = 14.dp, end = 14.dp, bottom = 8.dp), ) + accumulatedText.clear() } - else -> { - val image = epubBook?.images?.find { it.absPath == imgEntry.path } - image?.let { - AsyncImage( - model = ImageRequest.Builder(LocalContext.current) - .data(image.image) - .crossfade(true) - .build(), - contentDescription = null, - contentScale = ContentScale.Crop, - modifier = Modifier - .fillMaxSize() - .padding(vertical = 6.dp) - ) - } + // Handle the image + val image = epubBook?.images?.find { it.absPath == imgEntry.path } + image?.let { + AsyncImage( + model = ImageRequest.Builder(LocalContext.current) + .data(image.image) + .crossfade(true) + .build(), + contentDescription = null, + contentScale = ContentScale.Crop, + modifier = Modifier + .fillMaxSize() + .padding(vertical = 6.dp) + ) } } - } + // Display any remaining accumulated text after the last paragraph + if (index == paragraphs.lastIndex && accumulatedText.isNotEmpty()) { + Text( + text = accumulatedText.toString().trimEnd(), + fontSize = fontSize.sp, + lineHeight = 1.3.em, + fontFamily = state.fontFamily.fontFamily, + modifier = Modifier.padding(start = 14.dp, end = 14.dp, bottom = 8.dp), + ) + } + } } HorizontalDivider( diff --git a/app/src/test/java/com/starry/myne/EpubParserTest.kt b/app/src/test/java/com/starry/myne/EpubParserTest.kt index 58843f5d..991e480c 100644 --- a/app/src/test/java/com/starry/myne/EpubParserTest.kt +++ b/app/src/test/java/com/starry/myne/EpubParserTest.kt @@ -24,6 +24,7 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment import org.robolectric.annotation.Config import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream @@ -42,7 +43,7 @@ class EpubParserTest { @Before fun setup() { - epubParser = EpubParser() + epubParser = EpubParser(RuntimeEnvironment.getApplication()) // Create a sample EPUB file for testing testEpubFile = createSampleEpubFile(hasToc = false)