Respond to AI reviewer comments

Author: johnml1135

Src/Common/Controls/DetailControls/DataTree.cs

@@ -4536,9 +4536,39 @@ private bool EquivalentKeys(object[] newKey, object[] oldKey, bool fCheckInts)
 			return true;
 		}
 
-		private const int WM_MOUSEWHEEL = 0x020A;
 		private static readonly WheelRedirector s_wheelRedirector = new WheelRedirector();
 
+		internal static int GetWheelScrollPixels(DataTree dataTree, int delta)
+		{
+			if (delta == 0)
+				return 0;
+
+			int scrollLines = SystemInformation.MouseWheelScrollLines;
+			if (scrollLines == 0)
+				return 0;
+
+			if (scrollLines == int.MaxValue)
+				return Math.Sign(delta) * dataTree.ClientRectangle.Height;
+
+			double linesToScroll = (double)delta / SystemInformation.MouseWheelScrollDelta * scrollLines;
+			return (int)Math.Round(linesToScroll * dataTree.Font.Height, MidpointRounding.AwayFromZero);
+		}
+
+		internal static bool TryGetWheelScrollPosition(DataTree dataTree, int delta, out int newY)
+		{
+			int currentY = -dataTree.AutoScrollPosition.Y;
+			int maxScroll = Math.Max(0,
+				dataTree.AutoScrollMinSize.Height - dataTree.ClientRectangle.Height);
+			int pixelDelta = GetWheelScrollPixels(dataTree, delta);
+			newY = Math.Max(0, Math.Min(currentY - pixelDelta, maxScroll));
+			return newY != currentY;
+		}
+
+		internal static bool CanRedirectWheelMessage(DataTree dataTree)
+		{
+			return dataTree.IsHandleCreated && !dataTree.IsDisposed && dataTree.Visible;
+		}
+
 		/// <summary>
 		/// Application-level message filter that intercepts WM_MOUSEWHEEL messages
 		/// and scrolls the DataTree when the cursor is over its client area.
