Skip to content

Commit

Permalink
updates to match xtermjs's height calcuation. there was discrepancy w…
Browse files Browse the repository at this point in the history
…hen devicePixelRatio was not an integer leading to mis-sized terminal windows and cut off lines #302 (#305)
  • Loading branch information
sawka authored Feb 20, 2024
1 parent 0f030ff commit d453368
Show file tree
Hide file tree
Showing 13 changed files with 137 additions and 21 deletions.
7 changes: 6 additions & 1 deletion src/app/common/modals/modals.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type OV<V> = mobx.IObservableValue<V>;
type OArr<V> = mobx.IObservableArray<V>;

const RemotePtyRows = 9;
const RemotePtyTotalRows = 25;
const RemotePtyCols = 80;
const NumOfLines = 50;
const PasswordUnchangedSentinel = "--unchanged--";
Expand Down Expand Up @@ -1184,7 +1185,11 @@ class ViewRemoteConnDetailModal extends React.Component<{}, {}> {
ref={this.termRef}
data-remoteid={remote.remoteid}
style={{
height: textmeasure.termHeightFromRows(RemotePtyRows, termFontSize),
height: textmeasure.termHeightFromRows(
RemotePtyRows,
termFontSize,
RemotePtyTotalRows
),
width: termWidth,
}}
></div>
Expand Down
7 changes: 6 additions & 1 deletion src/app/common/modals/viewremoteconndetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import * as textmeasure from "../../../util/textmeasure";
import "./viewremoteconndetail.less";

const RemotePtyRows = 9;
const RemotePtyTotalRows = 25;
const RemotePtyCols = 80;

@mobxReact.observer
Expand Down Expand Up @@ -371,7 +372,11 @@ class ViewRemoteConnDetailModal extends React.Component<{}, {}> {
ref={this.termRef}
data-remoteid={remote.remoteid}
style={{
height: textmeasure.termHeightFromRows(RemotePtyRows, termFontSize),
height: textmeasure.termHeightFromRows(
RemotePtyRows,
termFontSize,
RemotePtyTotalRows
),
width: termWidth,
}}
></div>
Expand Down
3 changes: 2 additions & 1 deletion src/app/connections_deprecated/connections.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type OArr<V> = mobx.IObservableArray<V>;
type OMap<K, V> = mobx.ObservableMap<K, V>;

const RemotePtyRows = 8;
const RemotePtyTotalRows = 25;
const RemotePtyCols = 80;
const PasswordUnchangedSentinel = "--unchanged--";

Expand Down Expand Up @@ -1049,7 +1050,7 @@ class RemoteDetailView extends React.Component<{ model: RemotesModalModel; remot
ref={this.termRef}
data-remoteid={remote.remoteid}
style={{
height: textmeasure.termHeightFromRows(RemotePtyRows, termFontSize),
height: textmeasure.termHeightFromRows(RemotePtyRows, termFontSize, RemotePtyTotalRows),
width: termWidth,
}}
></div>
Expand Down
2 changes: 1 addition & 1 deletion src/app/line/linecomps.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ class LineCmd extends React.Component<
let height = 45 + 24; // height of zero height terminal
const usedRows = screen.getUsedRows(lineutil.getRendererContext(line), line, cmd, width);
if (usedRows > 0) {
height = 48 + 24 + termHeightFromRows(usedRows, GlobalModel.termFontSize.get());
height = 48 + 24 + termHeightFromRows(usedRows, GlobalModel.termFontSize.get(), cmd.getTermMaxRows());
}
return height;
}
Expand Down
1 change: 1 addition & 0 deletions src/app/line/lines.less
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@
padding: 0 0 10px 0;
flex-grow: 1;
position: relative;
overflow-x: hidden;

&::-webkit-scrollbar-thumb {
background-color: transparent !important;
Expand Down
2 changes: 0 additions & 2 deletions src/app/magiclayout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ let MagicLayout = {
ScreenMinContentSize: 100,
ScreenMaxContentSize: 5000,

// the 3 is for descenders, which get cut off in the terminal without this
TermDescendersHeight: 3,
TermWidthBuffer: 15,

TabWidth: 154,
Expand Down
14 changes: 12 additions & 2 deletions src/app/workspace/screen/screenview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,16 @@ import { If, For } from "tsx-control-statements/components";
import cn from "classnames";
import { debounce } from "throttle-debounce";
import dayjs from "dayjs";
import { GlobalCommandRunner, TabColors, TabIcons, ForwardLineContainer, GlobalModel, ScreenLines, Screen, Session } from "../../../model/model";
import {
GlobalCommandRunner,
TabColors,
TabIcons,
ForwardLineContainer,
GlobalModel,
ScreenLines,
Screen,
Session,
} from "../../../model/model";
import type { LineType, RenderModeType, LineFactoryProps } from "../../../types/types";
import * as T from "../../../types/types";
import localizedFormat from "dayjs/plugin/localizedFormat";
Expand Down Expand Up @@ -114,6 +123,7 @@ class ScreenView extends React.Component<{ session: Session; screen: Screen }, {
return <div className="screen-view" ref={this.screenViewRef}></div>;
}
let fontSize = GlobalModel.termFontSize.get();
let dprStr = sprintf("%0.3f", GlobalModel.devicePixelRatio.get());
let viewOpts = screen.viewOpts.get();
let hasSidebar = viewOpts?.sidebar?.open;
let winWidth = "100%";
Expand Down Expand Up @@ -150,7 +160,7 @@ class ScreenView extends React.Component<{ session: Session; screen: Screen }, {
return (
<div className="screen-view" data-screenid={screen.screenId} ref={this.screenViewRef}>
<ScreenWindowView
key={screen.screenId + ":" + fontSize}
key={screen.screenId + ":" + fontSize + ":" + dprStr}
session={session}
screen={screen}
width={winWidth}
Expand Down
42 changes: 39 additions & 3 deletions src/electron/emain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ function readAuthKey() {
return authKeyStr.trim();
}

let cmdOrAlt = process.platform === "darwin" ? "Cmd" : "Alt";
let menuTemplate = [
{
role: "appMenu",
Expand Down Expand Up @@ -198,9 +199,41 @@ let menuTemplate = [
{ role: "reload", accelerator: "Option+R" },
{ role: "toggleDevTools" },
{ type: "separator" },
{ role: "resetZoom" },
{ role: "zoomIn" },
{ role: "zoomOut" },
{
label: "Actual Size",
accelerator: cmdOrAlt + "+0",
click: () => {
if (MainWindow == null) {
return;
}
MainWindow.webContents.setZoomFactor(1);
MainWindow.webContents.send("zoom-changed");
},
},
{
label: "Zoom In",
accelerator: cmdOrAlt + "+Plus",
click: () => {
if (MainWindow == null) {
return;
}
const zoomFactor = MainWindow.webContents.getZoomFactor();
MainWindow.webContents.setZoomFactor(zoomFactor * 1.1);
MainWindow.webContents.send("zoom-changed");
},
},
{
label: "Zoom Out",
accelerator: cmdOrAlt + "+-",
click: () => {
if (MainWindow == null) {
return;
}
const zoomFactor = MainWindow.webContents.getZoomFactor();
MainWindow.webContents.setZoomFactor(zoomFactor / 1.1);
MainWindow.webContents.send("zoom-changed");
},
},
{ type: "separator" },
{ role: "togglefullscreen" },
],
Expand Down Expand Up @@ -367,6 +400,9 @@ function createMainWindow(clientData) {
win.on("close", () => {
MainWindow = null;
});
win.webContents.on("zoom-changed", (e) => {
win.webContents.send("zoom-changed");
});
win.webContents.setWindowOpenHandler(({ url, frameName }) => {
if (url.startsWith("https://docs.waveterm.dev/")) {
console.log("openExternal docs", url);
Expand Down
1 change: 1 addition & 0 deletions src/electron/preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ contextBridge.exposeInMainWorld("api", {
onWCmd: (callback) => ipcRenderer.on("w-cmd", callback),
onPCmd: (callback) => ipcRenderer.on("p-cmd", callback),
onRCmd: (callback) => ipcRenderer.on("r-cmd", callback),
onZoomChanged: (callback) => ipcRenderer.on("zoom-changed", callback),
onMetaArrowUp: (callback) => ipcRenderer.on("meta-arrowup", callback),
onMetaArrowDown: (callback) => ipcRenderer.on("meta-arrowdown", callback),
onMetaPageUp: (callback) => ipcRenderer.on("meta-pageup", callback),
Expand Down
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { sprintf } from "sprintf-js";
import { App } from "./app/app";
import * as DOMPurify from "dompurify";
import { loadFonts } from "./util/util";
import * as textmeasure from "./util/textmeasure";

// @ts-ignore
let VERSION = __WAVETERM_VERSION__;
Expand All @@ -28,5 +29,6 @@ document.addEventListener("DOMContentLoaded", () => {
(window as any).mobx = mobx;
(window as any).sprintf = sprintf;
(window as any).DOMPurify = DOMPurify;
(window as any).textmeasure = textmeasure;

console.log("WaveTerm", VERSION, BUILD);
40 changes: 37 additions & 3 deletions src/model/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ import {
windowHeightToRows,
termWidthFromCols,
termHeightFromRows,
clearMonoFontCache,
} from "../util/textmeasure";
import dayjs from "dayjs";
import localizedFormat from "dayjs/plugin/localizedFormat";
Expand Down Expand Up @@ -203,6 +204,7 @@ type ElectronApi = {
onPCmd: (callback: (mods: KeyModsType) => void) => void;
onRCmd: (callback: (mods: KeyModsType) => void) => void;
onWCmd: (callback: (mods: KeyModsType) => void) => void;
onZoomChanged: (callback: () => void) => void;
onMenuItemAbout: (callback: () => void) => void;
onMetaArrowUp: (callback: () => void) => void;
onMetaArrowDown: (callback: () => void) => void;
Expand Down Expand Up @@ -306,6 +308,11 @@ class Cmd {
return this.data.get().termopts;
}

getTermMaxRows(): number {
let termOpts = this.getTermOpts();
return termOpts?.rows;
}

getCmdStr(): string {
return this.data.get().cmdstr;
}
Expand Down Expand Up @@ -741,7 +748,7 @@ class Screen {
getMaxContentSize(): WindowSize {
if (this.lastScreenSize == null) {
let width = termWidthFromCols(80, GlobalModel.termFontSize.get());
let height = termHeightFromRows(25, GlobalModel.termFontSize.get());
let height = termHeightFromRows(25, GlobalModel.termFontSize.get(), 25);
return { width, height };
}
let winSize = this.lastScreenSize;
Expand All @@ -755,7 +762,7 @@ class Screen {
getIdealContentSize(): WindowSize {
if (this.lastScreenSize == null) {
let width = termWidthFromCols(80, GlobalModel.termFontSize.get());
let height = termHeightFromRows(25, GlobalModel.termFontSize.get());
let height = termHeightFromRows(25, GlobalModel.termFontSize.get(), 25);
return { width, height };
}
let winSize = this.lastScreenSize;
Expand Down Expand Up @@ -2394,7 +2401,7 @@ class HistoryViewModel {
} else {
this.activeItem.set(hitem.historyid);
let width = termWidthFromCols(80, GlobalModel.termFontSize.get());
let height = termHeightFromRows(25, GlobalModel.termFontSize.get());
let height = termHeightFromRows(25, GlobalModel.termFontSize.get(), 25);
this.specialLineContainer = new SpecialLineContainer(
this,
{ width, height },
Expand Down Expand Up @@ -3449,6 +3456,9 @@ class Model {
lineSettingsModal: OV<number> = mobx.observable.box(null, {
name: "lineSettingsModal",
}); // linenum
devicePixelRatio: OV<number> = mobx.observable.box(window.devicePixelRatio, {
name: "devicePixelRatio",
});
remotesModalModel: RemotesModalModel;
remotesModel: RemotesModel;

Expand Down Expand Up @@ -3513,6 +3523,7 @@ class Model {
getApi().onPCmd(this.onPCmd.bind(this));
getApi().onWCmd(this.onWCmd.bind(this));
getApi().onRCmd(this.onRCmd.bind(this));
getApi().onZoomChanged(this.onZoomChanged.bind(this));
getApi().onMenuItemAbout(this.onMenuItemAbout.bind(this));
getApi().onMetaArrowUp(this.onMetaArrowUp.bind(this));
getApi().onMetaArrowDown(this.onMetaArrowDown.bind(this));
Expand Down Expand Up @@ -3824,6 +3835,29 @@ class Model {
}
}

onZoomChanged(): void {
mobx.action(() => {
this.devicePixelRatio.set(window.devicePixelRatio);
clearMonoFontCache();
})();
}

getSelectedTermWrap(): TermWrap {
let screen = this.getActiveScreen();
if (screen == null) {
return null;
}
let lineNum = screen.selectedLine.get();
if (lineNum == null) {
return null;
}
let line = screen.getLineByNum(lineNum);
if (line == null) {
return null;
}
return screen.getTermWrap(line.lineid);
}

clearModals(): boolean {
let didSomething = false;
mobx.action(() => {
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/terminal/terminal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ class TerminalRenderer extends React.Component<
.get();
let cmd = screen.getCmd(line); // will not be null
let usedRows = screen.getUsedRows(lineutil.getRendererContext(line), line, cmd, width);
let termHeight = termHeightFromRows(usedRows, GlobalModel.termFontSize.get());
let termHeight = termHeightFromRows(usedRows, GlobalModel.termFontSize.get(), cmd.getTermMaxRows());
if (usedRows === 0) {
termHeight = 0;
}
Expand All @@ -169,6 +169,7 @@ class TerminalRenderer extends React.Component<
{ "zero-height": termHeight == 0 },
{ collapsed: collapsed }
)}
data-usedrows={usedRows}
>
<If condition={!isFocused}>
<div key="term-block" className="term-block" onClick={this.clickTermBlock}></div>
Expand Down
34 changes: 28 additions & 6 deletions src/util/textmeasure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ function getMonoFontSize(fontSize: number): { height: number; width: number } {
return size;
}

function clearMonoFontCache(): void {
MonoFontSizes = [];
}

function measureText(
text: string,
textOpts?: { pre?: boolean; mono?: boolean; fontSize?: number | string }
Expand Down Expand Up @@ -57,8 +61,9 @@ function measureText(
throw new Error("cannot measure text, no #measure div");
}
measureDiv.replaceChildren(textElem);
let rect = textElem.getBoundingClientRect();
return { width: rect.width, height: Math.ceil(rect.height) };
let height = textElem.offsetHeight;
let width = textElem.offsetWidth;
return { width: width, height: Math.ceil(height) };
}

function windowWidthToCols(width: number, fontSize: number): number {
Expand All @@ -82,10 +87,27 @@ function termWidthFromCols(cols: number, fontSize: number): number {
return Math.ceil(dr.width * cols) + MagicLayout.TermWidthBuffer;
}

function termHeightFromRows(rows: number, fontSize: number): number {
// we need to match the xtermjs calculation in CharSizeService.ts and DomRenderer.ts
// it does some crazy rounding depending on the value of window.devicePixelRatio
// works out to `realHeight = round(ceil(height * dpr) * rows / dpr) / rows`
// their calculation is based off the "totalRows" (so that argument has been added)
function termHeightFromRows(rows: number, fontSize: number, totalRows: number): number {
let dr = getMonoFontSize(fontSize);
// TODO: replace the TermDescendersHeight with some calculation based on termFontSize.
return Math.ceil(dr.height * rows) + MagicLayout.TermDescendersHeight;
const dpr = window.devicePixelRatio;
if (totalRows == null || totalRows == 0) {
totalRows = rows > 25 ? rows : 25;
}
let realHeight = Math.round((Math.ceil(dr.height * dpr) * totalRows) / dpr) / totalRows;
return Math.ceil(realHeight * rows);
}

export { measureText, getMonoFontSize, windowWidthToCols, windowHeightToRows, termWidthFromCols, termHeightFromRows };
export {
measureText,
getMonoFontSize,
windowWidthToCols,
windowHeightToRows,
termWidthFromCols,
termHeightFromRows,
clearMonoFontCache,
MonoFontSizes,
};

0 comments on commit d453368

Please sign in to comment.