diff --git a/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF b/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF index 653c7b05d9d..decf753be7c 100644 --- a/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.jface.text -Bundle-Version: 3.31.0.qualifier +Bundle-Version: 3.31.100.qualifier Bundle-Vendor: %providerName Bundle-Localization: plugin Export-Package: diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningLineHeaderAnnotation.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningLineHeaderAnnotation.java index 657e34cfd7f..45f814abf5c 100644 --- a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningLineHeaderAnnotation.java +++ b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/codemining/CodeMiningLineHeaderAnnotation.java @@ -133,20 +133,28 @@ static int getMultilineHeight(GC gc, List minings, StyledText style } private static int calculateLineHeight(List minings, GC gc, StyledText styledText, boolean ignoreFirstLine, int sumLineHeight, int miningIndex, String[] splitted) { + int styledTextLineHeight= 0; + if (styledText != null) { + styledTextLineHeight= styledText.getLineHeight(); + } for (int j= 0; j < splitted.length; j++) { String line= splitted[j]; if (j == 0 && ignoreFirstLine) { continue; } - if (j == splitted.length - 1 && miningIndex + 1 < minings.size()) { // last line, take first line from next mining - String nextLabel= minings.get(miningIndex + 1).getLabel(); - if (nextLabel != null) { - String firstFromNext= nextLabel.split("\\r?\\n|\\r")[0]; //$NON-NLS-1$ - line+= firstFromNext; + if (styledText != null) { + sumLineHeight+= styledTextLineHeight + styledText.getLineSpacing(); + } else { + if (j == splitted.length - 1 && miningIndex + 1 < minings.size()) { // last line, take first line from next mining + String nextLabel= minings.get(miningIndex + 1).getLabel(); + if (nextLabel != null) { + String firstFromNext= nextLabel.split("\\r?\\n|\\r")[0]; //$NON-NLS-1$ + line+= firstFromNext; + } } + Point ext= gc.textExtent(line); + sumLineHeight+= ext.y; } - Point ext= gc.textExtent(line); - sumLineHeight+= ext.y + (styledText != null ? styledText.getLineSpacing() : 0); } return sumLineHeight; } diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/AbstractCodeMining.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/AbstractCodeMining.java index 4894583d6a1..3547e2daa6b 100644 --- a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/AbstractCodeMining.java +++ b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/AbstractCodeMining.java @@ -145,7 +145,7 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { String title= getLabel() != null ? getLabel() : "no command"; //$NON-NLS-1$ gc.drawString(title, x, y, true); Point result= gc.stringExtent(title); - result.y+= textWidget.getLineSpacing(); + result.y= textWidget.getLineHeight() + textWidget.getLineSpacing(); return result; } diff --git a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/LineHeaderCodeMining.java b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/LineHeaderCodeMining.java index 46721f2d736..8c310ee2d2e 100644 --- a/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/LineHeaderCodeMining.java +++ b/bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/codemining/LineHeaderCodeMining.java @@ -84,6 +84,8 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { } static Point draw(String label, GC gc, StyledText textWidget, int x, int y, Callable superDrawCallable) { + int lineHeight= textWidget.getLineHeight(); + int lineSpacing= textWidget.getLineSpacing(); String title= label != null ? label : "no command"; //$NON-NLS-1$ String[] lines= title.split("\\r?\\n|\\r"); //$NON-NLS-1$ if (lines.length > 1) { @@ -92,8 +94,8 @@ static Point draw(String label, GC gc, StyledText textWidget, int x, int y, Call gc.drawString(line, x, y, true); Point ext= gc.stringExtent(line); result.x= Math.max(result.x, ext.x); - result.y+= ext.y + textWidget.getLineSpacing(); - y+= ext.y + textWidget.getLineSpacing(); + result.y+= lineHeight + lineSpacing; + y+= lineHeight + lineSpacing; } return result; } else { diff --git a/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/codemining/CodeMiningLineHeaderAnnotationTest.java b/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/codemining/CodeMiningLineHeaderAnnotationTest.java index 1fc1c03b33a..859e3294178 100644 --- a/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/codemining/CodeMiningLineHeaderAnnotationTest.java +++ b/tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/codemining/CodeMiningLineHeaderAnnotationTest.java @@ -14,6 +14,7 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import java.util.Arrays; +import java.util.List; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -22,6 +23,8 @@ import org.eclipse.swt.SWT; import org.eclipse.swt.custom.StyledText; +import org.eclipse.swt.graphics.Font; +import org.eclipse.swt.graphics.FontData; import org.eclipse.swt.graphics.GC; import org.eclipse.swt.graphics.Point; import org.eclipse.swt.layout.FillLayout; @@ -123,4 +126,95 @@ public String getLabel() { // https: //github.com/eclipse-platform/eclipse.platform.ui/issues/2786 assertNotEquals(0, cut.getHeight()); // getHeight should not return 0, otherwise editor content starts jumping around } + + /** + * Verifies that a multi-line line-header code mining contributes a height which is an exact + * multiple of the StyledText's regular line height (plus line spacing). Otherwise the line + * drawn below the code mining would no longer be vertically aligned with the regular text + * lines. This was visible on Windows with Consolas at odd point sizes (e.g. 9, 11, 13), where + * {@code gc.stringExtent(line).y} returned a different value than + * {@link StyledText#getLineHeight()}, so the lines below a multiline code mining were shifted + * by a few pixels relative to the regular grid. + */ + @Test + public void testTwoLineCodeMiningHeightMatchesTextWidgetLineHeight() throws Exception { + var doc= fViewer.getDocument(); + doc.set("line0\nline1\nline2"); + var textWidget= fViewer.getTextWidget(); + // A line spacing != 0 makes a regression in either factor immediately visible. + textWidget.setLineSpacing(3); + // Try to use Consolas at an odd size on Windows since this is the configuration in which + // the original bug was reported. On other platforms / when Consolas is not available the + // test still runs with the default font - the assertions below must hold for any font. + Font consolasFont= tryCreateConsolasFont(textWidget, 11); + try { + if (consolasFont != null) { + textWidget.setFont(consolasFont); + } + String codeMiningLabel= "code mining line1\ncode mining line2"; + var mining= new LineHeaderCodeMining(0, doc, null, null) { + @Override + public String getLabel() { + return codeMiningLabel; + } + }; + int normalLineHeight= textWidget.getLineHeight(); + int lineSpacing= textWidget.getLineSpacing(); + int numMiningLines= codeMiningLabel.split("\n").length; + int expectedMiningHeight= numMiningLines * (normalLineHeight + lineSpacing); + + // 1) The size returned by LineHeaderCodeMining.draw() must be a multiple of the + // StyledText's regular line height + line spacing - it must not depend on + // gc.stringExtent(...).y, which on Consolas/odd sizes differs from getLineHeight(). + var gc= new GC(textWidget); + try { + Point result= mining.draw(gc, textWidget, null, 0, 0); + assertEquals(expectedMiningHeight, result.y, + "two-line code mining height must be 2 * (textWidget.getLineHeight() + lineSpacing)"); + } finally { + gc.dispose(); + } + + // 2) CodeMiningLineHeaderAnnotation.getHeight(GC) drives the line vertical indent of the + // line below the mining (see InlinedAnnotationDrawingStrategy). Its result must equal + // expectedMiningHeight - meaning the first regular text line below the mining ends up + // at exactly that pixel offset above its un-indented position, on the same vertical + // grid as all other text lines. + var annotation= new CodeMiningLineHeaderAnnotation(new Position(0, 0), fViewer); + var support= new InlinedAnnotationSupport(); + support.install(fViewer, new AnnotationPainter(fViewer, null)); + var setSupport= AbstractInlinedAnnotation.class.getDeclaredMethod("setSupport", InlinedAnnotationSupport.class); + setSupport.setAccessible(true); + setSupport.invoke(annotation, support); + annotation.update(List.of(mining), null); + + GC gc2= new GC(textWidget); + try { + int multilineHeight= annotation.getHeight(gc2); + assertEquals(expectedMiningHeight, multilineHeight, + "multiline mining height must be a whole-line multiple of the StyledText line height"); + // The pixel position of the line directly below the code mining is + // (multilineHeight) above its un-indented y position, and that offset must be a + // whole number of regular text line heights so the lines below the mining stay + // aligned with the regular grid. + assertEquals(0, multilineHeight % (normalLineHeight + lineSpacing), + "line below the code mining must land on a regular text-line boundary"); + } finally { + gc2.dispose(); + } + } finally { + if (consolasFont != null) { + textWidget.setFont(null); + consolasFont.dispose(); + } + } + } + + private static Font tryCreateConsolasFont(StyledText textWidget, int height) { + FontData[] available= textWidget.getDisplay().getFontList("Consolas", true); //$NON-NLS-1$ + if (available == null || available.length == 0) { + return null; + } + return new Font(textWidget.getDisplay(), "Consolas", height, SWT.NORMAL); //$NON-NLS-1$ + } }