Skip to content

Commit

Permalink
Improve handling of fractional width fonts
Browse files Browse the repository at this point in the history
  • Loading branch information
citizenmatt authored and AlexPl292 committed Jul 6, 2022
1 parent 249bd37 commit 62632a4
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 36 deletions.
43 changes: 30 additions & 13 deletions src/main/java/com/maddyhome/idea/vim/helper/EditorHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.jetbrains.annotations.Range;

import java.awt.*;
import java.awt.geom.Point2D;
import java.nio.CharBuffer;
import java.util.Collections;
import java.util.Comparator;
Expand Down Expand Up @@ -245,12 +246,27 @@ public static int getApproximateScreenHeight(final @NotNull Editor editor) {
* font. It does not include inlays or folds.
* <p>
* Note that this value is only approximate and should be avoided whenever possible!
* </p>
*
* @param editor The editor
* @return The number of screen columns
*/
public static int getApproximateScreenWidth(final @NotNull Editor editor) {
return getVisibleArea(editor).width / EditorUtil.getPlainSpaceWidth(editor);
return (int)(getVisibleArea(editor).width / getPlainSpaceWidthFloat(editor));
}

/**
* Gets the width of the space character in the editor's plain font as a float.
* <p>
* Font width can be fractional, but {@link EditorUtil#getPlainSpaceWidth(Editor)} returns it as an int, which can
* lead to rounding errors.
* </p>
*
* @param editor The editor
* @return The width of the space character in the editor's plain font in pixels. It might be a fractional value.
*/
public static float getPlainSpaceWidthFloat(final @NotNull Editor editor) {
return EditorUtil.fontForChar(' ', Font.PLAIN, editor).charWidth2D(' ');
}

/**
Expand Down Expand Up @@ -802,19 +818,20 @@ else if (visualColumn > 0) {
}
}

final int columnLeftX = editor.visualPositionToXY(new VisualPosition(visualLine, targetVisualColumn)).x;
final int columnLeftX = (int) Math.round(editor.visualPositionToPoint2D(new VisualPosition(visualLine, targetVisualColumn)).getX());
scrollHorizontally(editor, columnLeftX);
}

public static void scrollColumnToMiddleOfScreen(@NotNull Editor editor, int visualLine, int visualColumn) {
final Point point = editor.visualPositionToXY(new VisualPosition(visualLine, visualColumn));
final Point2D point = editor.visualPositionToPoint2D(new VisualPosition(visualLine, visualColumn));
final int screenWidth = getVisibleArea(editor).width;

// Snap the column to the nearest standard column grid. This positions us nicely if there are an odd or even number
// of columns. It also works with inline inlays and folds. It is slightly inaccurate for proportional fonts, but is
// still a good solution. Besides, what kind of monster uses Vim with proportional fonts?
final int standardColumnWidth = EditorUtil.getPlainSpaceWidth(editor);
final int x = max(0, point.x - (screenWidth / standardColumnWidth / 2 * standardColumnWidth));
final float standardColumnWidth = EditorHelper.getPlainSpaceWidthFloat(editor);
final int screenMidColumn = (int) (screenWidth / standardColumnWidth / 2);
final int x = max(0, (int) Math.round(point.getX() - (screenMidColumn * standardColumnWidth)));
scrollHorizontally(editor, x);
}

Expand All @@ -839,7 +856,7 @@ public static void scrollColumnToRightOfScreen(@NotNull Editor editor, int visua
}

// Scroll to the left edge of the target column, minus a screenwidth, and adjusted for inlays
final int targetColumnRightX = editor.visualPositionToXY(new VisualPosition(visualLine, targetVisualColumn + 1)).x;
final int targetColumnRightX = (int) Math.round(editor.visualPositionToPoint2D(new VisualPosition(visualLine, targetVisualColumn + 1)).getX());
final int screenWidth = getVisibleArea(editor).width;
scrollHorizontally(editor, targetColumnRightX - screenWidth);
}
Expand Down Expand Up @@ -1001,18 +1018,18 @@ private static int getFullVisualColumn(final @NotNull Editor editor, int x, int
// Note that visualPos.leansRight will be true for the right half side of the character grid
VisualPosition closestVisualPosition = editor.xyToVisualPosition(new Point(x, y));

// Make sure we get the character that contains this XY, not the editor's decision about closest character. The
// editor will give us the next character if X is over half way through the character grid.
int xActualLeft = editor.visualPositionToXY(closestVisualPosition).x;
// Make sure we get the character that contains this XY, not the editor's decision about the closest character. The
// editor will give us the next character if X is over halfway through the character grid. Take into account that
// the font size might be fractional, but the editor's area is integer. Use floating point values and round.
long xActualLeft = Math.round(editor.visualPositionToPoint2D(closestVisualPosition).getX());
if (xActualLeft > x) {
closestVisualPosition = getPreviousNonInlayVisualPosition(editor, closestVisualPosition);
xActualLeft = editor.visualPositionToXY(closestVisualPosition).x;
xActualLeft = Math.round(editor.visualPositionToPoint2D(closestVisualPosition).getX());
}

if (xActualLeft >= leftBound) {
final int xActualRight =
editor.visualPositionToXY(new VisualPosition(closestVisualPosition.line, closestVisualPosition.column + 1)).x -
1;
final VisualPosition nextVisualPosition = new VisualPosition(closestVisualPosition.line, closestVisualPosition.column + 1);
final long xActualRight = Math.round(editor.visualPositionToPoint2D(nextVisualPosition).getX()) - 1;
if (xActualRight <= rightBound) {
return closestVisualPosition.column;
}
Expand Down
10 changes: 6 additions & 4 deletions src/test/java/org/jetbrains/plugins/ideavim/VimTestCase.kt
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import com.intellij.openapi.editor.LogicalPosition
import com.intellij.openapi.editor.VisualPosition
import com.intellij.openapi.editor.colors.EditorColors
import com.intellij.openapi.editor.ex.EditorEx
import com.intellij.openapi.editor.ex.util.EditorUtil
import com.intellij.openapi.fileEditor.ex.FileEditorManagerEx
import com.intellij.openapi.fileTypes.FileType
import com.intellij.openapi.fileTypes.PlainTextFileType
Expand Down Expand Up @@ -82,6 +81,7 @@ import org.junit.Assert
import java.awt.event.KeyEvent
import java.util.*
import javax.swing.KeyStroke
import kotlin.math.roundToInt

/**
* @author vlan
Expand Down Expand Up @@ -169,7 +169,9 @@ abstract class VimTestCase : UsefulTestCase() {
get() = 35

protected fun setEditorVisibleSize(width: Int, height: Int) {
EditorTestUtil.setEditorVisibleSize(myFixture.editor, width, height)
val w = (width * EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)).roundToInt()
val h = height * myFixture.editor.lineHeight
EditorTestUtil.setEditorVisibleSizeInPixels(myFixture.editor, w, h)
}

protected fun setEditorVirtualSpace() {
Expand Down Expand Up @@ -609,7 +611,7 @@ abstract class VimTestCase : UsefulTestCase() {
// per platform (e.g. Windows is 7, Mac is 8) so we can't guarantee correct positioning for tests if we use hard coded
// pixel widths
protected fun addInlay(offset: Int, relatesToPrecedingText: Boolean, widthInColumns: Int): Inlay<*> {
val widthInPixels = EditorUtil.getPlainSpaceWidth(myFixture.editor) * widthInColumns
val widthInPixels = (EditorHelper.getPlainSpaceWidthFloat(myFixture.editor) * widthInColumns).roundToInt()
return EditorTestUtil.addInlay(myFixture.editor, offset, relatesToPrecedingText, widthInPixels)
}

Expand All @@ -619,7 +621,7 @@ abstract class VimTestCase : UsefulTestCase() {
// float if necessary. We'd still be working scaled to the line height, so fractional values should still work.
protected fun addBlockInlay(offset: Int, showAbove: Boolean, heightInRows: Int): Inlay<*> {
val widthInColumns = 10 // Arbitrary width. We don't care.
val widthInPixels = EditorUtil.getPlainSpaceWidth(myFixture.editor) * widthInColumns
val widthInPixels = (EditorHelper.getPlainSpaceWidthFloat(myFixture.editor) * widthInColumns).roundToInt()
val heightInPixels = myFixture.editor.lineHeight * heightInRows
return EditorTestUtil.addBlockInlay(myFixture.editor, offset, false, showAbove, widthInPixels, heightInPixels)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,16 @@
package org.jetbrains.plugins.ideavim.action.scroll

import com.intellij.openapi.editor.Inlay
import com.intellij.openapi.editor.ex.util.EditorUtil
import com.maddyhome.idea.vim.VimPlugin
import com.maddyhome.idea.vim.api.injector
import com.maddyhome.idea.vim.helper.EditorHelper
import com.maddyhome.idea.vim.helper.VimBehaviorDiffers
import com.maddyhome.idea.vim.options.OptionConstants
import com.maddyhome.idea.vim.options.OptionScope
import com.maddyhome.idea.vim.vimscript.model.datatypes.VimInt
import org.jetbrains.plugins.ideavim.VimTestCase
import org.junit.Assert
import kotlin.math.roundToInt

/*
*zs*
Expand Down Expand Up @@ -77,7 +78,7 @@ class ScrollFirstScreenColumnActionTest : VimTestCase() {
typeText(injector.parser.parseKeys("100|" + "zs"))
val visibleArea = myFixture.editor.scrollingModel.visibleArea
val textWidth = visibleArea.width - inlay.widthInPixels
val availableColumns = textWidth / EditorUtil.getPlainSpaceWidth(myFixture.editor)
val availableColumns = (textWidth / EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)).roundToInt()

// The first visible text column will be 99, with the inlay positioned to the left of it
assertVisibleLineBounds(0, 99, 99 + availableColumns - 1)
Expand Down Expand Up @@ -112,6 +113,6 @@ class ScrollFirstScreenColumnActionTest : VimTestCase() {

private fun getAvailableColumns(inlay: Inlay<*>): Int {
val textWidth = myFixture.editor.scrollingModel.visibleArea.width - inlay.widthInPixels
return textWidth / EditorUtil.getPlainSpaceWidth(myFixture.editor)
return (textWidth / EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)).roundToInt()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@
package org.jetbrains.plugins.ideavim.action.scroll

import com.intellij.openapi.editor.Inlay
import com.intellij.openapi.editor.ex.util.EditorUtil
import com.maddyhome.idea.vim.VimPlugin
import com.maddyhome.idea.vim.api.injector
import com.maddyhome.idea.vim.helper.EditorHelper
import com.maddyhome.idea.vim.options.OptionConstants
import com.maddyhome.idea.vim.options.OptionScope
import com.maddyhome.idea.vim.vimscript.model.datatypes.VimInt
import org.jetbrains.plugins.ideavim.VimTestCase
import org.junit.Assert
import kotlin.math.roundToInt

/*
*ze*
Expand Down Expand Up @@ -109,14 +110,15 @@ class ScrollLastScreenColumnActionTest : VimTestCase() {
typeText(injector.parser.parseKeys("100|" + "ze"))
val visibleArea = myFixture.editor.scrollingModel.visibleArea
val textWidth = visibleArea.width - inlay.widthInPixels
val availableColumns = textWidth / EditorUtil.getPlainSpaceWidth(myFixture.editor)
val availableColumns = (textWidth / EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)).roundToInt()

// The last visible text column will be 99, but it will be positioned before the inlay
assertVisibleLineBounds(0, 99 - availableColumns + 1, 99)

// We have to assert the location of the inlay
Assert.assertEquals(visibleArea.x + textWidth, inlay.bounds!!.x)
Assert.assertEquals(visibleArea.x + visibleArea.width, inlay.bounds!!.x + inlay.bounds!!.width)
val inlayX = myFixture.editor.visualPositionToPoint2D(inlay.visualPosition).x.roundToInt()
Assert.assertEquals(visibleArea.x + textWidth, inlayX)
Assert.assertEquals(visibleArea.x + visibleArea.width, inlayX + inlay.widthInPixels)
}

fun `test last screen column does not include subsequent inline inlay associated with following text`() {
Expand All @@ -130,6 +132,6 @@ class ScrollLastScreenColumnActionTest : VimTestCase() {

private fun getAvailableColumns(inlay: Inlay<*>): Int {
val textWidth = myFixture.editor.scrollingModel.visibleArea.width - inlay.widthInPixels
return textWidth / EditorUtil.getPlainSpaceWidth(myFixture.editor)
return (textWidth / EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)).roundToInt()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,16 @@

package org.jetbrains.plugins.ideavim.group.motion

import com.intellij.openapi.editor.ex.util.EditorUtil
import com.maddyhome.idea.vim.VimPlugin
import com.maddyhome.idea.vim.api.injector
import com.maddyhome.idea.vim.helper.EditorHelper
import com.maddyhome.idea.vim.options.OptionConstants
import com.maddyhome.idea.vim.options.OptionScope
import com.maddyhome.idea.vim.vimscript.model.datatypes.VimInt
import org.jetbrains.plugins.ideavim.SkipNeovimReason
import org.jetbrains.plugins.ideavim.TestWithoutNeovim
import org.jetbrains.plugins.ideavim.VimTestCase
import kotlin.math.roundToInt

@Suppress("ClassName")
class MotionGroup_ScrollCaretIntoViewHorizontally_Test : VimTestCase() {
Expand Down Expand Up @@ -110,7 +111,7 @@ class MotionGroup_ScrollCaretIntoViewHorizontally_Test : VimTestCase() {
// These columns are hard to calculate, because the visible offset depends on the rendered width of the inlay
// Also, because we're scrolling right (adding columns to the right) we make the right most column line up
val textWidth = myFixture.editor.scrollingModel.visibleArea.width - inlay.widthInPixels
val availableColumns = textWidth / EditorUtil.getPlainSpaceWidth(myFixture.editor)
val availableColumns = (textWidth / EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)).roundToInt()
assertVisibleLineBounds(0, 119 - availableColumns + 1, 119)
}

Expand Down Expand Up @@ -185,7 +186,7 @@ class MotionGroup_ScrollCaretIntoViewHorizontally_Test : VimTestCase() {
typeText(injector.parser.parseKeys("120|zs" + "20h"))
// These columns are hard to calculate, because the visible offset depends on the rendered width of the inlay
val textWidth = myFixture.editor.scrollingModel.visibleArea.width - inlay.widthInPixels
val availableColumns = textWidth / EditorUtil.getPlainSpaceWidth(myFixture.editor)
val availableColumns = (textWidth / EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)).roundToInt()
assertVisibleLineBounds(0, 99, 99 + availableColumns - 1)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,16 @@ import org.jetbrains.plugins.ideavim.SkipNeovimReason
import org.jetbrains.plugins.ideavim.TestWithoutNeovim
import org.jetbrains.plugins.ideavim.VimTestCase
import org.junit.Assert
import kotlin.math.roundToInt

class EditorHelperTest : VimTestCase() {
@TestWithoutNeovim(SkipNeovimReason.NOT_VIM_TESTING)
fun `test scroll column to left of screen`() {
configureByColumns(100)
EditorHelper.scrollColumnToLeftOfScreen(myFixture.editor, 0, 2)
val visibleArea = myFixture.editor.scrollingModel.visibleArea
val columnWidth = visibleArea.width / screenWidth
Assert.assertEquals(2 * columnWidth, visibleArea.x)
val columnWidth = EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)
Assert.assertEquals((2 * columnWidth).roundToInt(), visibleArea.x)
}

@TestWithoutNeovim(SkipNeovimReason.NOT_VIM_TESTING)
Expand All @@ -40,8 +41,8 @@ class EditorHelperTest : VimTestCase() {
val column = screenWidth + 2
EditorHelper.scrollColumnToRightOfScreen(myFixture.editor, 0, column)
val visibleArea = myFixture.editor.scrollingModel.visibleArea
val columnWidth = visibleArea.width / screenWidth
Assert.assertEquals((column - screenWidth + 1) * columnWidth, visibleArea.x)
val columnWidth = EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)
Assert.assertEquals(((column - screenWidth + 1) * columnWidth).roundToInt(), visibleArea.x)
}

@TestWithoutNeovim(SkipNeovimReason.NOT_VIM_TESTING)
Expand All @@ -52,8 +53,8 @@ class EditorHelperTest : VimTestCase() {
// Put column 100 into position 41 -> offset is 59 columns
EditorHelper.scrollColumnToMiddleOfScreen(myFixture.editor, 0, 99)
val visibleArea = myFixture.editor.scrollingModel.visibleArea
val columnWidth = visibleArea.width / screenWidth
Assert.assertEquals(59 * columnWidth, visibleArea.x)
val columnWidth = EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)
Assert.assertEquals((59 * columnWidth).roundToInt(), visibleArea.x)
}

@TestWithoutNeovim(SkipNeovimReason.NOT_VIM_TESTING)
Expand All @@ -65,7 +66,7 @@ class EditorHelperTest : VimTestCase() {
// Put column 100 into position 41 -> offset is 59 columns
EditorHelper.scrollColumnToMiddleOfScreen(myFixture.editor, 0, 99)
val visibleArea = myFixture.editor.scrollingModel.visibleArea
val columnWidth = visibleArea.width / screenWidth
Assert.assertEquals(59 * columnWidth, visibleArea.x)
val columnWidth = EditorHelper.getPlainSpaceWidthFloat(myFixture.editor)
Assert.assertEquals((59 * columnWidth).roundToInt(), visibleArea.x)
}
}

0 comments on commit 62632a4

Please sign in to comment.