LT-22610: Fix randomly missing WS labels — RefreshDisplay must always reconstruct

Src/Common/SimpleRootSite/SimpleRootSite.cs

@@ -252,7 +252,6 @@ public void OnInputLanguageChanged(IKeyboardDefinition previousKeyboard, IKeyboa
 		/// <summary>True if we are waiting to do a refresh on the view (will be done when the view
 		/// becomes visible); false otherwise</summary>
 		protected bool m_fRefreshPending = false;
-		private bool m_fForceNextRefreshDisplay;
 		private RefreshPhase m_refreshPhase;
 		private bool m_fRefreshReplayRequested;
 
@@ -1054,7 +1053,6 @@ protected void NotifyDataAccessSemanticsChanged()
 			if (m_rootb?.Site == null)
 				return;
 
-			m_fForceNextRefreshDisplay = true;
 			if (!Visible || FindForm() == null)
 			{
 				m_fRefreshPending = true;
@@ -2535,17 +2533,8 @@ public virtual bool RefreshDisplay()
 				return false;
 			}
 
-			// PATH-L5: Skip the expensive selection save/restore and drawing
-			// suspension when the VwRootBox reports no pending changes.
-			// The root box's NeedsReconstruct flag is set by PropChanged,
-			// OnStylesheetChange, and other mutation paths. When false,
-			// Reconstruct() would be a no-op (PATH-R1), so we can avoid
-			// the managed overhead entirely.
-			if (!ShouldReconstructDisplay())
-			{
-				m_fRefreshPending = false;
-				return false;
-			}
+			// Always reconstruct: view constructors read state the root box can't see
+			// (e.g. WS lists), so skipping on NeedsReconstruct left stale views (LT-22610).
 
 			// Rebuild the display... the drastic way.
 			SelectionRestorer restorer = CreateSelectionRestorer();
@@ -2560,7 +2549,6 @@ public virtual bool RefreshDisplay()
 				{
 					m_rootb.Reconstruct();
 					m_fRefreshPending = false;
-					m_fForceNextRefreshDisplay = false;
 				}
 				if (reconstructStopwatch != null)
 				{
@@ -2605,11 +2593,6 @@ private void QueueRefreshDisplay()
 			});
 		}
 
-		private bool ShouldReconstructDisplay()
-		{
-			return m_fForceNextRefreshDisplay || m_rootb.NeedsReconstruct;
-		}
-
 		/// ------------------------------------------------------------------------------------
 		/// <summary>
 		/// Creates a new selection restorer.

Src/Common/SimpleRootSite/SimpleRootSiteTests/SimpleRootSiteTests_RefreshDisplayNeedsReconstruct.cs → Src/Common/SimpleRootSite/SimpleRootSiteTests/SimpleRootSiteTests_RefreshDisplayReconstruct.cs

@@ -36,7 +36,7 @@ protected override SelectionRestorer CreateSelectionRestorer()
 	}
 
 	[TestFixture]
-	public class RefreshDisplayNeedsReconstructTests
+	public class RefreshDisplayReconstructTests
 	{
 		private RefreshDisplayDummyRootSite m_site;
 		private Mock<IVwRootBox> m_rootbMock;
@@ -70,19 +70,10 @@ public void TearDown()
 		}
 
 		[Test]
-		public void RefreshDisplay_SkipsReconstruct_WhenRootBoxDoesNotNeedReconstruct()
+		public void RefreshDisplay_AlwaysReconstructs()
 		{
-			m_rootbMock.SetupGet(rb => rb.NeedsReconstruct).Returns(false);
-
-			Assert.That(m_site.RefreshDisplay(), Is.False);
-			Assert.That(m_site.RefreshPending, Is.False);
-			m_rootbMock.Verify(rb => rb.Reconstruct(), Times.Never);
-		}
-
-		[Test]
-		public void RefreshDisplay_Reconstructs_WhenRootBoxNeedsReconstruct()
-		{
-			m_rootbMock.SetupGet(rb => rb.NeedsReconstruct).Returns(true);
+			// LT-22610: view constructors read state the root box can't see, so
+			// RefreshDisplay must always reconstruct, regardless of NeedsReconstruct.
 			m_rootbMock.Setup(rb => rb.Reconstruct());
 
 			Assert.That(m_site.RefreshDisplay(), Is.False);
@@ -91,9 +82,8 @@ public void RefreshDisplay_Reconstructs_WhenRootBoxNeedsReconstruct()
 		}
 
 		[Test]
