-
-
Notifications
You must be signed in to change notification settings - Fork 211
Add PdfSource and RenderQuality #167
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.
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contributing to this project. This is your first pull request and we are so glad to have you onboard. We will review the request and get back to you soon. We love your contributions! Join our Discord community here to discuss this PR or ask questions.
Hi, Can you please resolve the conflicts caused due to merging old PRs. Appreicate the changes you made. |
there has been issues with small fonts being barely seen due to low pdf quality. Can we see this PR merge anytime soon if it helps with that issue? @afreakyelf |
Hi @miseke-ffw, can you please resolve the conflicts. Thank you! |
headers: HeaderData = HeaderData(), | ||
lifecycleOwner: LifecycleOwner = LocalLifecycleOwner.current, | ||
statusCallBack: PdfRendererView.StatusCallBack? = null | ||
statusCallBack: PdfRendererView.StatusCallBack? = null, | ||
scrolledToTop: (Boolean) -> Unit = {}, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
statusCallBack: PdfRendererView.StatusCallBack? = null | ||
statusCallBack: PdfRendererView.StatusCallBack? = null, | ||
scrolledToTop: (Boolean) -> Unit = {}, | ||
onPageRendered: (page: Int) -> Unit = {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make lambdas as null by default
scrolledToTop: ((Boolean) -> Unit)? = null,
onPageRendered: ((page: Int) -> Unit)? = null,
import android.net.Uri | ||
import java.io.File | ||
|
||
sealed class PdfSource { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need class
here? use sealed interface
intead
Hi, I have taken over this pull request and added render quality and a simple (but not perfect) scroll to top functionality. |
@afreakyelf , @vitoksmile , please re-review this, thank you very much! :) |
@@ -217,7 +219,8 @@ class PdfRendererView @JvmOverloads constructor( | |||
pdfRendererCore, | |||
this, | |||
pageMargin, | |||
enableLoadingForPages | |||
enableLoadingForPages, | |||
renderQuality |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add comma, so we avoid 2 lines changes in next PRs
renderQuality,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -75,6 +76,9 @@ internal class PdfViewAdapter( | |||
|
|||
if (cached != null && currentBoundPage == position) { | |||
if (DEBUG_LOGS_ENABLED) Log.d("PdfViewAdapter", "✅ Loaded page $position from cache") | |||
val aspectRatio = cached.width.toFloat() / cached.height.toFloat() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can height
be zero somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrapped runCatching for safety
No description provided.