Simplify inline hint tagging and fix duplicated tags. (#76525)

Author: CyrusNajmabadi

src/EditorFeatures/Core.Wpf/InlineHints/CachedAdornmentTagSpan.cs

@@ -0,0 +1,32 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Microsoft.VisualStudio.Text.Editor;
+using Microsoft.VisualStudio.Text.Formatting;
+using Microsoft.VisualStudio.Text.Tagging;
+
+namespace Microsoft.CodeAnalysis.Editor.InlineHints;
+
+internal partial class InlineHintsTaggerProvider
+{
+    /// <summary>
+    /// The computed adornment tag for an inline hint, along with information needed to determine if it can be reused.
+    /// This is created and cached on <see cref="InlineHintDataTag{TAdditionalInformation}.AdditionalData"/> on demand
+    /// so that we only create adornment tags once and reuse as long as possible.
+    /// </summary>
+    /// <param name="classified">Whether or not the adornment tag was classified.  If the option for this changes, this
+    /// cached tag should not be reused.</param>
+    /// <param name="format">The text formatting used to create the hint.  If this format no longer matches the current
+    /// formatting, this should not be reused.</param>
+    /// <param name="adornmentTagSpan">The actual adornment tag to render.</param>
+    private sealed class CachedAdornmentTagSpan(
+        bool classified,
+        TextFormattingRunProperties format,
+        TagSpan<IntraTextAdornmentTag> adornmentTagSpan)
+    {
+        public bool Classified { get; } = classified;
+        public TextFormattingRunProperties Format { get; } = format;
+        public TagSpan<IntraTextAdornmentTag> AdornmentTagSpan { get; } = adornmentTagSpan;
+    }
+}

src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsTagger.cs

@@ -3,37 +3,34 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Collections.Generic;
+using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
+using Microsoft.CodeAnalysis.Editor.Tagging;
 using Microsoft.CodeAnalysis.ErrorReporting;
+using Microsoft.CodeAnalysis.Options;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Text;
+using Microsoft.CodeAnalysis.Text.Shared.Extensions;
+using Microsoft.CodeAnalysis.Utilities;
 using Microsoft.VisualStudio.Text;
 using Microsoft.VisualStudio.Text.Classification;
 using Microsoft.VisualStudio.Text.Editor;
 using Microsoft.VisualStudio.Text.Formatting;
 using Microsoft.VisualStudio.Text.Tagging;
 using Roslyn.Utilities;
 
-namespace Microsoft.CodeAnalysis.Editor.InlineHints
+namespace Microsoft.CodeAnalysis.Editor.InlineHints;
+
+internal partial class InlineHintsTaggerProvider
 {
     /// <summary>
-    /// The purpose of this tagger is to convert the <see cref="InlineHintDataTag"/> to the <see
+    /// The purpose of this tagger is to convert the <see cref="InlineHintDataTag{TAdditionalInformation}"/> to the <see
     /// cref="InlineHintsTag"/>, which actually creates the UIElement. It reacts to tags changing and updates the
     /// adornments accordingly.
     /// </summary>
-    internal sealed class InlineHintsTagger : ITagger<IntraTextAdornmentTag>, IDisposable
+    private sealed class InlineHintsTagger : EfficientTagger<IntraTextAdornmentTag>
     {
-        private readonly ITagAggregator<InlineHintDataTag> _tagAggregator;
-
-        /// <summary>
-        /// stores the parameter hint tags in a global location
-        /// </summary>
-        private readonly List<(IMappingTagSpan<InlineHintDataTag> mappingTagSpan, TagSpan<IntraTextAdornmentTag>? tagSpan)> _cache = [];
-
-        /// <summary>
-        /// Stores the snapshot associated with the cached tags in <see cref="_cache" />
-        /// </summary>
-        private ITextSnapshot? _cacheSnapshot;
+        private readonly EfficientTagger<InlineHintDataTag<CachedAdornmentTagSpan>> _underlyingTagger;
 
         private readonly IClassificationFormatMap _formatMap;
 
@@ -45,77 +42,59 @@ internal sealed class InlineHintsTagger : ITagger<IntraTextAdornmentTag>, IDispo
 
         private readonly InlineHintsTaggerProvider _taggerProvider;
 
-        private readonly ITextBuffer _buffer;
         private readonly IWpfTextView _textView;
-
-        public event EventHandler<SnapshotSpanEventArgs>? TagsChanged;
+        private readonly ITextBuffer _subjectBuffer;
 
         public InlineHintsTagger(
             InlineHintsTaggerProvider taggerProvider,
             IWpfTextView textView,
-            ITextBuffer buffer,
-            ITagAggregator<InlineHintDataTag> tagAggregator)
+            ITextBuffer subjectBuffer,
+            EfficientTagger<InlineHintDataTag<CachedAdornmentTagSpan>> tagger)
         {
             _taggerProvider = taggerProvider;
 
             _textView = textView;
-            _buffer = buffer;
+            _subjectBuffer = subjectBuffer;
+
+            // When the underlying tagger produced new data tags, inform any clients of us that we have new adornment tags.
+            _underlyingTagger = tagger;
+            _underlyingTagger.TagsChanged += OnTagsChanged;
 
-            _tagAggregator = tagAggregator;
             _formatMap = taggerProvider.ClassificationFormatMapService.GetClassificationFormatMap(textView);
             _hintClassification = taggerProvider.ClassificationTypeRegistryService.GetClassificationType(InlineHintsTag.TagId);
+
             _formatMap.ClassificationFormatMappingChanged += this.OnClassificationFormatMappingChanged;
-            _tagAggregator.BatchedTagsChanged += TagAggregator_BatchedTagsChanged;
+            _taggerProvider.GlobalOptionService.AddOptionChangedHandler(this, OnGlobalOptionChanged);
         }
 
-        /// <summary>
-        /// Goes through all the spans in which tags have changed and
-        /// invokes a TagsChanged event. Using the BatchedTagsChangedEvent since it is raised
-        /// on the same thread that created the tag aggregator, unlike TagsChanged.
-        /// </summary>
-        private void TagAggregator_BatchedTagsChanged(object sender, BatchedTagsChangedEventArgs e)
+        public override void Dispose()
         {
-            _taggerProvider.ThreadingContext.ThrowIfNotOnUIThread();
-            InvalidateCache();
-
-            var tagsChanged = TagsChanged;
-            if (tagsChanged is null)
-            {
-                return;
-            }
-
-            var mappingSpans = e.Spans;
-            foreach (var item in mappingSpans)
-            {
-                var spans = item.GetSpans(_buffer);
-                foreach (var span in spans)
-                {
-                    if (tagsChanged != null)
-                    {
-                        tagsChanged.Invoke(this, new SnapshotSpanEventArgs(span));
-                    }
-                }
-            }
+            _formatMap.ClassificationFormatMappingChanged -= OnClassificationFormatMappingChanged;
+            _taggerProvider.GlobalOptionService.RemoveOptionChangedHandler(this, OnGlobalOptionChanged);
+            _underlyingTagger.TagsChanged -= OnTagsChanged;
+            _underlyingTagger.Dispose();
         }
 
         private void OnClassificationFormatMappingChanged(object sender, EventArgs e)
         {
             _taggerProvider.ThreadingContext.ThrowIfNotOnUIThread();
+
+            // When classifications change we need to rebuild the inline tags with updated Font and Color information.
+
             if (_format != null)
             {
                 _format = null;
-                InvalidateCache();
-
-                // When classifications change we need to rebuild the inline tags with updated Font and Color information.
-                var tags = GetTags(new NormalizedSnapshotSpanCollection(_textView.TextViewLines.FormattedSpan));
-
-                foreach (var tag in tags)
-                {
-                    TagsChanged?.Invoke(this, new SnapshotSpanEventArgs(tag.Span));
-                }
+                OnTagsChanged(this, new SnapshotSpanEventArgs(_subjectBuffer.CurrentSnapshot.GetFullSpan()));
             }
         }
 
+        private void OnGlobalOptionChanged(object sender, object target, OptionChangedEventArgs e)
+        {
+            // Reclassify everything.
+            if (e.HasOption(option => option.Equals(InlineHintsViewOptionsStorage.ColorHints)))
+                OnTagsChanged(this, new SnapshotSpanEventArgs(_subjectBuffer.CurrentSnapshot.GetFullSpan()));
+        }
+
         private TextFormattingRunProperties Format
         {
             get
@@ -126,86 +105,60 @@ private TextFormattingRunProperties Format
             }
         }
 
-        private void InvalidateCache()
-        {
-            _taggerProvider.ThreadingContext.ThrowIfNotOnUIThread();
-            _cacheSnapshot = null;
-            _cache.Clear();
-        }
-
-        IEnumerable<ITagSpan<IntraTextAdornmentTag>> ITagger<IntraTextAdornmentTag>.GetTags(NormalizedSnapshotSpanCollection spans)
-            => GetTags(spans);
-
-        public IReadOnlyList<TagSpan<IntraTextAdornmentTag>> GetTags(NormalizedSnapshotSpanCollection spans)
+        public override void AddTags(
+            NormalizedSnapshotSpanCollection spans,
+            SegmentedList<TagSpan<IntraTextAdornmentTag>> adornmentTagSpans)
         {
             try
             {
                 if (spans.Count == 0)
-                    return [];
+                    return;
 
+                // If the snapshot has changed, we can't use any of the cached data, as it is associated with the
+                // original snapshot they were created against.
                 var snapshot = spans[0].Snapshot;
-                if (snapshot != _cacheSnapshot)
-                {
-                    // Calculate UI elements
-                    _cache.Clear();
-                    _cacheSnapshot = snapshot;
-
-                    // Calling into the InlineParameterNameHintsDataTaggerProvider which only responds with the current
-                    // active view and disregards and requests for tags not in that view
-                    var fullSpan = new SnapshotSpan(snapshot, 0, snapshot.Length);
-                    var tags = _tagAggregator.GetTags(new NormalizedSnapshotSpanCollection(fullSpan));
-                    foreach (var tag in tags)
-                    {
-                        // Gets the associated span from the snapshot span and creates the IntraTextAdornmentTag from the data
-                        // tags. Only dealing with the dataTagSpans if the count is 1 because we do not see a multi-buffer case
-                        // occurring
-                        var dataTagSpans = tag.Span.GetSpans(snapshot);
-                        if (dataTagSpans.Count == 1)
-                        {
-                            _cache.Add((tag, tagSpan: null));
-                        }
-                    }
-                }
 
                 var document = snapshot.GetOpenDocumentInCurrentContextWithChanges();
-                var classify = document != null && _taggerProvider.EditorOptionsService.GlobalOptions.GetOption(InlineHintsViewOptionsStorage.ColorHints, document.Project.Language);
+                if (document is null)
+                    return;
+
+                var colorHints = _taggerProvider.EditorOptionsService.GlobalOptions.GetOption(InlineHintsViewOptionsStorage.ColorHints, document.Project.Language);
+
+                using var _1 = SegmentedListPool.GetPooledList<TagSpan<InlineHintDataTag<CachedAdornmentTagSpan>>>(out var dataTagSpans);
+                _underlyingTagger.AddTags(spans, dataTagSpans);
+
+                // Presize so we can add the elements below without continually resizing.
+                adornmentTagSpans.Capacity += dataTagSpans.Count;
+
+                using var _2 = PooledHashSet<int>.GetInstance(out var seenPositions);
 
-                var selectedSpans = new List<TagSpan<IntraTextAdornmentTag>>();
-                for (var i = 0; i < _cache.Count; i++)
+                var format = this.Format;
+                foreach (var dataTagSpan in dataTagSpans)
                 {
-                    var tagSpans = _cache[i].mappingTagSpan.Span.GetSpans(snapshot);
-                    if (tagSpans.Count == 1)
-                    {
-                        var tagSpan = tagSpans[0];
-                        if (spans.IntersectsWith(tagSpan))
-                        {
-                            if (_cache[i].tagSpan is not { } hintTagSpan)
-                            {
-                                var hintUITag = InlineHintsTag.Create(
-                                        _cache[i].mappingTagSpan.Tag.Hint, Format, _textView, tagSpan, _taggerProvider, _formatMap, classify);
-
-                                hintTagSpan = new TagSpan<IntraTextAdornmentTag>(tagSpan, hintUITag);
-                                _cache[i] = (_cache[i].mappingTagSpan, hintTagSpan);
-                            }
-
-                            selectedSpans.Add(hintTagSpan);
-                        }
-                    }
+                    if (seenPositions.Add(dataTagSpan.Span.Start))
+                        adornmentTagSpans.Add(GetOrCreateAdornmentTagsSpan(dataTagSpan, colorHints, format));
                 }
-
-                return selectedSpans;
             }
             catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, ErrorSeverity.General))
             {
                 throw ExceptionUtilities.Unreachable();
             }
         }
 