-		public void NotifyDataAccessSemanticsChanged_Reconstructs_WhenRootBoxDoesNotNeedReconstruct()
+		public void NotifyDataAccessSemanticsChanged_Reconstructs()
 		{
-			m_rootbMock.SetupGet(rb => rb.NeedsReconstruct).Returns(false);
 			m_rootbMock.Setup(rb => rb.Reconstruct());
 
 			m_site.NotifyDataAccessSemanticsChangedForTest();
@@ -105,7 +95,6 @@ public void NotifyDataAccessSemanticsChanged_Reconstructs_WhenRootBoxDoesNotNeed
 		[Test]
 		public void NotifyDataAccessSemanticsChanged_DefersUntilVisible()
 		{
-			m_rootbMock.SetupGet(rb => rb.NeedsReconstruct).Returns(false);
 			m_rootbMock.Setup(rb => rb.Reconstruct());
 			m_site.Visible = false;
 
@@ -125,7 +114,6 @@ public void SetRootBoxDataAccess_DoesNotReconstruct_WhenSwapIsCheap()
 		{
 			var replacementDataAccess = Mock.Of<ISilDataAccess>();
 			m_rootbMock.SetupSet(rb => rb.DataAccess = replacementDataAccess);
-			m_rootbMock.SetupGet(rb => rb.NeedsReconstruct).Returns(false);
 
 			m_site.SetRootBoxDataAccessForTest(replacementDataAccess);
 
@@ -139,7 +127,6 @@ public void SetRootBoxDataAccessAndRefresh_Reconstructs_WhenSwapChangesDisplaySe
 		{
 			var replacementDataAccess = Mock.Of<ISilDataAccess>();
 			m_rootbMock.SetupSet(rb => rb.DataAccess = replacementDataAccess);
-			m_rootbMock.SetupGet(rb => rb.NeedsReconstruct).Returns(false);
 			m_rootbMock.Setup(rb => rb.Reconstruct());
 
 			m_site.SetRootBoxDataAccessAndRefreshForTest(replacementDataAccess);

openspec/changes/render-speedup-benchmark/refresh-dirty-flag-audit.md

@@ -2,6 +2,14 @@
 
 Date: 2026-03-10
 
+> **Superseded 2026-07-10 (LT-22610, PR #1006):** the managed PATH-L5 gate
+> described below (`SimpleRootSite.RefreshDisplay()` skipping on
+> `NeedsReconstruct`) was removed — it missed managed-only view-constructor
+> state (e.g. writing-system display lists) invisible to the native flag,
+> causing stale views. `RefreshDisplay()` now always reconstructs. The native
+> PATH-R1 guard and dirtying paths documented here are unaffected and still
+> accurate.
+
 ## Purpose
 
 This document audits the parts of FieldWorks that can require a visual refresh after the PATH-L5 / PATH-R1 optimization work.
@@ -15,7 +23,7 @@ The recent `SetRootObjects()` fix in [Src/views/VwRootBox.cpp](../../../../Src/v
 ## Executive Summary
 
 - The native source of truth for whether a box tree needs rebuilding is `VwRootBox::m_fNeedsReconstruct` in [Src/views/VwRootBox.h](../../../../Src/views/VwRootBox.h).
-- The managed fast path is [SimpleRootSite.RefreshDisplay()](../../../../Src/Common/SimpleRootSite/SimpleRootSite.cs), which now skips managed overhead when `m_rootb.NeedsReconstruct == false`.
+- The managed fast path was [SimpleRootSite.RefreshDisplay()](../../../../Src/Common/SimpleRootSite/SimpleRootSite.cs) skipping managed overhead when `m_rootb.NeedsReconstruct == false`; this gate was removed in LT-22610 (see superseded note above) and `RefreshDisplay()` now always reconstructs.
 - The native fast path is `VwRootBox::Reconstruct()`, which now returns early when `m_fConstructed && !m_fNeedsReconstruct`.
 - The architecture is correct only if every semantic mutation that can stale the view either:
   - sets `m_fNeedsReconstruct = true` directly in native code, or
@@ -50,11 +58,11 @@ Confirmed native clearing paths:
 
 ### Managed refresh gate
 
-[SimpleRootSite.RefreshDisplay()](../../../../Src/Common/SimpleRootSite/SimpleRootSite.cs) does this:
+[SimpleRootSite.RefreshDisplay()](../../../../Src/Common/SimpleRootSite/SimpleRootSite.cs) did this (removed in LT-22610; step 3 no longer exists — see superseded note above):
 
 1. If a decorator is installed as `m_rootb.DataAccess`, call `decorator.Refresh()`.
 2. If the control is not visible, defer refresh.
-3. If `!m_rootb.NeedsReconstruct`, return without selection save/restore or drawing suspension.
+3. ~~If `!m_rootb.NeedsReconstruct`, return without selection save/restore or drawing suspension.~~
 4. Otherwise call `m_rootb.Reconstruct()`.
 
 This means the system has two layered assumptions:

openspec/changes/render-speedup-benchmark/tasks.md

@@ -85,6 +85,6 @@
 - [x] PATH-L1-VERIFY Run full benchmark suite and compare before/after timing evidence. Result: **99.99% warm render reduction** (153.00ms → 0.01ms). All 15 scenarios pass with 0% pixel variance. Cold render unaffected (62.33ms → 62.95ms).
 
 **Deferred** (future iterations):
-- [x] PATH-L5 Skip Reconstruct when data unchanged — gate `SimpleRootSite.RefreshDisplay()` on `VwRootBox.NeedsReconstruct` and cover it with focused tests.
+- [x] ~~PATH-L5 Skip Reconstruct when data unchanged — gate `SimpleRootSite.RefreshDisplay()` on `VwRootBox.NeedsReconstruct` and cover it with focused tests.~~ **Reverted 2026-07-10 (LT-22610, PR #1006):** the gate missed managed-only view-constructor state, causing stale views. `RefreshDisplay()` now always reconstructs.
 - [ ] PATH-L3 Per-paragraph layout caching — dirty-flag line-breaking in `VwParagraphBox::DoLayout()`.
 - [ ] PATH-L2 Deferred layout in Reconstruct — remove internal `Layout()` call from `Reconstruct()` (blocked: `RootBoxSizeChanged` callback needs dimensions immediately).