Fix #126 - Support multibyte characters in rendering (#127)

* Initial try to change rendering

* Add TODO note

* Add wcwidth

* Merge RenderCache, add wcwidth

* Fix issue with redraw of character, but need to add 'characterWidth' to cell instead of calculating wcwidth everywhere

* Start factoring out characterWidth as a property on the cell

* Refactor to use characterWidth property instead of calling wcwidth everywhere

* Add cursorPixelWidth to state, and pick up in cursor component

* Try removing optimizations to see if that addresses renderer - might need to 'force render'

* Update logic so that we only dirty cells if the new character is smaller in width than the old character

* Add orce concept to DeltaRegionTracker to handle clearing cells in the multi character width case

* Fix tslint; remove whitespace setting that fights with TS formatter

* Fix comment

* Fix spacing for fontwidth

browser/src/DeltaRegionTracker.ts

@@ -4,11 +4,12 @@ import { Grid } from "./Grid"
 export interface IDeltaCellPosition {
     x: number
     y: number
+    force?: boolean
 }
 
 export interface IDeltaRegionTracker {
     getModifiedCells(): IDeltaCellPosition[]
-    notifyCellModified(x: number, y: number): void
+    notifyCellModified(x: number, y: number, force?: boolean): void
     notifyCellRendered(x: number, y: number): void
 }
 
@@ -52,14 +53,15 @@ export class IncrementalDeltaRegionTracker implements IDeltaRegionTracker {
         this._cells = this._cells.filter((dcp) => this._dirtyGrid.getCell(dcp.x, dcp.y))
     }
 
