Skip to content

Commit edbad22

Browse files
tobias-melcherBeckerWdf
authored andcommitted
Fix multiline code mining height alignment with text widget lines
The line height of multiline code minings was computed via gc.stringExtent(line).y, which on Windows with Consolas at odd point sizes (e.g. 9, 11, 13) returns a different value than StyledText.getLineHeight(). As a result, the regular text lines drawn below a multiline code mining were shifted by a few pixels and no longer aligned with the rest of the text grid. Use textWidget.getLineHeight() (plus textWidget.getLineSpacing()) in LineHeaderCodeMining.draw, AbstractCodeMining.draw and CodeMiningLineHeaderAnnotation.calculateLineHeight so each code mining line contributes exactly one StyledText line height.
1 parent 9f96bf3 commit edbad22

4 files changed

Lines changed: 114 additions & 10 deletions

File tree

bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningLineHeaderAnnotation.java

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,20 +133,28 @@ static int getMultilineHeight(GC gc, List<ICodeMining> minings, StyledText style
133133
}
134134

135135
private static int calculateLineHeight(List<ICodeMining> minings, GC gc, StyledText styledText, boolean ignoreFirstLine, int sumLineHeight, int miningIndex, String[] splitted) {
136+
int styledTextLineHeight= 0;
137+
if (styledText != null) {
138+
styledTextLineHeight= styledText.getLineHeight();
139+
}
136140
for (int j= 0; j < splitted.length; j++) {
137141
String line= splitted[j];
138142
if (j == 0 && ignoreFirstLine) {
139143
continue;
140144
}
141-
if (j == splitted.length - 1 && miningIndex + 1 < minings.size()) { // last line, take first line from next mining
142-
String nextLabel= minings.get(miningIndex + 1).getLabel();
143-
if (nextLabel != null) {
144-
String firstFromNext= nextLabel.split("\\r?\\n|\\r")[0]; //$NON-NLS-1$
145-
line+= firstFromNext;
145+
if (styledText != null) {
146+
sumLineHeight+= styledTextLineHeight + styledText.getLineSpacing();
147+
} else {
148+
if (j == splitted.length - 1 && miningIndex + 1 < minings.size()) { // last line, take first line from next mining
149+
String nextLabel= minings.get(miningIndex + 1).getLabel();
150+
if (nextLabel != null) {
151+
String firstFromNext= nextLabel.split("\\r?\\n|\\r")[0]; //$NON-NLS-1$
152+
line+= firstFromNext;
153+
}
146154
}
155+
Point ext= gc.textExtent(line);
156+
sumLineHeight+= ext.y;
147157
}
148-
Point ext= gc.textExtent(line);
149-
sumLineHeight+= ext.y + (styledText != null ? styledText.getLineSpacing() : 0);
150158
}
151159
return sumLineHeight;
152160
}

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/AbstractCodeMining.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) {
145145
String title= getLabel() != null ? getLabel() : "no command"; //$NON-NLS-1$
146146
gc.drawString(title, x, y, true);
147147
Point result= gc.stringExtent(title);
148-
result.y+= textWidget.getLineSpacing();
148+
result.y= textWidget.getLineHeight() + textWidget.getLineSpacing();
149149
return result;
150150
}
151151

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/LineHeaderCodeMining.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) {
8484
}
8585

