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

Use variants for underline dotted. #4703

Merged
merged 27 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
32da1d8
Use variants for underline dotted.
tisilent Aug 21, 2023
cb175e5
Variants to Canvas.
tisilent Aug 21, 2023
c81dfee
Using ext to store variants.
tisilent Aug 21, 2023
fe7f17f
clean
tisilent Aug 22, 2023
5f0647f
clean
tisilent Aug 22, 2023
68ded1a
Merge branch 'master' into underline-dotted
Tyriar Aug 22, 2023
25afc19
Merge branch 'underline-dotted' of https://github.com/tisilent/xterm.…
tisilent Aug 23, 2023
f5d128b
Merge remote-tracking branch 'upstream/master' into pr/tisilent/4703
Tyriar Aug 23, 2023
1f3143c
Remove unused member
Tyriar Aug 23, 2023
2b0106f
Logic move to CellColorResolver
tisilent Aug 24, 2023
e09ed98
Merge branch 'underline-dotted' of https://github.com/tisilent/xterm.…
tisilent Aug 24, 2023
564371e
Clean unused member
tisilent Aug 24, 2023
edfacbb
Adjusting the execution process
tisilent Aug 24, 2023
52c87bc
Add tests for computeVarinatOffset
tisilent Aug 24, 2023
6ace416
Add license header
tisilent Aug 24, 2023
f1f0191
Merge branch 'master' into underline-dotted
Tyriar Aug 24, 2023
f488187
Remove unused private prop
Tyriar Aug 24, 2023
d15abd7
Update src/browser/renderer/shared/CellColorResolver.ts
tisilent Aug 25, 2023
08f7b4c
Update src/browser/renderer/shared/CellColorResolver.ts
tisilent Aug 25, 2023
bec1642
Modify variantOffset calculation logic
tisilent Aug 25, 2023
810e626
Clean
tisilent Aug 25, 2023
42d6c2b
Use hex
tisilent Aug 25, 2023
7fa56fd
Merge branch 'master' into underline-dotted
tisilent Sep 1, 2023
bfa6f65
Merge branch 'master' into underline-dotted
Tyriar Sep 11, 2023
390e4f3
Merge remote-tracking branch 'upstream/master' into pr/tisilent/4703
Tyriar Nov 2, 2023
6214f4c
Varinat -> Variant
Tyriar Nov 2, 2023
2f074b8
Explain VARIANT_OFFSET
Tyriar Nov 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions addons/xterm-addon-canvas/src/BaseRenderLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { ICoreBrowserService, IThemeService } from 'browser/services/Services';
import { ReadonlyColorSet } from 'browser/Types';
import { CellData } from 'common/buffer/CellData';
import { WHITESPACE_CELL_CODE } from 'common/buffer/Constants';
import { IBufferService, IDecorationService, IOptionsService } from 'common/services/Services';
import { IBufferService, IDecorationService, IOptionsService, IUnicodeService } from 'common/services/Services';
import { ICellData, IDisposable } from 'common/Types';
import { Terminal } from 'xterm';
import { IRenderLayer } from './Types';
Expand Down Expand Up @@ -55,10 +55,11 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer
protected readonly _bufferService: IBufferService,
protected readonly _optionsService: IOptionsService,
protected readonly _decorationService: IDecorationService,
protected readonly _coreBrowserService: ICoreBrowserService
protected readonly _coreBrowserService: ICoreBrowserService,
unicodeService: IUnicodeService
) {
super();
this._cellColorResolver = new CellColorResolver(this._terminal, this._selectionModel, this._decorationService, this._coreBrowserService, this._themeService);
this._cellColorResolver = new CellColorResolver(this._terminal, this._selectionModel, this._decorationService, this._coreBrowserService, this._themeService, unicodeService);
this._canvas = document.createElement('canvas');
this._canvas.classList.add(`xterm-${id}-layer`);
this._canvas.style.zIndex = zIndex.toString();
Expand Down Expand Up @@ -367,7 +368,7 @@ export abstract class BaseRenderLayer extends Disposable implements IRenderLayer
*/
protected _drawChars(cell: ICellData, x: number, y: number): void {
const chars = cell.getChars();
this._cellColorResolver.resolve(cell, x, this._bufferService.buffer.ydisp + y);
this._cellColorResolver.resolve(cell, x, this._bufferService.buffer.ydisp + y, this._deviceCellWidth);

if (!this._charAtlas) {
return;
Expand Down
6 changes: 4 additions & 2 deletions addons/xterm-addon-canvas/src/CanvasAddon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ITerminal } from 'browser/Types';
import { EventEmitter, forwardEvent } from 'common/EventEmitter';
import { Disposable, toDisposable } from 'common/Lifecycle';
import { setTraceLogger } from 'common/services/LogService';
import { IBufferService, IDecorationService, ILogService } from 'common/services/Services';
import { IBufferService, IDecorationService, ILogService, IUnicodeService } from 'common/services/Services';
import { ITerminalAddon, Terminal } from 'xterm';
import { CanvasRenderer } from './CanvasRenderer';

Expand Down Expand Up @@ -47,12 +47,14 @@ export class CanvasAddon extends Disposable implements ITerminalAddon {
const decorationService: IDecorationService = unsafeCore._decorationService;
const logService: ILogService = unsafeCore._logService;
const themeService: IThemeService = unsafeCore._themeService;
const unicodeService: IUnicodeService = unsafeCore.unicodeService;

// Set trace logger just in case it hasn't been yet which could happen when the addon is
// bundled separately to the core module
setTraceLogger(logService);

this._renderer = new CanvasRenderer(terminal, screenElement, linkifier, bufferService, charSizeService, optionsService, characterJoinerService, coreService, coreBrowserService, decorationService, themeService);
this._renderer = new CanvasRenderer(terminal, screenElement, linkifier, bufferService, charSizeService, optionsService, characterJoinerService, coreService, coreBrowserService, decorationService, themeService, unicodeService);

this.register(forwardEvent(this._renderer.onChangeTextureAtlas, this._onChangeTextureAtlas));
this.register(forwardEvent(this._renderer.onAddTextureAtlasCanvas, this._onAddTextureAtlasCanvas));
renderService.setRenderer(this._renderer);
Expand Down
13 changes: 7 additions & 6 deletions addons/xterm-addon-canvas/src/CanvasRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { IRenderDimensions, IRenderer, IRequestRedrawEvent } from 'browser/rende
import { ICharSizeService, ICharacterJoinerService, ICoreBrowserService, IThemeService } from 'browser/services/Services';
import { EventEmitter, forwardEvent } from 'common/EventEmitter';
import { Disposable, toDisposable } from 'common/Lifecycle';
import { IBufferService, ICoreService, IDecorationService, IOptionsService } from 'common/services/Services';
import { IBufferService, ICoreService, IDecorationService, IOptionsService, IUnicodeService } from 'common/services/Services';
import { Terminal } from 'xterm';
import { CursorRenderLayer } from './CursorRenderLayer';
import { LinkRenderLayer } from './LinkRenderLayer';
Expand Down Expand Up @@ -43,15 +43,16 @@ export class CanvasRenderer extends Disposable implements IRenderer {
coreService: ICoreService,
private readonly _coreBrowserService: ICoreBrowserService,
decorationService: IDecorationService,
private readonly _themeService: IThemeService
private readonly _themeService: IThemeService,
unicodeService: IUnicodeService
) {
super();
const allowTransparency = this._optionsService.rawOptions.allowTransparency;
this._renderLayers = [
new TextRenderLayer(this._terminal, this._screenElement, 0, allowTransparency, this._bufferService, this._optionsService, characterJoinerService, decorationService, this._coreBrowserService, _themeService),
new SelectionRenderLayer(this._terminal, this._screenElement, 1, this._bufferService, this._coreBrowserService, decorationService, this._optionsService, _themeService),
new LinkRenderLayer(this._terminal, this._screenElement, 2, linkifier2, this._bufferService, this._optionsService, decorationService, this._coreBrowserService, _themeService),
new CursorRenderLayer(this._terminal, this._screenElement, 3, this._onRequestRedraw, this._bufferService, this._optionsService, coreService, this._coreBrowserService, decorationService, _themeService)
new TextRenderLayer(this._terminal, this._screenElement, 0, allowTransparency, this._bufferService, this._optionsService, characterJoinerService, decorationService, this._coreBrowserService, _themeService, unicodeService),
new SelectionRenderLayer(this._terminal, this._screenElement, 1, this._bufferService, this._coreBrowserService, decorationService, this._optionsService, _themeService, unicodeService),
new LinkRenderLayer(this._terminal, this._screenElement, 2, linkifier2, this._bufferService, this._optionsService, decorationService, this._coreBrowserService, _themeService, unicodeService),
new CursorRenderLayer(this._terminal, this._screenElement, 3, this._onRequestRedraw, this._bufferService, this._optionsService, coreService, this._coreBrowserService, decorationService, _themeService, unicodeService)
];
for (const layer of this._renderLayers) {
forwardEvent(layer.onAddTextureAtlasCanvas, this._onAddTextureAtlasCanvas);
Expand Down
7 changes: 4 additions & 3 deletions addons/xterm-addon-canvas/src/CursorRenderLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { toDisposable } from 'common/Lifecycle';
import { isFirefox } from 'common/Platform';
import { ICellData } from 'common/Types';
import { CellData } from 'common/buffer/CellData';
import { IBufferService, ICoreService, IDecorationService, IOptionsService } from 'common/services/Services';
import { IBufferService, ICoreService, IDecorationService, IOptionsService, IUnicodeService } from 'common/services/Services';
import { Terminal } from 'xterm';
import { BaseRenderLayer } from './BaseRenderLayer';

Expand Down Expand Up @@ -39,9 +39,10 @@ export class CursorRenderLayer extends BaseRenderLayer {
private readonly _coreService: ICoreService,
coreBrowserService: ICoreBrowserService,
decorationService: IDecorationService,
themeService: IThemeService
themeService: IThemeService,
unicodeService: IUnicodeService
) {
super(terminal, container, 'cursor', zIndex, true, themeService, bufferService, optionsService, decorationService, coreBrowserService);
super(terminal, container, 'cursor', zIndex, true, themeService, bufferService, optionsService, decorationService, coreBrowserService, unicodeService);
this._state = {
x: 0,
y: 0,
Expand Down
7 changes: 4 additions & 3 deletions addons/xterm-addon-canvas/src/LinkRenderLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { is256Color } from 'browser/renderer/shared/CharAtlasUtils';
import { INVERTED_DEFAULT_COLOR } from 'browser/renderer/shared/Constants';
import { IRenderDimensions } from 'browser/renderer/shared/Types';
import { ICoreBrowserService, IThemeService } from 'browser/services/Services';
import { IBufferService, IDecorationService, IOptionsService } from 'common/services/Services';
import { IBufferService, IDecorationService, IOptionsService, IUnicodeService } from 'common/services/Services';
import { Terminal } from 'xterm';
import { BaseRenderLayer } from './BaseRenderLayer';

Expand All @@ -24,9 +24,10 @@ export class LinkRenderLayer extends BaseRenderLayer {
optionsService: IOptionsService,
decorationService: IDecorationService,
coreBrowserService: ICoreBrowserService,
themeService: IThemeService
themeService: IThemeService,
unicodeService: IUnicodeService
) {
super(terminal, container, 'link', zIndex, true, themeService, bufferService, optionsService, decorationService, coreBrowserService);
super(terminal, container, 'link', zIndex, true, themeService, bufferService, optionsService, decorationService, coreBrowserService, unicodeService);

this.register(linkifier2.onShowLinkUnderline(e => this._handleShowLinkUnderline(e)));
this.register(linkifier2.onHideLinkUnderline(e => this._handleHideLinkUnderline(e)));
Expand Down
7 changes: 4 additions & 3 deletions addons/xterm-addon-canvas/src/SelectionRenderLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { IRenderDimensions } from 'browser/renderer/shared/Types';
import { BaseRenderLayer } from './BaseRenderLayer';
import { IBufferService, IDecorationService, IOptionsService } from 'common/services/Services';
import { IBufferService, IDecorationService, IOptionsService, IUnicodeService } from 'common/services/Services';
import { ICoreBrowserService, IThemeService } from 'browser/services/Services';
import { Terminal } from 'xterm';

Expand All @@ -27,9 +27,10 @@ export class SelectionRenderLayer extends BaseRenderLayer {
coreBrowserService: ICoreBrowserService,
decorationService: IDecorationService,
optionsService: IOptionsService,
themeService: IThemeService
themeService: IThemeService,
unicodeService: IUnicodeService
) {
super(terminal, container, 'selection', zIndex, true, themeService, bufferService, optionsService, decorationService, coreBrowserService);
super(terminal, container, 'selection', zIndex, true, themeService, bufferService, optionsService, decorationService, coreBrowserService, unicodeService);
this._clearState();
}

Expand Down
7 changes: 4 additions & 3 deletions addons/xterm-addon-canvas/src/TextRenderLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { CharData, ICellData } from 'common/Types';
import { AttributeData } from 'common/buffer/AttributeData';
import { CellData } from 'common/buffer/CellData';
import { Content, NULL_CELL_CODE } from 'common/buffer/Constants';
import { IBufferService, IDecorationService, IOptionsService } from 'common/services/Services';
import { IBufferService, IDecorationService, IOptionsService, IUnicodeService } from 'common/services/Services';
import { Terminal } from 'xterm';
import { BaseRenderLayer } from './BaseRenderLayer';
import { GridCache } from './GridCache';
Expand Down Expand Up @@ -39,9 +39,10 @@ export class TextRenderLayer extends BaseRenderLayer {
private readonly _characterJoinerService: ICharacterJoinerService,
decorationService: IDecorationService,
coreBrowserService: ICoreBrowserService,
themeService: IThemeService
themeService: IThemeService,
unicodeService: IUnicodeService
) {
super(terminal, container, 'text', zIndex, alpha, themeService, bufferService, optionsService, decorationService, coreBrowserService);
super(terminal, container, 'text', zIndex, alpha, themeService, bufferService, optionsService, decorationService, coreBrowserService, unicodeService);
this._state = new GridCache<CharData>();
this.register(optionsService.onSpecificOptionChange('allowTransparency', value => this._setTransparency(value)));
}
Expand Down
12 changes: 12 additions & 0 deletions addons/xterm-addon-image/src/ImageStorage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,18 @@ class ExtendedAttrsImage implements IExtendedAttrsImage {
this._ext |= value & (Attributes.CM_MASK | Attributes.RGB_MASK);
}

public get underlineVarinatOffset(): number {
const val = (this._ext & ExtFlags.VARIANT_OFFSET) >> 29;
if (val < 0) {
return val ^ 4294967288;
}
return val;
}
public set underlineVarinatOffset(value: number) {
this._ext &= ~ExtFlags.VARIANT_OFFSET;
this._ext |= (value << 29) & ExtFlags.VARIANT_OFFSET;
}

private _urlId: number = 0;
public get urlId(): number {
return this._urlId;
Expand Down
4 changes: 3 additions & 1 deletion addons/xterm-addon-webgl/src/WebglAddon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ITerminal } from 'browser/Types';
import { EventEmitter, forwardEvent } from 'common/EventEmitter';
import { Disposable, toDisposable } from 'common/Lifecycle';
import { getSafariVersion, isSafari } from 'common/Platform';
import { ICoreService, IDecorationService, ILogService, IOptionsService } from 'common/services/Services';
import { ICoreService, IDecorationService, ILogService, IOptionsService, IUnicodeService } from 'common/services/Services';
import { ITerminalAddon, Terminal } from 'xterm';
import { WebglRenderer } from './WebglRenderer';
import { setTraceLogger } from 'common/services/LogService';
Expand Down Expand Up @@ -54,6 +54,7 @@ export class WebglAddon extends Disposable implements ITerminalAddon {
const decorationService: IDecorationService = unsafeCore._decorationService;
const logService: ILogService = unsafeCore._logService;
const themeService: IThemeService = unsafeCore._themeService;
const unicodeService: IUnicodeService = unsafeCore.unicodeService;

// Set trace logger just in case it hasn't been yet which could happen when the addon is
// bundled separately to the core module
Expand All @@ -68,6 +69,7 @@ export class WebglAddon extends Disposable implements ITerminalAddon {
decorationService,
optionsService,
themeService,
unicodeService,
this._preserveDrawingBuffer
));
this.register(forwardEvent(this._renderer.onContextLoss, this._onContextLoss));
Expand Down
8 changes: 5 additions & 3 deletions addons/xterm-addon-webgl/src/WebglRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { AttributeData } from 'common/buffer/AttributeData';
import { CellData } from 'common/buffer/CellData';
import { Attributes, Content, NULL_CELL_CHAR, NULL_CELL_CODE } from 'common/buffer/Constants';
import { traceCall } from 'common/services/LogService';
import { ICoreService, IDecorationService, IOptionsService } from 'common/services/Services';
import { ICoreService, IDecorationService, IOptionsService, IUnicodeService } from 'common/services/Services';
import { IDisposable, Terminal } from 'xterm';
import { GlyphRenderer } from './GlyphRenderer';
import { RectangleRenderer } from './RectangleRenderer';
Expand Down Expand Up @@ -70,13 +70,14 @@ export class WebglRenderer extends Disposable implements IRenderer {
private readonly _decorationService: IDecorationService,
private readonly _optionsService: IOptionsService,
private readonly _themeService: IThemeService,
private readonly _unicodeService: IUnicodeService,
preserveDrawingBuffer?: boolean
) {
super();

this.register(this._themeService.onChangeColors(() => this._handleColorChange()));

this._cellColorResolver = new CellColorResolver(this._terminal, this._model.selection, this._decorationService, this._coreBrowserService, this._themeService);
this._cellColorResolver = new CellColorResolver(this._terminal, this._model.selection, this._decorationService, this._coreBrowserService, this._themeService, this._unicodeService);

this._core = (this._terminal as any)._core;

Expand Down Expand Up @@ -446,10 +447,11 @@ export class WebglRenderer extends Disposable implements IRenderer {

chars = cell.getChars();
code = cell.getCode();

i = ((y * terminal.cols) + x) * RENDER_MODEL_INDICIES_PER_CELL;

// Load colors/resolve overrides into work colors
this._cellColorResolver.resolve(cell, x, row);
this._cellColorResolver.resolve(cell, x, row, this.dimensions.device.cell.width);

// Override colors for cursor cell
if (isCursorVisible && row === cursorY) {
Expand Down
44 changes: 40 additions & 4 deletions src/browser/renderer/shared/CellColorResolver.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { computeVarinatOffset } from 'browser/renderer/shared/RendererUtils';
import { ISelectionRenderModel } from 'browser/renderer/shared/Types';
import { ICoreBrowserService, IThemeService } from 'browser/services/Services';
import { ReadonlyColorSet } from 'browser/Types';
import { Attributes, BgFlags, FgFlags } from 'common/buffer/Constants';
import { IDecorationService } from 'common/services/Services';
import { Attributes, BgFlags, ExtFlags, FgFlags, NULL_CELL_CODE, UnderlineStyle } from 'common/buffer/Constants';
import { IDecorationService, IUnicodeService } from 'common/services/Services';
import { ICellData } from 'common/Types';
import { Terminal } from 'xterm';

Expand All @@ -13,6 +14,8 @@ let $hasFg = false;
let $hasBg = false;
let $isSelected = false;
let $colors: ReadonlyColorSet | undefined;
let $y = -1;
let $variantOffset = 0;

export class CellColorResolver {
/**
Expand All @@ -30,15 +33,16 @@ export class CellColorResolver {
private readonly _selectionRenderModel: ISelectionRenderModel,
private readonly _decorationService: IDecorationService,
private readonly _coreBrowserService: ICoreBrowserService,
private readonly _themeService: IThemeService
private readonly _themeService: IThemeService,
private readonly _unicodeService: IUnicodeService
) {
}

/**
* Resolves colors for the cell, putting the result into the shared {@link result}. This resolves
* overrides, inverse and selection for the cell which can then be used to feed into the renderer.
*/
public resolve(cell: ICellData, x: number, y: number): void {
public resolve(cell: ICellData, x: number, y: number, deviceCellWidth: number): void {
this.result.bg = cell.bg;
this.result.fg = cell.fg;
this.result.ext = cell.bg & BgFlags.HAS_EXTENDED ? cell.extended.ext : 0;
Expand All @@ -53,6 +57,16 @@ export class CellColorResolver {
$isSelected = false;
$colors = this._themeService.colors;

if ($y !== y) {
$variantOffset = 0;
}
$y = y;

const code = cell.getCode();
if (code === NULL_CELL_CODE && cell.extended.underlineStyle !== UnderlineStyle.DOTTED) {
$variantOffset = 0;
}

// Apply decorations on the bottom layer
this._decorationService.forEachDecorationAtCell(x, y, 'bottom', d => {
if (d.backgroundColorRGB) {
Expand Down Expand Up @@ -133,5 +147,27 @@ export class CellColorResolver {
// Use the override if it exists
this.result.bg = $hasBg ? $bg : this.result.bg;
this.result.fg = $hasFg ? $fg : this.result.fg;

// Reset overrides variantOffset
this.result.ext &= ~ExtFlags.VARIANT_OFFSET;
this.result.ext |= ($variantOffset << 29) & ExtFlags.VARIANT_OFFSET;

// Compute next varinatOffset
tisilent marked this conversation as resolved.
Show resolved Hide resolved
if (cell.extended.underlineStyle === UnderlineStyle.DOTTED) {
if (code !== NULL_CELL_CODE) {
const fontSize = this._terminal.options.fontSize;
const drp = this._coreBrowserService.dpr;
const lineWidth = Math.max(1, Math.floor(fontSize! * drp / 15));
Copy link
Member

Choose a reason for hiding this comment

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

If you pass in IOptionsService you can use rawOptions.fontSize and avoid the unsafe !

tisilent marked this conversation as resolved.
Show resolved Hide resolved
let chWidth: number;
if (typeof code === 'number') {
chWidth = this._unicodeService.wcwidth(code);
} else {
chWidth = this._unicodeService.getStringCellWidth(code);
}
Copy link
Member

Choose a reason for hiding this comment

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

code should always be a number so this isn't necessary. Did you mean to pass in chars?

$variantOffset = computeVarinatOffset(deviceCellWidth * chWidth, lineWidth, $variantOffset);
Copy link
Member

Choose a reason for hiding this comment

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

I was expecting the variant to be based on the left-most pixel of the glyph, so chWidth wouldn't be needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to take a look at the process here.....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. 😄

}
} else {
$variantOffset = 0;
}
}
}