-    public notifyCellModified(x: number, y: number): void {
-        if (this._dirtyGrid.getCell(x, y)) {
+    public notifyCellModified(x: number, y: number, force?: boolean): void {
+        if (this._dirtyGrid.getCell(x, y) && !force) {
             return
         }
 
         this._cells.push({
             x,
             y,
+            force,
         })
 
         this._dirtyGrid.setCell(x, y, true)

browser/src/Renderer/CanvasRenderer.ts

@@ -18,7 +18,7 @@ export class CanvasRenderer implements INeovimRenderer {
         // Assert canvas
         this._canvas = element
         this._setContextDimensions()
-        this._canvasContext = <any> this._canvas.getContext("2d") // FIXME: null
+        this._canvasContext = <any>this._canvas.getContext("2d") // FIXME: null
 
         this._renderCache = new RenderCache(this._canvasContext, this._getPixelRatio())
     }
@@ -54,21 +54,24 @@ export class CanvasRenderer implements INeovimRenderer {
             if (cell) {
                 const lastRenderedCell = this._lastRenderedCell.getCell(x, y)
 
-                if (lastRenderedCell === cell) {
-                    deltaRegionTracker.notifyCellRendered(x, y)
-                    return
+                if (!pos.force) {
+                    if (lastRenderedCell === cell) {
+                        deltaRegionTracker.notifyCellRendered(x, y)
+                        return
+                    }
+
+                    if (lastRenderedCell
+                        && lastRenderedCell.backgroundColor === cell.backgroundColor
+                        && lastRenderedCell.character === cell.character
+                        && lastRenderedCell.foregroundColor === cell.foregroundColor) {
+                        this._lastRenderedCell.setCell(x, y, cell)
+                        deltaRegionTracker.notifyCellRendered(x, y)
+                        return
+                    }
                 }
 
-                if (lastRenderedCell
-                    && lastRenderedCell.backgroundColor === cell.backgroundColor
-                    && lastRenderedCell.character === cell.character
-                    && lastRenderedCell.foregroundColor === cell.foregroundColor) {
-                    this._lastRenderedCell.setCell(x, y, cell)
-                    deltaRegionTracker.notifyCellRendered(x, y)
-                    return
-                }
-
-                this._canvasContext.clearRect(drawX, drawY, fontWidth, fontHeight)
+                const calculatedWidth = cell.characterWidth
+                this._canvasContext.clearRect(drawX, drawY, fontWidth * calculatedWidth, fontHeight)
 
                 const defaultBackgroundColor = "rgba(255, 255, 255, 0)"
                 let backgroundColor = defaultBackgroundColor
@@ -85,9 +88,9 @@ export class CanvasRenderer implements INeovimRenderer {
                         foregroundColor,
                         drawX,
                         drawY,
-                        <any> screenInfo.fontFamily, // FIXME: null
-                        <any> screenInfo.fontSize, // FIXME: null
-                        fontWidth,
+                        <any>screenInfo.fontFamily, // FIXME: null
+                        <any>screenInfo.fontSize, // FIXME: null
+                        fontWidth * cell.characterWidth,
                         fontHeight)
                 } else if (backgroundColor !== defaultBackgroundColor) {
                     this._canvasContext.fillStyle = backgroundColor

browser/src/Screen.ts

@@ -4,6 +4,8 @@ import { Grid } from "./Grid"
 
 export type Mode = "insert" | "normal"
 
+const wcwidth = require("wcwidth") // tslint:disable-line no-var-requires
+
 export interface IHighlight {
     bold?: boolean
     italic?: boolean
@@ -36,6 +38,13 @@ export interface IScreen {
 
 export interface ICell {
     character: string
+
+    /**
+     * Specify the width of the character. Some Unicode characters will take up multiple
+     * cells, like `한`, which needs to be accounted for in rendering.
+     */
+    characterWidth: number
+
     foregroundColor?: string
     backgroundColor?: string
 }
@@ -133,6 +142,7 @@ export class NeovimScreen implements IScreen {
     public getCell(x: number, y: number): ICell {
         const defaultCell = {
             character: "",
+            characterWidth: 1,
         }
 
         const cell = this._grid.getCell(x, y)
@@ -165,11 +175,18 @@ export class NeovimScreen implements IScreen {
                 const col = this._cursorColumn
 
                 for (let i = 0; i < characters.length; i++) {
+                    const character = characters[i]
+
+                    const characterWidth = wcwidth(character)
+
                     this._setCell(col + i, row, {
                         foregroundColor,
                         backgroundColor,
-                        character: characters[i],
+                        character,
+                        characterWidth,
                     })
+
+                    i += characterWidth - 1
                 }
 
                 this._cursorColumn += characters.length
@@ -185,6 +202,7 @@ export class NeovimScreen implements IScreen {
                         foregroundColor,
                         backgroundColor,
                         character: "",
+                        characterWidth: 1,
                     })
                 }
                 break
@@ -276,6 +294,20 @@ export class NeovimScreen implements IScreen {
                 return
             }
         }
+
+        // In the case where the new character is smaller (in character width) than the old character,
+        // we need to dirty the additional cells so that the delta tracker knows it should clear
+        // the item
+        const currentCharacterWidth = currentCell ? currentCell.characterWidth : 1
+        const newCharacterWidth = cell.characterWidth
+
+        if (newCharacterWidth < currentCharacterWidth) {
+            for (let offsetX = 1; offsetX < currentCharacterWidth; offsetX++) {
+                // Add 'force' option that is passed to renderer
+                this._deltaTracker.notifyCellModified(x + offsetX, y, true)
+            }
+        }
+
         this._deltaTracker.notifyCellModified(x, y)
         this._grid.setCell(x, y, cell)
     }

browser/src/UI/ActionCreators.ts

@@ -1,10 +1,11 @@
-export const setCursorPosition = (cursorPixelX: any, cursorPixelY: any, fontPixelWidth: any, fontPixelHeight: any) => ({
+export const setCursorPosition = (cursorPixelX: any, cursorPixelY: any, fontPixelWidth: any, fontPixelHeight: any, cursorPixelWidth: number) => ({
     type: "SET_CURSOR_POSITION",
     payload: {
         pixelX: cursorPixelX,
         pixelY: cursorPixelY,
         fontPixelWidth,
         fontPixelHeight,
+        cursorPixelWidth,
     },
 })
 

browser/src/UI/Actions.ts

@@ -5,6 +5,7 @@ export interface ISetCursorPositionAction {
         pixelY: number,
         fontPixelWidth: number,
         fontPixelHeight: number,
+        cursorPixelWidth: number,
     }
 }
 

browser/src/UI/Reducer.ts

@@ -17,6 +17,7 @@ export const reducer = (s: State.IState, a: Actions.Action) => {
                 cursorPixelY: a.payload.pixelY,
                 fontPixelWidth: a.payload.fontPixelWidth,
                 fontPixelHeight: a.payload.fontPixelHeight,
+                cursorPixelWidth: a.payload.cursorPixelWidth,
             })
         case "SET_MODE":
             return { ...s, ...{ mode: a.payload.mode } }

browser/src/UI/State.ts

@@ -1,6 +1,7 @@
 export interface IState {
     cursorPixelX: number
     cursorPixelY: number
+    cursorPixelWidth: number
     fontPixelWidth: number
     fontPixelHeight: number
     mode: string

browser/src/UI/components/Cursor.tsx

@@ -33,11 +33,11 @@ class _Cursor extends React.Component<CursorProps, void> {
     }
 }
 
-const mapStateToProps =(state: State.IState) => {
+const mapStateToProps = (state: State.IState) => {
     return {
         x: state.cursorPixelX,
         y: state.cursorPixelY,
-        width: state.fontPixelWidth,
+        width: state.cursorPixelWidth,
         height: state.fontPixelHeight,
         mode: state.mode,
         color: state.foregroundColor,

browser/src/UI/index.tsx

@@ -16,11 +16,14 @@ import { reducer } from "./Reducer"
 
 import { InstallHelp } from "./components/InstallHelp"
 
+import { IScreen } from "./../Screen"
+
 export const events = new EventEmitter()
 
 let state: State.IState = {
     cursorPixelX: 10,
     cursorPixelY: 10,
+    cursorPixelWidth: 10,
     fontPixelWidth: 10,
     fontPixelHeight: 10,
     mode: "normal",
@@ -45,8 +48,13 @@ export function setBackgroundColor(backgroundColor: string): void {
     backgroundColorElement.style.opacity = Config.getValue<string>("prototype.editor.backgroundOpacity")
 }
 
-export function setCursorPosition(cursorPixelX: number, cursorPixelY: number, fontPixelWidth: number, fontPixelHeight: number): void {
-    store.dispatch(ActionCreators.setCursorPosition(cursorPixelX, cursorPixelY, fontPixelWidth, fontPixelHeight))
+export function setCursorPosition(screen: IScreen): void {
+        const cursorWidth = screen.getCell(screen.cursorColumn, screen.cursorRow).characterWidth
+        _setCursorPosition(screen.cursorColumn * screen.fontWidthInPixels, screen.cursorRow * screen.fontHeightInPixels, screen.fontWidthInPixels, screen.fontHeightInPixels, cursorWidth * screen.fontWidthInPixels)
+}
+
+function _setCursorPosition(cursorPixelX: number, cursorPixelY: number, fontPixelWidth: number, fontPixelHeight: number, cursorPixelWidth: number): void {
+    store.dispatch(ActionCreators.setCursorPosition(cursorPixelX, cursorPixelY, fontPixelWidth, fontPixelHeight, cursorPixelWidth))
 }
 
 // TODO: Can we use bindaction creators for this?

browser/src/index.tsx

@@ -163,7 +163,7 @@ const start = (args: string[]) => {
 
     const renderFunction = () => {
         if (pendingTimeout) {
-            UI.setCursorPosition(screen.cursorColumn * screen.fontWidthInPixels, screen.cursorRow * screen.fontHeightInPixels, screen.fontWidthInPixels, screen.fontHeightInPixels)
+            UI.setCursorPosition(screen)
         }
 
         renderer.update(screen, deltaRegion)
@@ -176,7 +176,7 @@ const start = (args: string[]) => {
 
     const updateFunction = () => {
         // TODO: Move cursor to component
-        UI.setCursorPosition(screen.cursorColumn * screen.fontWidthInPixels, screen.cursorRow * screen.fontHeightInPixels, screen.fontWidthInPixels, screen.fontHeightInPixels)
+        UI.setCursorPosition(screen)
 
         UI.setBackgroundColor(screen.backgroundColor)
 

browser/tslint.json

@@ -20,6 +20,14 @@
       "allow-pascal-case",
       "ban-keywords",
       "check-format"
+    ],
+    "whitespace": [
+        true,
+        "check-branch",
+        "check-decl",
+        "check-operator",
+        "check-separator",
+        "check-type"
     ]
   }
 }

package.json

@@ -54,7 +54,8 @@
     "react-redux": "^4.4.5",
     "recursive-readdir": "^2.1.0",
     "redux": "^3.5.2",
-    "typescript": "^2.0.7"
+    "typescript": "^2.0.7",
+    "wcwidth": "^1.0.1"
   },
   "devDependencies": {
     "@types/electron": "^1.4.27",