Skip to content

Commit

Permalink
Fix #5112: Rendering Inline SVGs (#5208)
Browse files Browse the repository at this point in the history
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixes #5112 

We discovered that `mathImg_` dimesions are in **ex**, and not **px**,
and this unit is not natively rendered on android.

This PR introduces a method that will convert ex to px and use the
resulting dimesions to compute the rendering size of a math image to be
rendered inline. The formula is based on the [androidSvg
library](https://github.com/oppia/androidsvg/blob/5bc9c7553e94c3476e8ea32baea3c77567228fcd/androidsvg/src/main/java/com/caverock/androidsvg/androidrendering/SVGAndroidRenderer.java#L5018)
that is also based on CSS3 specs.

To preserve the size computation of block SVGs, we have created a
seperate method that handles the calculation of rendering dimensions for
inline SVGs.

We have only tested these changes through visual observation, and
confirming that the images scale correctly when reading text size is
changed.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
|Before|After|
|--|--|

|![IMG_0705](https://github.com/oppia/oppia-android/assets/99066793/62e00542-091c-48b1-9e63-aa1ca3c9abff)|![Screenshot_1698283405](https://github.com/oppia/oppia-android/assets/59600948/e1c534ee-b379-461c-bc39-7854fe57dc70)|

|![Screenshot_1698284125](https://github.com/oppia/oppia-android/assets/59600948/3f47014d-f4e3-4430-9e1e-d4b4609014e7)|![Screenshot_1698283411](https://github.com/oppia/oppia-android/assets/59600948/786445ec-0b75-41af-882b-ae93e2da9b4d)|

|![Screenshot_1697714168](https://github.com/oppia/oppia-android/assets/59600948/9fa8daeb-25ca-4b90-ac66-a6228c81f120)|![Screenshot_1698284519](https://github.com/oppia/oppia-android/assets/59600948/0b39d37d-1471-4ecf-8626-13a01ec35c3e)|

|![Screenshot_1697745323](https://github.com/oppia/oppia-android/assets/59600948/8152c286-8845-4f3b-8ecf-93acd03dbc3e)|![Screenshot_1698284666](https://github.com/oppia/oppia-android/assets/59600948/dbff75ba-bb08-4855-b49a-0e4d337c478b)|

|![Screenshot_1697714820](https://github.com/oppia/oppia-android/assets/59600948/65b7eadc-c8b0-4963-8dbf-d4cd19590e98)|![Screenshot_1698284933](https://github.com/oppia/oppia-android/assets/59600948/9b517dc5-fc75-43f8-b6af-a4e24b1cde28)|

Images scaled to Largest text and display size, and dark mode
||||
|--|--|--|

|![image](https://github.com/oppia/oppia-android/assets/59600948/cedd58b2-a16f-4ed7-9856-df9371242d3a)|![image](https://github.com/oppia/oppia-android/assets/59600948/acf79348-72cb-4732-b91a-1f934861d9f0)|![image](https://github.com/oppia/oppia-android/assets/59600948/fa07209d-fb54-4f21-a267-cc9bd7d26177)|

|![image](https://github.com/oppia/oppia-android/assets/59600948/241f8501-21ce-488f-be6e-a6f2b4bde198)|![image](https://github.com/oppia/oppia-android/assets/59600948/82d0a270-e486-4553-96ae-559ad7b784a2)|![image](https://github.com/oppia/oppia-android/assets/59600948/6080aff6-aff0-46ad-b30c-88e086aeeac7)|

---------

Co-authored-by: Ben Henning <ben@oppia.org>
  • Loading branch information
adhiamboperes and BenHenning committed Nov 14, 2023
1 parent 913cf5f commit 642a778
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 20 deletions.
4 changes: 2 additions & 2 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ git_repository(
# to correctly size in-line SVGs (such as those needed for LaTeX-based math expressions).
git_repository(
name = "androidsvg",
commit = "1265eb1087056cf3fc2e10442e5545bc65c109ce",
commit = "5bc9c7553e94c3476e8ea32baea3c77567228fcd",
remote = "https://github.com/oppia/androidsvg",
shallow_since = "1686302944 -0700",
shallow_since = "1686304726 -0700",
)

git_repository(
Expand Down
2 changes: 1 addition & 1 deletion utility/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ dependencies {
'androidx.appcompat:appcompat:1.0.2',
'androidx.lifecycle:lifecycle-livedata-ktx:2.2.0-alpha03',
'androidx.work:work-runtime-ktx:2.4.0',
'com.github.oppia:androidsvg:1265eb1087056cf3fc2e10442e5545bc65c109ce',
'com.github.oppia:androidsvg:5bc9c7553e94c3476e8ea32baea3c77567228fcd',
'com.github.oppia:kotlitex:43139c140833c7120f351d63d74b42c253d2b213',
'com.github.bumptech.glide:glide:4.11.0',
'com.google.dagger:dagger:2.24',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ package org.oppia.android.util.parser.svg
import android.graphics.Picture
import android.graphics.RectF
import android.text.TextPaint
import com.caverock.androidsvg.RenderOptions
import com.caverock.androidsvg.SVG
import com.caverock.androidsvg.utils.RenderOptionsBase
import com.caverock.androidsvg.androidrendering.RenderOptions
import com.caverock.androidsvg.androidrendering.RenderOptionsBase
import com.caverock.androidsvg.androidrendering.SVG
import org.oppia.android.util.parser.image.ImageTransformation

/**
Expand Down Expand Up @@ -39,18 +39,14 @@ class ScalableVectorGraphic {
}

/**
* Returns the [SvgSizeSpecs] corresponding to this SVG, based on the specified [textPaint]. If a
* [TextPaint] is supplied, the returns specs will include text-based adjustments (for in-line
* images). Otherwise, the returned specs will be arranged for rendering the SVG in a standalone
* manner.
* Returns the [SvgSizeSpecs] corresponding to this SVG.
*
* The returned specs will be arranged for rendering the SVG in a standalone manner.
*/
fun computeSizeSpecs(textPaint: TextPaint?): SvgSizeSpecs {
val options = RenderOptionsBase().also { if (textPaint != null) it.textPaint(textPaint) }
fun computeSizeSpecs(): SvgSizeSpecs {
val options = RenderOptionsBase()
val documentWidth = parsedSvg.value.getDocumentWidthOrNull(options)
val documentHeight = parsedSvg.value.getDocumentHeightOrNull(options)
val verticalAlignment = if (textPaint != null) {
adjustAlignmentForAndroid(parsedSvg.value.getVerticalAlignment(options))
} else 0f

val viewBox: RectF? = parsedSvg.value.documentViewBox
val viewBoxWidth = viewBox?.width()
Expand All @@ -75,16 +71,54 @@ class ScalableVectorGraphic {
intrinsicHeight,
renderedWidth = imageFileNameWidth ?: intrinsicWidth,
renderedHeight = imageFileNameHeight ?: intrinsicHeight,
verticalAlignment = 0f
)
}

/**
* Returns the [SvgSizeSpecs] corresponding to this SVG, based on the specified [textPaint].
* Based on the supplied [TextPaint], the returned specs will include text-based adjustments
* (for in-line images).
*/
fun computeSizeSpecsForTextPicture(textPaint: TextPaint?): SvgSizeSpecs {
val options = textPaint?.let { RenderOptionsBase().textPaint(it) } ?: RenderOptionsBase()
val documentWidth = parsedSvg.value.getDocumentWidthOrNull(options)
val documentHeight = parsedSvg.value.getDocumentHeightOrNull(options)

val imageFileNameWidth = extractedWidth?.toFloat()
val imageFileNameHeight = extractedHeight?.toFloat()

val fontMetrics = textPaint?.fontMetrics
val fontHeight = fontMetrics?.descent?.minus(fontMetrics.ascent) ?: 0f

val adjustedWidth =
imageFileNameWidth?.convertExToPx(fontHeight) ?: documentWidth ?: DEFAULT_SIZE_PX
val adjustedHeight =
imageFileNameHeight?.convertExToPx(fontHeight) ?: documentHeight ?: DEFAULT_SIZE_PX

val verticalAlignment = textPaint?.let {
adjustAlignmentForAndroid(parsedSvg.value.getVerticalAlignment(options))
} ?: 0f

return SvgSizeSpecs(
adjustedWidth,
adjustedHeight,
renderedWidth = adjustedWidth,
renderedHeight = adjustedHeight,
verticalAlignment
)
}

private fun Float.convertExToPx(fontHeight: Float): Float {
return this * fontHeight * 0.5f
}

/**
* Returns an Android [Picture] including the draw instructions for rendering this SVG within a
* line of text whose size and style is configured by the provided [textPaint].
*/
fun renderToTextPicture(textPaint: TextPaint): Picture {
return computeSizeSpecs(textPaint).let { (width, height, _) ->
return computeSizeSpecsForTextPicture(textPaint).let { (width, height, _) ->
val options =
RenderOptions().textPaint(textPaint).viewPort(0f, 0f, width, height) as RenderOptions
parsedSvg.value.renderToPicture(options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,17 @@ abstract class SvgPictureDrawable(
* when [textPaint] is null and text rendering when otherwise.
*/
protected fun reinitialize(textPaint: TextPaint?) {
picture = textPaint?.let {
scalableVectorGraphic.renderToTextPicture(it)
} ?: scalableVectorGraphic.renderToBlockPicture()
val newPicture = if (textPaint != null) {
intrinsicSize = scalableVectorGraphic.computeSizeSpecsForTextPicture(textPaint)
scalableVectorGraphic.renderToTextPicture(textPaint)
} else {
intrinsicSize = scalableVectorGraphic.computeSizeSpecs()
scalableVectorGraphic.renderToBlockPicture()
}

picture = newPicture

// TODO(#4246): Fix both SVG rendering performance and upscaling to ensure images aren't blurry.
intrinsicSize = scalableVectorGraphic.computeSizeSpecs(textPaint)
if (scalableVectorGraphic.shouldBeRenderedAsBitmap()) {
recomputeBitmap()
}
Expand Down

0 comments on commit 642a778

Please sign in to comment.