diff --git a/team/bundles/org.eclipse.compare/HANDOVER.md b/team/bundles/org.eclipse.compare/HANDOVER.md new file mode 100644 index 00000000000..075880ed691 --- /dev/null +++ b/team/bundles/org.eclipse.compare/HANDOVER.md @@ -0,0 +1,93 @@ +# Compare Bundle — Handover Notes + +## Dark Theme Diff Colors + +### Problem + +The diff highlight colors in the Compare editor (both 2-way compare and unified diff) have +poor contrast in Eclipse Dark Theme. The issue was reported in +[eclipse-platform#2993](https://github.com/eclipse-platform/eclipse.platform/issues/2993). + +### Color Architecture + +Both the **2-way compare** (`TextMergeViewer`) and the **unified diff code mining** +(`UnifiedDiffCodeMiningProvider`) derive their diff background colors from two base colors +registered in the JFace color registry: + +| Registry key | Default value | Configured via | +|-----------------|-------------------|-----------------------------------------| +| `ADDITION_COLOR` | `COLOR_DARK_GREEN` | Eclipse Preferences → Colors and Fonts | +| `DELETION_COLOR` | `COLOR_RED` | Eclipse Preferences → Colors and Fonts | + +These base colors are intentionally **not used directly** as background colors. Instead, +`TextMergeViewer.ColorPalette` derives three tints by interpolating the base color toward +the editor background: + +``` +normal = interpolate(base, background, scale_normal) // stroke/border +fill = interpolate(base, background, scale_fill) // line background +textFill = interpolate(base, background, scale_textFill) // inline text highlight +``` + +`UnifiedDiffCodeMiningProvider` uses the same interpolation for the code mining (overlay) +path. The annotation painter path reads final colors directly from the preference store +keys `inlineDiffColor` / `deletionInlineDiffColor` etc., which are overridden per-theme +via `resources/css/dark.css`. + +### The Dark Theme Problem + +With a light theme background (~`255,255,255`) the interpolation at scale 0.9 produces a +soft pastel tint — clearly visible. With a dark theme background (~`30,31,34`) the same +scale produces a color nearly indistinguishable from the background. For example: + +- `DELETION_COLOR` = `COLOR_RED` = `(255,0,0)`, dark bg = `(30,31,34)` +- fill at scale 0.9 → `(52,27,30)` — very dark, barely visible + +It is **not possible** to reach a well-contrasted dark theme color by only changing the +base color (`ADDITION_COLOR` / `DELETION_COLOR`), because the 0.9 scale washes out any +base color. Working backwards, even pure `(255,0,0)` only yields `(52,27,30)` as the fill. + +### Solution + +We preserve the existing user-configurable color architecture (users set `ADDITION_COLOR` +/ `DELETION_COLOR` in Preferences and the three tints are derived from those). To improve +dark theme contrast, the **scale factors are reduced when the dark theme is active**: + +| Theme | `normal` scale | `fill` scale | `detail` scale | +|-------|---------------|-------------|----------------| +| Light | 0.6 | 0.9 | 0.8 | +| Dark | 0.3 | 0.79 | 0.5 | + +The scales are centralised in three static methods `normalScale(boolean dark)`, +`fillScale(boolean dark)` and `detailScale(boolean dark)` on +`UnifiedDiffCodeMiningProvider`, which all three painting paths call. + +Dark theme detection uses `INFORMATION_BACKGROUND_COLOR` from the theme's color registry +as a proxy (sum of RGB channels < 3×128 = dark). + +Note: `ADDITION_COLOR` and `DELETION_COLOR` are **JFace color registry keys**, not +preference store keys. They cannot be overridden via the `IEclipsePreferences` CSS +mechanism in `e4-dark_preferencestyle.css` — that mechanism only affects preference store +entries. The default values `COLOR_DARK_GREEN=(0,128,0)` and `COLOR_RED=(255,0,0)` are +therefore always the base colors fed into the interpolation. + +With the reduced dark scale factors these produce: + +| Color | Target (Option C) | Produced | +|-------|-------------------|---------| +| Addition fill | `(13,59,13)` | `(23,51,26)` | +| Addition detail | `(20,82,20)` | `(15,79,17)` | +| Deletion fill | `(59,13,13)` | `(77,24,26)` | +| Deletion detail | `(82,20,20)` | `(142,15,17)` | + +The annotation painter path (`dark.css`) is updated to match the same computed values so +that both rendering paths produce consistent colors. + +### Files Changed + +| File | Change | +|------|--------| +| `compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffManager.java` | `UnifiedDiffPaintListener` uses dark-aware scale factor | +| `compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java` | `ColorPalette` uses dark-aware scale factors | +| `compare/org/eclipse/compare/unifieddiff/internal/UnifiedDiffCodeMiningProvider.java` | Code mining path uses dark-aware scale factors; `isDarkTheme()` helper added | +| `resources/css/dark.css` | Annotation painter colors updated to match computed values | diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java index 4d80b783e12..284a749d5c2 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/contentmergeviewer/TextMergeViewer.java @@ -364,9 +364,10 @@ public ColorPalette(RGB background, String registryKey, RGB defaultRGB) { } else { selected = defaultRGB; } - normal = DiffColors.interpolate(selected, background, DiffColors.BORDER_SCALE); - fill = DiffColors.interpolate(selected, background, DiffColors.FILL_SCALE); - textFill = DiffColors.interpolate(selected, background, DiffColors.TEXT_FILL_SCALE); + boolean dark = DiffColors.isDarkTheme(); + normal = DiffColors.interpolate(selected, background, DiffColors.borderScale(dark)); + fill = DiffColors.interpolate(selected, background, DiffColors.fillScale(dark)); + textFill = DiffColors.interpolate(selected, background, DiffColors.detailScale(dark)); } } diff --git a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffColors.java b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffColors.java index 564e2e95301..da430cf6aa7 100644 --- a/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffColors.java +++ b/team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/DiffColors.java @@ -14,6 +14,7 @@ package org.eclipse.compare.internal; import org.eclipse.swt.graphics.RGB; +import org.eclipse.ui.PlatformUI; /** * The shared color palette for diff rendering. Both the traditional two-way @@ -35,9 +36,33 @@ private DiffColors() { /** The hunk fill band; the lightest of the three. */ public static final double FILL_SCALE = 0.9; - /** Border color of an addition/deletion hunk; darker than the fill band. */ + private static final String INFORMATION_BACKGROUND_COLOR = "org.eclipse.ui.workbench.INFORMATION_BACKGROUND"; //$NON-NLS-1$ + + /** Returns {@code true} when the active workbench theme is dark. */ + public static boolean isDarkTheme() { + var reg = PlatformUI.getWorkbench().getThemeManager().getCurrentTheme().getColorRegistry(); + var color = reg.getRGB(INFORMATION_BACKGROUND_COLOR); + return color != null && color.red + color.green + color.blue < 3 * 128; + } + + /** Scale for the border/stroke color; adapts to dark vs. light themes. */ + public static double borderScale(boolean dark) { + return dark ? 0.3 : BORDER_SCALE; + } + + /** Scale for the line-band fill; adapts to dark vs. light themes. */ + public static double fillScale(boolean dark) { + return dark ? 0.79 : FILL_SCALE; + } + + /** Scale for the word-level detail highlight; adapts to dark vs. light themes. */ + public static double detailScale(boolean dark) { + return dark ? 0.5 : TEXT_FILL_SCALE; + } + + /** Border color of an addition/deletion hunk; adapts to dark vs. light themes. */ public static RGB borderColor(RGB diffColor, RGB background) { - return interpolate(diffColor, background, BORDER_SCALE); + return interpolate(diffColor, background, borderScale(isDarkTheme())); } /** 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 c7ff124dabf..7692ecf93f0 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 @@ -157,8 +157,9 @@ public CompletableFuture> provideCodeMinings(ITextVi this.foldButtonColor.dispose(); } // the word-level diff is tinted stronger so it stands out against the band - this.detailedDiffColor = new Color(interpolate(deletionColor, background, DiffColors.TEXT_FILL_SCALE)); - this.deletionBackgroundColor = new Color(interpolate(deletionColor, background, DiffColors.FILL_SCALE)); + boolean dark = DiffColors.isDarkTheme(); + this.detailedDiffColor = new Color(interpolate(deletionColor, background, DiffColors.detailScale(dark))); + this.deletionBackgroundColor = new Color(interpolate(deletionColor, background, DiffColors.fillScale(dark))); this.borderColor = new Color(UnifiedDiffText.borderColor(deletionColor, background)); this.foldSeparatorColor = new Color(separatorBackground(background)); this.foldButtonColor = new Color(buttonBackground(background)); 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 b6d14021740..225aa42ba16 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 @@ -31,6 +31,7 @@ import org.eclipse.compare.contentmergeviewer.ITokenComparator; import org.eclipse.compare.contentmergeviewer.TokenComparator; import org.eclipse.compare.internal.CompareMessages; +import org.eclipse.compare.internal.DiffColors; import org.eclipse.compare.internal.DocLineComparator; import org.eclipse.compare.rangedifferencer.IRangeComparator; import org.eclipse.compare.rangedifferencer.RangeDifference; @@ -1822,7 +1823,8 @@ public UnifiedDiffPaintListener(ITextViewer viewer, IAnnotationModel model, Unif color = JFaceResources.getColorRegistry().getRGB("ADDITION_COLOR"); //$NON-NLS-1$ } RGB background = UnifiedDiffCodeMiningProvider.getBackground(); - RGB interpolated = UnifiedDiffCodeMiningProvider.interpolate(color, background, 0.9); + boolean dark = DiffColors.isDarkTheme(); + RGB interpolated = DiffColors.interpolate(color, background, DiffColors.fillScale(dark)); this.additionBackgroundColor = new Color(interpolated); this.borderColor = new Color(UnifiedDiffText.borderColor(color, background)); } diff --git a/team/bundles/org.eclipse.compare/resources/css/dark.css b/team/bundles/org.eclipse.compare/resources/css/dark.css index 3fe06399e98..8349594f4d4 100644 --- a/team/bundles/org.eclipse.compare/resources/css/dark.css +++ b/team/bundles/org.eclipse.compare/resources/css/dark.css @@ -1,7 +1,7 @@ IEclipsePreferences#org-eclipse-ui-editors:org-eclipse-compare { preferences: - 'inlineDiffColor=27,40,30' - 'detailedInlineDiffColor=24,50,27' - 'deletionInlineDiffColor=52,27,30' - 'detailedDeletionInlineDiffColor=74,24,27' + 'inlineDiffColor=23,51,26' + 'detailedInlineDiffColor=15,79,17' + 'deletionInlineDiffColor=77,24,26' + 'detailedDeletionInlineDiffColor=142,15,17' }