Skip to content
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

Fix #4170: Adding LatexImageSpan for vertical alignment of locally rendered and cached LaTeX #5647

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package org.oppia.android.util.parser.html

import android.app.Application
import android.content.res.AssetManager
import android.graphics.Canvas
import android.graphics.Paint
import android.graphics.drawable.Drawable
import android.text.Editable
import android.text.Spannable
import android.text.style.ImageSpan
Expand Down Expand Up @@ -60,7 +63,7 @@ class MathTagHandler(
}
is MathContent.MathAsLatex -> {
if (cacheLatexRendering) {
ImageSpan(
LatexImageSpan(
imageRetriever.loadMathDrawable(
content.rawLatex,
lineHeight,
Expand Down Expand Up @@ -144,3 +147,73 @@ class MathTagHandler(
return mathVal?.let { "Math content $it" } ?: ""
}
}

/** An [ImageSpan] that vertically centers a LaTeX drawable within the surrounding text. */
private class LatexImageSpan(drawable: Drawable?) :
ImageSpan(drawable ?: createEmptyDrawable()) {
override fun getSize(
paint: Paint,
text: CharSequence,
start: Int,
end: Int,
fontMetricsInt: Paint.FontMetricsInt?
): Int {
val d = drawable
val bounds = d.bounds

fontMetricsInt?.let {
val paintFm = paint.fontMetricsInt
val textHeight = paintFm.descent - paintFm.ascent
val latexHeight = bounds.height()

val centeringOffset = (textHeight - latexHeight) / 2

it.ascent = paintFm.ascent + centeringOffset
it.top = paintFm.top + centeringOffset
it.descent = it.ascent + latexHeight
it.bottom = it.top + latexHeight
}

return bounds.right
}

override fun draw(
canvas: Canvas,
text: CharSequence,
start: Int,
end: Int,
x: Float,
top: Int,
y: Int,
bottom: Int,
paint: Paint
) {
val d = drawable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we change to a more descriptive variable name?

canvas.save()

val fontMetrics = paint.fontMetricsInt
val latexHeight = d.bounds.height()

val centerY = y + (fontMetrics.descent + fontMetrics.ascent) / 2
val drawableY = centerY - (latexHeight / 2)

canvas.translate(x, drawableY.toFloat())
d.draw(canvas)
canvas.restore()
}

companion object {
private fun createEmptyDrawable(): Drawable {
return object : Drawable() {
override fun draw(canvas: Canvas) {}
override fun setAlpha(alpha: Int) {}
override fun setColorFilter(colorFilter: android.graphics.ColorFilter?) {}
override fun getOpacity(): Int = android.graphics.PixelFormat.TRANSPARENT

init {
setBounds(0, 0, 1, 1)
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package org.oppia.android.util.parser.html

import android.app.Application
import android.content.Context
import android.graphics.Canvas
import android.graphics.Color
import android.graphics.Paint
import android.text.Html
import android.text.Spannable
import android.text.style.ImageSpan
Expand All @@ -21,6 +23,8 @@ import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.Captor
import org.mockito.Mock
import org.mockito.Mockito.eq
import org.mockito.Mockito.mock
import org.mockito.Mockito.times
import org.mockito.Mockito.verify
import org.mockito.Mockito.verifyNoMoreInteractions
Expand All @@ -39,6 +43,7 @@ import org.robolectric.annotation.Config
import org.robolectric.annotation.LooperMode
import javax.inject.Inject
import javax.inject.Singleton
import kotlin.math.abs
import kotlin.reflect.KClass

private const val MATH_MARKUP_1 =
Expand Down Expand Up @@ -107,7 +112,127 @@ class MathTagHandlerTest {
}

// TODO(#3085): Introduce test for verifying that the error log scenario is logged correctly.
@Test
fun testParseHtml_withMathMarkup_cachingOn_imageSpanHasCorrectMetrics() {

val parsedHtml = CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithCachedMathSupport
)
val imageSpans = parsedHtml.getSpansFromWholeString(ImageSpan::class)
assertThat(imageSpans).hasLength(1)

val paint = Paint()
paint.textSize = 20f
val originalMetrics = Paint.FontMetricsInt()
paint.getFontMetricsInt(originalMetrics)

val spanMetrics = Paint.FontMetricsInt()
imageSpans[0].getSize(paint, parsedHtml, 0, parsedHtml.length, spanMetrics)

// The span's center should align with the text's center
val originalCenter = (originalMetrics.descent + originalMetrics.ascent) / 2
val spanCenter = (spanMetrics.descent + spanMetrics.ascent) / 2
assertThat(abs(originalCenter - spanCenter)).isLessThan(2)
}

@Test
fun testParseHtml_withMathMarkup_cachingOn_drawsAtCorrectVerticalPosition() {

val parsedHtml = CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithCachedMathSupport
)

val imageSpans = parsedHtml.getSpansFromWholeString(ImageSpan::class)
assertThat(imageSpans).hasLength(1)

val mockCanvas = mock(Canvas::class.java)
val paint = Paint()
paint.textSize = 20f

val metrics = paint.fontMetricsInt
val y = 100
val expectedCenterY = y + (metrics.descent + metrics.ascent) / 2f

imageSpans[0].draw(
mockCanvas,
parsedHtml,
0,
parsedHtml.length,
0f,
0,
y,
200,
paint
)
// The canvas should be translated to center the drawable vertically
verify(mockCanvas).save()
verify(mockCanvas).translate(
eq(0f),
capture(floatCaptor)
)
// The translation should position the drawable centered around the text baseline
val drawable = imageSpans[0].drawable
val expectedTranslation = expectedCenterY - (drawable.bounds.height() / 2)
assertThat(floatCaptor.value).isWithin(1f).of(expectedTranslation)

verify(mockCanvas).restore()
}

@Test
fun testParseHtml_withMathMarkup_cachingOn_maintainsConsistentHeight() {

val parsedHtml = CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithCachedMathSupport
)

val imageSpans = parsedHtml.getSpansFromWholeString(ImageSpan::class)
assertThat(imageSpans).hasLength(1)

val paint = Paint()
paint.textSize = 20f

val metrics1 = Paint.FontMetricsInt()
val metrics2 = Paint.FontMetricsInt()

val size1 = imageSpans[0].getSize(paint, parsedHtml, 0, parsedHtml.length, metrics1)
val size2 = imageSpans[0].getSize(paint, parsedHtml, 0, parsedHtml.length, metrics2)

assertThat(size1).isEqualTo(size2)
assertThat(metrics1.ascent).isEqualTo(metrics2.ascent)
assertThat(metrics1.descent).isEqualTo(metrics2.descent)
assertThat(metrics1.top).isEqualTo(metrics2.top)
assertThat(metrics1.bottom).isEqualTo(metrics2.bottom)
}

@Test
fun testParseHtml_withMathMarkup_cachingOn_respectsLineHeight() {

val parsedHtml = CustomHtmlContentHandler.fromHtml(
html = MATH_WITHOUT_FILENAME_MARKUP,
imageRetriever = mockImageRetriever,
customTagHandlers = tagHandlersWithCachedMathSupport
)

val imageSpans = parsedHtml.getSpansFromWholeString(ImageSpan::class)
assertThat(imageSpans).hasLength(1)

val paint = Paint()
paint.textSize = 20f

val metrics = Paint.FontMetricsInt()
imageSpans[0].getSize(paint, parsedHtml, 0, parsedHtml.length, metrics)

// Verify that the total height does not exceed the line height
val totalHeight = metrics.bottom - metrics.top
val lineHeight = paint.textSize * 1.2f
assertThat(totalHeight.toFloat()).isLessThan(lineHeight)
}
@Test
fun testParseHtml_emptyString_doesNotIncludeImageSpan() {
val parsedHtml =
Expand Down
Loading