Add Android rendering logic for PDF reports#3774
Conversation
0c304e7 to
d7bd17c
Compare
shobhitagarwal1612
left a comment
There was a problem hiding this comment.
Could you please add screenshots to the description?
|
@shobhitagarwal1612 there is no UI entry point in this PR to show the direct output. But I've added a screenshot of how the doc will look like once the feature is completed in the next PR, hope it helps |
…and QrCodeGenerator
shobhitagarwal1612
left a comment
There was a problem hiding this comment.
Thanks for sending the PR. I've left a few comments for you to review. On a high level, the PR looks reasonable but due to the large diff, it's hard to throughly review the code. Would you mind breaking it up into small 2-3 PRs?
| writer(document, images, DocumentPdfCanvas(pdf), totalPages = totalPages) | ||
| .drawDocument(document) | ||
| File(outputPath).outputStream().use { pdf.writeTo(it) } |
There was a problem hiding this comment.
Since this is file operation, should it be wrapped in IO dispatchers block? Or will this be handled at the caller?
| photoFilenames | ||
| .filter { it.isNotEmpty() } | ||
| .forEach { filename -> | ||
| loadPhotoBitmap(filename)?.let { bitmap -> | ||
| bitmapsToRelease += bitmap | ||
| images[PdfImageSet.ImageRef.Photo(filename)] = PdfImage(bitmap) | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we use async / awaitAll to decode the images concurrently?
| private val photoMaxWidthPx = pointsToRenderPixels(PdfConfig.TABLE_ANSWER_TEXT_WIDTH.toFloat()) | ||
| private val photoMaxHeightPx = pointsToRenderPixels(PdfConfig.PHOTO_MAX_HEIGHT.toFloat()) | ||
|
|
||
| override suspend fun load(qrContent: String?, photoFilenames: Set<String>): PdfImageSet { |
There was a problem hiding this comment.
Similar to previous question, are we planning to wrap the caller with io dispatcher? If not, should we do it here?
| val answerLayout = | ||
| if (answerText.isEmpty()) null | ||
| else staticLayout(answerText, paints.body, PdfConfig.TABLE_ANSWER_TEXT_WIDTH) |
There was a problem hiding this comment.
nit: Can be replaced with ?. operator
val answerLayout = answerText.takeIf { it.isNotEmpty() }
?.let { staticLayout(it, paints.body, PdfConfig.TABLE_ANSWER_TEXT_WIDTH) }
| while (width / (sampleSize * 2) >= maxWidth || height / (sampleSize * 2) >= maxHeight) { | ||
| sampleSize *= 2 |
There was a problem hiding this comment.
nit: this is hard to read. Can we either use local variable or helper methods?
|
extracted 2 PRs out of this one: This way, this PR becomes focused on the Android implementation of the interfaces needed to generate the PDF report. Since his depends on the PRs above, I'll mark this as draft for the time being |
Towards #3715
This PR adds the specific Android implementations for rendering PDF files.
Main changes:
PdfCursorto keep track of the current position within a page and its reserved spacesPdfPageControllerto keep track of page counts and their lifecyclesPdfWriter,PdfCanvas,PdfTextPaintsandDocumentPdfCanvasin order to draw the layoutThere are no functionality changes yet. The full flow will be implemented in a final follow up PR. For reference, the current layout will generate the following document structure:

@shobhitagarwal1612 PTAL?