Skip to content

Make Terminal operate on and cache colors instead of RGB values #2481

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.

Sign up for GitHub

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

Draft
HeikoKlare wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Make Terminal operate on and cache colors instead of RGB values #2481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions ...pse.terminal.control/src/org/eclipse/terminal/internal/emulator/VT100TerminalControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
import org.eclipse.swt.events.KeyListener;
import org.eclipse.swt.events.MouseAdapter;
import org.eclipse.swt.events.MouseEvent;
import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.Font;
import org.eclipse.swt.graphics.RGB;
import org.eclipse.swt.layout.GridData;
Expand Down Expand Up @@ -678,7 +679,7 @@ private void updatePreferences(PropertyChangeEvent unused) {
}

private void onTerminalColorsChanged() {
Map<TerminalColor, RGB> map = new EnumMap<>(TerminalColor.class);
Map<TerminalColor, Color> map = new EnumMap<>(TerminalColor.class);
TerminalColor[] values = TerminalColor.values();
for (TerminalColor terminalColor : values) {
RGB rgb = null;
Expand All @@ -694,7 +695,7 @@ private void onTerminalColorsChanged() {
if (rgb == null) {
rgb = TerminalColorPresets.INSTANCE.getDefaultPreset().getRGB(terminalColor);
Copy link
Contributor

@laeubi laeubi Feb 26, 2026

Choose a reason for hiding this comment

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

here as well ...

}
map.put(terminalColor, rgb);
map.put(terminalColor, new Color(rgb));
}
fCtlText.updateColors(map);
}
Expand Down
3 changes: 1 addition & 2 deletions ...eclipse.terminal.control/src/org/eclipse/terminal/internal/textcanvas/ILinelRenderer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.RGB;
import org.eclipse.terminal.model.TerminalColor;

/**
Expand All @@ -38,7 +37,7 @@ public interface ILinelRenderer {
*/
void updateFont(String fontName);

void updateColors(Map<TerminalColor, RGB> map);
void updateColors(Map<TerminalColor, Color> map);

void setInvertedColors(boolean invert);

Expand Down
25 changes: 13 additions & 12 deletions ...s/org.eclipse.terminal.control/src/org/eclipse/terminal/internal/textcanvas/StyleMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Map;

import org.eclipse.jface.resource.JFaceResources;
import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.Font;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Point;
Expand All @@ -46,7 +47,7 @@ public class StyleMap {
private boolean fInvertColors;
private boolean fProportional;
private final int[] fOffsets = new int[256];
private final Map<TerminalColor, RGB> fColorMap = new EnumMap<>(TerminalColor.class);
private final Map<TerminalColor, Color> fColorMap = new EnumMap<>(TerminalColor.class);

public StyleMap() {
fDefaultStyle = TerminalStyle.getDefaultStyle();
Expand All @@ -55,11 +56,11 @@ public StyleMap() {
}

private void initColors() {
Map<TerminalColor, RGB> map = new EnumMap<>(TerminalColor.class);
Map<TerminalColor, Color> map = new EnumMap<>(TerminalColor.class);
TerminalColor[] values = TerminalColor.values();
for (TerminalColor terminalColor : values) {
RGB rgb = TerminalColorPresets.INSTANCE.getDefaultPreset().getRGB(terminalColor);
map.put(terminalColor, rgb);
Color color = new Color(TerminalColorPresets.INSTANCE.getDefaultPreset().getRGB(terminalColor));
map.put(terminalColor, color);
}
updateColors(map);
}
Expand All @@ -68,11 +69,11 @@ private void initFont() {
updateFont(ITerminalConstants.FONT_DEFINITION);
}

private RGB getRGB(TerminalColor color) {
private Color getColor(TerminalColor color) {
return fColorMap.get(color);
}

public RGB getForegrondRGB(TerminalStyle style) {
public Color getForegroundColor(TerminalStyle style) {
style = defaultIfNull(style);
RGB foregroundRGB;
if (style.isReverse()) {
Expand All @@ -81,7 +82,7 @@ public RGB getForegrondRGB(TerminalStyle style) {
foregroundRGB = style.getForegroundRGB();
}
if (foregroundRGB != null) {
return foregroundRGB;
return new Color(foregroundRGB);
}

TerminalColor color;
Expand All @@ -96,10 +97,10 @@ public RGB getForegrondRGB(TerminalStyle style) {
}

color = color.convertColor(fInvertColors, style.isBold());
return getRGB(color);
return getColor(color);
}

public RGB getBackgroundRGB(TerminalStyle style) {
public Color getBackgroundColor(TerminalStyle style) {
style = defaultIfNull(style);
RGB backgroundRGB;
if (style.isReverse()) {
Expand All @@ -108,7 +109,7 @@ public RGB getBackgroundRGB(TerminalStyle style) {
backgroundRGB = style.getBackgroundRGB();
}
if (backgroundRGB != null) {
return backgroundRGB;
return new Color(backgroundRGB);
}

TerminalColor color;
Expand All @@ -123,7 +124,7 @@ public RGB getBackgroundRGB(TerminalStyle style) {
}

color = color.convertColor(fInvertColors, style.isBold());
return getRGB(color);
return getColor(color);
}

private TerminalStyle defaultIfNull(TerminalStyle style) {
Expand Down Expand Up @@ -267,7 +268,7 @@ public int getCharOffset(char c) {
return fOffsets[c];
}

public void updateColors(Map<TerminalColor, RGB> colorMap) {
public void updateColors(Map<TerminalColor, Color> colorMap) {
fColorMap.putAll(colorMap);
}
}
3 changes: 1 addition & 2 deletions ...org.eclipse.terminal.control/src/org/eclipse/terminal/internal/textcanvas/TextCanvas.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.eclipse.swt.graphics.Device;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.graphics.RGB;
import org.eclipse.swt.graphics.Rectangle;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.terminal.control.ITerminalMouseListener;
Expand Down Expand Up @@ -650,7 +649,7 @@ public void updateFont(String fontName) {
calculateGrid();
}

public void updateColors(Map<TerminalColor, RGB> map) {
public void updateColors(Map<TerminalColor, Color> map) {
fCellRenderer.updateColors(map);
redraw();
}
Expand Down
18 changes: 8 additions & 10 deletions ...lipse.terminal.control/src/org/eclipse/terminal/internal/textcanvas/TextLineRenderer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.graphics.RGB;
import org.eclipse.terminal.connector.Logger;
import org.eclipse.terminal.internal.control.impl.TerminalPlugin;
import org.eclipse.terminal.model.ITerminalTextDataReadOnly;
Expand Down Expand Up @@ -86,8 +85,8 @@ public void drawLine(ITextCanvasModel model, GC gc, int line, int x, int y, int
int colStart = line == hsStart.y ? hsStart.x : 0;
int colEnd = line == hsEnd.y ? hsEnd.x : getTerminalText().getWidth();
if (colStart < colEnd) {
RGB defaultFg = fStyleMap.getForegrondRGB(null);
doubleBufferGC.setForeground(new Color(doubleBufferGC.getDevice(), defaultFg));
Color defaultFg = fStyleMap.getForegroundColor(null);
doubleBufferGC.setForeground(defaultFg);
Comment on lines +88 to +89
Copy link
Contributor

@laeubi laeubi Feb 26, 2026

Choose a reason for hiding this comment

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

as we have Color getDefaultBackgroundColor() should the not better be a Color getForegroundColor() as well?

This would prevent repeating the code used here and in setupGC

Copy link
Contributor Author

@HeikoKlare HeikoKlare Feb 26, 2026

Choose a reason for hiding this comment

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

It's not a repetition as here it's about the default color (style == null) and in setupGC its the style-specific color. We can of course introduce additional methods like getDefaultForegroundColor() or the like, but it's will only be for consistency, not for reuse.

drawUnderline(doubleBufferGC, colStart, colEnd);
}
}
Expand Down Expand Up @@ -133,8 +132,7 @@ private void fillBackground(GC gc, int x, int y, int width, int height) {

@Override
public Color getDefaultBackgroundColor() {
RGB backgroundRGB = fStyleMap.getBackgroundRGB(null);
return new Color(backgroundRGB);
return fStyleMap.getBackgroundColor(null);
}

private void drawCursor(ITextCanvasModel model, GC gc, int row, int x, int y, int colFirst) {
Expand Down Expand Up @@ -196,10 +194,10 @@ private void drawUnderline(GC gc, int colStart, int colEnd) {
}

private void setupGC(GC gc, TerminalStyle style) {
RGB foregrondColor = fStyleMap.getForegrondRGB(style);
gc.setForeground(new Color(gc.getDevice(), foregrondColor));
RGB backgroundColor = fStyleMap.getBackgroundRGB(style);
gc.setBackground(new Color(gc.getDevice(), backgroundColor));
Color foregroundColor = fStyleMap.getForegroundColor(style);
gc.setForeground(foregroundColor);
Color backgroundColor = fStyleMap.getBackgroundColor(style);
gc.setBackground(backgroundColor);

Font f = fStyleMap.getFont(style);
if (f != gc.getFont()) {
Expand All @@ -217,7 +215,7 @@ public void updateFont(String fontName) {
}

@Override
public void updateColors(Map<TerminalColor, RGB> map) {
public void updateColors(Map<TerminalColor, Color> map) {
fStyleMap.updateColors(map);
}

Expand Down
Loading