-        public void Dispose()
+        private TagSpan<IntraTextAdornmentTag> GetOrCreateAdornmentTagsSpan(
+            TagSpan<InlineHintDataTag<CachedAdornmentTagSpan>> dataTagSpan, bool classify, TextFormattingRunProperties format)
         {
-            _tagAggregator.BatchedTagsChanged -= TagAggregator_BatchedTagsChanged;
-            _tagAggregator.Dispose();
-            _formatMap.ClassificationFormatMappingChanged -= OnClassificationFormatMappingChanged;
+            // If we've never computed the adornment info, or options have changed, then compute and cache the new information.
+            var cachedTagInformation = dataTagSpan.Tag.AdditionalData;
+            if (cachedTagInformation is null || cachedTagInformation.Classified != classify || cachedTagInformation.Format != format)
+            {
+                var adornmentSpan = dataTagSpan.Span;
+                cachedTagInformation = new(classify, format, new TagSpan<IntraTextAdornmentTag>(adornmentSpan, InlineHintsTag.Create(
+                    dataTagSpan.Tag.Hint, format, _textView, adornmentSpan, _taggerProvider, _formatMap, classify)));
+                dataTagSpan.Tag.AdditionalData = cachedTagInformation;
+            }
+
+            return cachedTagInformation.AdornmentTagSpan;
         }
     }
 }