8686
static Point draw(String label, GC gc, StyledText textWidget, int x, int y, Callable<Point> superDrawCallable) {
87+
int lineHeight= textWidget.getLineHeight();
88+
int lineSpacing= textWidget.getLineSpacing();
8789
String title= label != null ? label : "no command"; //$NON-NLS-1$
8890
String[] lines= title.split("\\r?\\n|\\r"); //$NON-NLS-1$
8991
if (lines.length > 1) {
@@ -92,8 +94,8 @@ static Point draw(String label, GC gc, StyledText textWidget, int x, int y, Call
9294
gc.drawString(line, x, y, true);
9395
Point ext= gc.stringExtent(line);
9496
result.x= Math.max(result.x, ext.x);
95-
result.y+= ext.y + textWidget.getLineSpacing();
96-
y+= ext.y + textWidget.getLineSpacing();
97+
result.y+= lineHeight + lineSpacing;
98+
y+= lineHeight + lineSpacing;
9799
}
98100
return result;
99101
} else {

tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/codemining/CodeMiningLineHeaderAnnotationTest.java

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import static org.junit.jupiter.api.Assertions.assertNotEquals;
1515

1616
import java.util.Arrays;
17+
import java.util.List;
1718

1819
import org.junit.jupiter.api.AfterEach;
1920
import org.junit.jupiter.api.Assertions;
@@ -22,6 +23,8 @@
2223

2324
import org.eclipse.swt.SWT;
2425
import org.eclipse.swt.custom.StyledText;
26+
import org.eclipse.swt.graphics.Font;
27+
import org.eclipse.swt.graphics.FontData;
2528
import org.eclipse.swt.graphics.GC;
2629
import org.eclipse.swt.graphics.Point;
2730
import org.eclipse.swt.layout.FillLayout;
@@ -123,4 +126,95 @@ public String getLabel() {
123126
// https: //github.com/eclipse-platform/eclipse.platform.ui/issues/2786
124127
assertNotEquals(0, cut.getHeight()); // getHeight should not return 0, otherwise editor content starts jumping around
125128
}
129+
130+
/**
131+
* Verifies that a multi-line line-header code mining contributes a height which is an exact
132+
* multiple of the StyledText's regular line height (plus line spacing). Otherwise the line
133+
* drawn below the code mining would no longer be vertically aligned with the regular text
134+
* lines. This was visible on Windows with Consolas at odd point sizes (e.g. 9, 11, 13), where
135+
* {@code gc.stringExtent(line).y} returned a different value than
136+
* {@link StyledText#getLineHeight()}, so the lines below a multiline code mining were shifted
137+
* by a few pixels relative to the regular grid.
138+
*/
139+
@Test
140+
public void testTwoLineCodeMiningHeightMatchesTextWidgetLineHeight() throws Exception {
141+
var doc= fViewer.getDocument();
142+
doc.set("line0\nline1\nline2");
143+
var textWidget= fViewer.getTextWidget();
144+
// A line spacing != 0 makes a regression in either factor immediately visible.
145+
textWidget.setLineSpacing(3);
146+
// Try to use Consolas at an odd size on Windows since this is the configuration in which
147+
// the original bug was reported. On other platforms / when Consolas is not available the
148+
// test still runs with the default font - the assertions below must hold for any font.
149+
Font consolasFont= tryCreateConsolasFont(textWidget, 11);
150+
try {
151+
if (consolasFont != null) {
152+
textWidget.setFont(consolasFont);
153+
}
154+
String codeMiningLabel= "code mining line1\ncode mining line2";
155+
var mining= new LineHeaderCodeMining(0, doc, null, null) {
156+
@Override
157+
public String getLabel() {
158+
return codeMiningLabel;
159+
}
160+
};
161+
int normalLineHeight= textWidget.getLineHeight();
162+
int lineSpacing= textWidget.getLineSpacing();
163+
int numMiningLines= codeMiningLabel.split("\n").length;
164+
int expectedMiningHeight= numMiningLines * (normalLineHeight + lineSpacing);
165+
166+
// 1) The size returned by LineHeaderCodeMining.draw() must be a multiple of the
167+
// StyledText's regular line height + line spacing - it must not depend on
168+
// gc.stringExtent(...).y, which on Consolas/odd sizes differs from getLineHeight().
169+
var gc= new GC(textWidget);
170+
try {
171+
Point result= mining.draw(gc, textWidget, null, 0, 0);
172+
assertEquals(expectedMiningHeight, result.y,
173+
"two-line code mining height must be 2 * (textWidget.getLineHeight() + lineSpacing)");
174+
} finally {
175+
gc.dispose();
176+
}
177+
178+
// 2) CodeMiningLineHeaderAnnotation.getHeight(GC) drives the line vertical indent of the
179+
// line below the mining (see InlinedAnnotationDrawingStrategy). Its result must equal
180+
// expectedMiningHeight - meaning the first regular text line below the mining ends up
181+
// at exactly that pixel offset above its un-indented position, on the same vertical
182+
// grid as all other text lines.
183+
var annotation= new CodeMiningLineHeaderAnnotation(new Position(0, 0), fViewer);
184+
var support= new InlinedAnnotationSupport();
185+
support.install(fViewer, new AnnotationPainter(fViewer, null));
186+
var setSupport= AbstractInlinedAnnotation.class.getDeclaredMethod("setSupport", InlinedAnnotationSupport.class);
187+
setSupport.setAccessible(true);
188+
setSupport.invoke(annotation, support);
189+
annotation.update(List.of(mining), null);
190+
191+
GC gc2= new GC(textWidget);
192+
try {
193+
int multilineHeight= annotation.getHeight(gc2);
194+
assertEquals(expectedMiningHeight, multilineHeight,
195+
"multiline mining height must be a whole-line multiple of the StyledText line height");
196+
// The pixel position of the line directly below the code mining is
197+
// (multilineHeight) above its un-indented y position, and that offset must be a
198+
// whole number of regular text line heights so the lines below the mining stay
199+
// aligned with the regular grid.
200+
assertEquals(0, multilineHeight % (normalLineHeight + lineSpacing),
201+
"line below the code mining must land on a regular text-line boundary");
202+
} finally {
203+
gc2.dispose();
204+
}
205+
} finally {
206+
if (consolasFont != null) {
207+
textWidget.setFont(null);
208+
consolasFont.dispose();
209+
}
210+
}
211+
}
212+
213+
private static Font tryCreateConsolasFont(StyledText textWidget, int height) {
214+
FontData[] available= textWidget.getDisplay().getFontList("Consolas", true); //$NON-NLS-1$
215+
if (available == null || available.length == 0) {
216+
return null;
217+
}
218+
return new Font(textWidget.getDisplay(), "Consolas", height, SWT.NORMAL); //$NON-NLS-1$
219+
}
126220
}

0 commit comments

Comments
 (0)