diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java index 7bcf51683f5..1064e73e9b2 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java @@ -749,7 +749,7 @@ public String getRawLabel() { } @Override - public void dispose() { + public synchronized void dispose() { styleRanges = null; lastRectangle = null; cachedFont = null; @@ -770,7 +770,7 @@ private List styleRanges(String label) { } @Override - public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { + public synchronized Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { gc.setBackground(this.deletionBackgroundColor); Color c = textWidget.getForeground(); gc.setForeground(c); @@ -797,7 +797,7 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { if (ranges.isEmpty()) { // no syntax coloring available; fall back to plain rendering result = super.draw(gc, textWidget, color, x, y); - drawBandBorder(gc, this.borderColor, y, textWidget.getBounds().width, lastRectangle.height, + drawBandBorder(gc, this.borderColor, y, textWidget.getBounds().width, result.y, this.bandBorder); return result; } @@ -809,7 +809,9 @@ public Point draw(GC gc, StyledText textWidget, Color color, int x, int y) { gc.setFont(font); drawStyleRanges(gc, textWidget, ranges, label, rawLabel, styledFonts, x, y, null, whitespace, trailingCarriageReturn, trailingLineFeed); - drawBandBorder(gc, this.borderColor, y, textWidget.getBounds().width, lastRectangle.height, + // Syntax-coloring callbacks can dispose the mining and clear lastRectangle. + // Use the size measured by this draw call rather than the invalidated cache. + drawBandBorder(gc, this.borderColor, y, textWidget.getBounds().width, result.y, this.bandBorder); return result; } diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java index 9777291f2bf..7bd961dcaf8 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java @@ -1672,8 +1672,9 @@ private static void applyDiffRightStr(ITextViewer tv, IAnnotationModel model, Un positions.add(pos); replaceStrings.add(diff.rightStr); } - model.removeAnnotation(lanno); } + // The painter must see deleted headers when it clears their reserved height. + removeAnnotations(model, annos); if (tv instanceof ISourceViewerExtension5 ext) { ext.updateCodeMinings(); } diff --git a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffCodeMiningProviderTest.java b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffCodeMiningProviderTest.java index 2ef2306dbb0..465998c7e07 100644 --- a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffCodeMiningProviderTest.java +++ b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffCodeMiningProviderTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -49,6 +50,7 @@ import org.eclipse.jface.text.BadLocationException; import org.eclipse.jface.text.Document; import org.eclipse.jface.text.IDocument; +import org.eclipse.jface.text.IRegion; import org.eclipse.jface.text.ITextViewer; import org.eclipse.jface.text.Position; import org.eclipse.jface.text.TextAttribute; @@ -74,6 +76,7 @@ import org.eclipse.jface.text.source.projection.ProjectionAnnotationModel; import org.eclipse.jface.text.source.projection.ProjectionViewer; import org.eclipse.swt.SWT; +import org.eclipse.swt.custom.StyleRange; import org.eclipse.swt.custom.StyledText; import org.eclipse.swt.graphics.Color; import org.eclipse.swt.graphics.Font; @@ -633,6 +636,55 @@ private void draw(UnifiedDiffLineHeaderCodeMining mining, List scannedDo } } + @Test + public void testFooterMiningDisposalDuringPlainDrawingDoesNotLoseHeight() throws Exception { + assertFooterDisposalDuringDrawing(false); + } + + @Test + public void testFooterMiningDisposalDuringStyledDrawingDoesNotLoseHeight() throws Exception { + assertFooterDisposalDuringDrawing(true); + } + + private void assertFooterDisposalDuringDrawing(boolean styled) throws Exception { + switchToDocument(new Document("line 0\nx")); + assertTrue(open("line 0\nchanged\n", MODE).isOK()); + UnifiedDiffFooterCodeMining footer = footerMining(); + UnifiedDiffFooterCodeMining[] painting = new UnifiedDiffFooterCodeMining[1]; + ProjectionViewer paintingViewer = new ProjectionViewer(shell, null, null, false, SWT.V_SCROLL) { + @Override + public List computeStyleRanges(IDocument syntaxDocument, IRegion region) { + // Invalidate the cache after draw measured its size, before it draws the border. + painting[0].dispose(); + return styled ? new ArrayList<>(List.of(new StyleRange(region.getOffset(), region.getLength(), + display.getSystemColor(SWT.COLOR_DARK_BLUE), null))) : new ArrayList<>(); + } + }; + paintingViewer.setDocument(document); + paintingViewer.getTextWidget().setSize(400, 100); + Color deletionColor = new Color(display, 11, 22, 33); + Color detailedDiffColor = new Color(display, 44, 55, 66); + Color borderColor = new Color(display, 77, 88, 99); + painting[0] = new UnifiedDiffFooterCodeMining(document, provider, footer.getUnifiedDiff(), 4, + deletionColor, detailedDiffColor, borderColor, paintingViewer); + Image image = new Image(display, 400, 100); + GC gc = new GC(image); + try { + Point size = painting[0].draw(gc, paintingViewer.getTextWidget(), null, 0, 0); + assertTrue(size.y > 0, "drawing must retain its measured height after cache invalidation"); + assertNull(painting[0].getLastRectangle(), "disposal must have cleared the cached rectangle"); + assertTrue(firstColumnWith(image, borderColor) >= 0, "drawing must still paint the band border"); + } finally { + gc.dispose(); + image.dispose(); + painting[0].dispose(); + paintingViewer.getTextWidget().dispose(); + deletionColor.dispose(); + detailedDiffColor.dispose(); + borderColor.dispose(); + } + } + /** * A word-level footer highlight following bold text must start after the bold * text, not where the base font would put it. Inspects the pixels diff --git a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffManagerTest.java b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffManagerTest.java index bef57eb6b6e..603d6a49c67 100644 --- a/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffManagerTest.java +++ b/team/tests/org.eclipse.team.tests.core/src/org/eclipse/team/tests/ui/UnifiedDiffManagerTest.java @@ -23,6 +23,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -1127,6 +1128,14 @@ public void testRevertingEarlierDiffRefreshesRemainingCodeMiningPosition() throw List diffs = UnifiedDiffManager.get(viewer()); assertEquals(2, diffs.size(), "the input must produce two revert diffs"); + StyledText tw = viewer().getTextWidget(); + tw.setSize(800, 600); + assertNotNull(waitForLineHeaderAnnotation()); + UnifiedDiffManager.UnifiedDiff laterDiff = diffs.get(1); + lineHeaderMiningFor(laterDiff); + firePaint(tw); + int firstLine = tw.getLineAtOffset(diffs.get(0).leftStart); + assertTrue(tw.getLineVerticalIndent(firstLine) > 0, "the first mining must reserve height before reverting"); viewer().setSelectedRange(diffs.get(0).leftStart, 0); executeEditorCommand("org.eclipse.compare.acceptCurrentDiff"); //$NON-NLS-1$ @@ -1134,9 +1143,16 @@ public void testRevertingEarlierDiffRefreshesRemainingCodeMiningPosition() throw diffs = UnifiedDiffManager.get(viewer()); assertEquals(1, diffs.size(), "reverting the first diff must leave the later diff active"); + assertSame(laterDiff, diffs.get(0), "the surviving band must belong to the later diff"); + assertEquals(0, tw.getLineVerticalIndent(firstLine), "removing the first header must clear its reserved height"); UnifiedDiffLineHeaderCodeMining mining = lineHeaderMiningFor(diffs.get(0)); assertEquals(afterRevert.indexOf("new later replacement"), mining.getPosition().offset, "the remaining code mining must be positioned after the reverted text"); + assertTrue(mining.getLabel().contains("old later"), "the surviving mining must retain the later deletion text"); + firePaint(tw); + assertNotNull(mining.getLastRectangle(), "the surviving mining must be painted"); + assertEquals(tw.getLocationAtOffset(tw.getOffsetAtLine(tw.getLineAtOffset(mining.getPosition().offset))).x, + mining.getLastRectangle().x, "the surviving mining must paint at the line origin"); } @Test