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

Improve handling of fractional width fonts #525

Merged
merged 1 commit into from
Jul 6, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
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)
}
}