@@ -4549,12 +4579,13 @@ private bool EquivalentKeys(object[] newKey, object[] oldKey, bool fCheckInts)
 		/// </summary>
 		private sealed class WheelRedirector : IMessageFilter
 		{
-			private readonly List<DataTree> m_dataTrees = new List<DataTree>();
+			private readonly HashSet<DataTree> m_dataTrees = new HashSet<DataTree>();
 			private bool m_installed;
 
 			public void Register(DataTree dataTree)
 			{
-				m_dataTrees.Add(dataTree);
+				if (!m_dataTrees.Add(dataTree))
+					return;
 				if (!m_installed)
 				{
 					Application.AddMessageFilter(this);
@@ -4564,7 +4595,9 @@ public void Register(DataTree dataTree)
 
 			public void Unregister(DataTree dataTree)
 			{
-				m_dataTrees.Remove(dataTree);
+				if (!m_dataTrees.Remove(dataTree))
+					return;
+
 				if (m_dataTrees.Count == 0 && m_installed)
 				{
 					Application.RemoveMessageFilter(this);
@@ -4574,28 +4607,28 @@ public void Unregister(DataTree dataTree)
 
 			public bool PreFilterMessage(ref Message m)
 			{
-				if (m.Msg != WM_MOUSEWHEEL)
+				if (m.Msg != (int)Win32.WinMsgs.WM_MOUSEWHEEL)
 					return false;
 
 				Point cursor = Cursor.Position;
-				for (int i = 0; i < m_dataTrees.Count; i++)
+				foreach (var dataTree in m_dataTrees)
 				{
-					var dataTree = m_dataTrees[i];
-					if (!dataTree.IsHandleCreated || dataTree.IsDisposed)
+					if (!CanRedirectWheelMessage(dataTree))
 						continue;
 
 					Rectangle bounds = dataTree.RectangleToScreen(dataTree.ClientRectangle);
 					if (!bounds.Contains(cursor))
 						continue;
 
 					int delta = (short)((long)m.WParam >> 16);
-					int currentY = -dataTree.AutoScrollPosition.Y;
-					int maxScroll = Math.Max(0,
-						dataTree.AutoScrollMinSize.Height - dataTree.ClientRectangle.Height);
-					int newY = Math.Max(0, Math.Min(currentY - delta, maxScroll));
-					if (newY != currentY)
+					int newY;
+					if (TryGetWheelScrollPosition(dataTree, delta, out newY))
+					{
 						dataTree.AutoScrollPosition = new Point(0, newY);
-					return true;
+						return true;
+					}
+
+					return false;
 				}
 
 				return false;

Src/Common/Controls/DetailControls/DetailControlsTests/DataTreeTests.cs

@@ -4,7 +4,9 @@
 
 using System;
 using System.Collections.Generic;
+using System.Drawing;
 using System.IO;
+using System.Reflection;
 using System.Windows.Forms;
 using System.Xml;
 using NUnit.Framework;
@@ -275,6 +277,85 @@ public void GetGuidForJumpToTool_UsesRootObject_WhenNoCurrentSlice()
 			}
 		}
 
+		[Test]
+		public void GetWheelScrollPixels_UsesSystemWheelSettings()
+		{
+			m_dtree.Bounds = new Rectangle(0, 0, 200, 100);
+
+			int delta = SystemInformation.MouseWheelScrollDelta;
+			int scrollLines = SystemInformation.MouseWheelScrollLines;
+			int expectedPixels;
+			if (scrollLines == 0)
+			{
+				expectedPixels = 0;
+			}
+			else if (scrollLines == int.MaxValue)
+			{
+				expectedPixels = m_dtree.ClientRectangle.Height;
+			}
+			else
+			{
+				expectedPixels = (int)Math.Round((double)scrollLines * m_dtree.Font.Height,
+					MidpointRounding.AwayFromZero);
+			}
+
+			Assert.That(DataTree.GetWheelScrollPixels(m_dtree, delta), Is.EqualTo(expectedPixels));
+			Assert.That(DataTree.GetWheelScrollPixels(m_dtree, -delta), Is.EqualTo(-expectedPixels));
+		}
+
+		[Test]
+		public void TryGetWheelScrollPosition_ReturnsFalse_WhenAlreadyAtTop()
+		{
+			m_dtree.Bounds = new Rectangle(0, 0, 200, 100);
+			m_dtree.AutoScrollMinSize = new Size(200, 1000);
+			m_dtree.AutoScrollPosition = new Point(0, 0);
+
+			int newY;
+			bool handled = DataTree.TryGetWheelScrollPosition(m_dtree,
+				SystemInformation.MouseWheelScrollDelta, out newY);
+
+			Assert.That(handled, Is.False);
+			Assert.That(newY, Is.EqualTo(0));
+		}
+
+		[Test]
+		public void CanRedirectWheelMessage_ReturnsFalse_WhenDataTreeHidden()
+		{
+			m_parent.Show();
+			m_dtree.Show();
+			Assert.That(m_dtree.IsHandleCreated, Is.True);
+
+			m_dtree.Hide();
+
+			Assert.That(DataTree.CanRedirectWheelMessage(m_dtree), Is.False);
+		}
+
+		[Test]
+		public void Register_RegisteredTwice_AddsOneEntryAndSingleUnregisterRemovesIt()
+		{
+			Type wheelRedirectorType = typeof(DataTree).GetNestedType("WheelRedirector", BindingFlags.NonPublic);
+			Assert.That(wheelRedirectorType, Is.Not.Null);
+
+			object redirector = Activator.CreateInstance(wheelRedirectorType, true);
+			MethodInfo register = wheelRedirectorType.GetMethod("Register", BindingFlags.Public | BindingFlags.Instance);
+			MethodInfo unregister = wheelRedirectorType.GetMethod("Unregister", BindingFlags.Public | BindingFlags.Instance);
+			FieldInfo dataTreesField = wheelRedirectorType.GetField("m_dataTrees", BindingFlags.NonPublic | BindingFlags.Instance);
+			Assert.That(register, Is.Not.Null);
+			Assert.That(unregister, Is.Not.Null);
+			Assert.That(dataTreesField, Is.Not.Null);
+
+			register.Invoke(redirector, new object[] { m_dtree });
+			register.Invoke(redirector, new object[] { m_dtree });
+
+			object registrations = dataTreesField.GetValue(redirector);
+			PropertyInfo countProperty = registrations.GetType().GetProperty("Count", BindingFlags.Public | BindingFlags.Instance);
+			Assert.That(countProperty, Is.Not.Null);
+			Assert.That((int)countProperty.GetValue(registrations, null), Is.EqualTo(1));
+
+			unregister.Invoke(redirector, new object[] { m_dtree });
+			Assert.That((int)countProperty.GetValue(registrations, null), Is.EqualTo(0));
+		}
+
 		/// <summary></summary>
 		[Test]
 		public void OwnedObjects()