VisualTreeHelper.GetDescendantBounds() reports incorrect value for Row with collapsed RowDetails that were previously expanded

[Metritutus]

• .NET Core Version: 3.1 and .NET 5
• Windows version: Version 21H1 (OS Build 19043.1110)
• Does the bug reproduce also in WPF for .NET Framework 4.8?: Yes

Problem description:
VisualTreeHelper.GetDescendantBounds() reports the wrong boundaries for a DataGridRow when that row's details were previously expanded and then collapsed.

This is very easy to reproduce and extremely frustrating.

InvalidateMeasure(), InvalidateArrange() and InvalidateVisual() do not correct the problem.
So far the only way I've found to get the correct value is to call DataGrid.Items.Refresh(), however this is completely overkill when needing to refresh a single row, both in performance and in functional impact that will affect selected items, expansion state, etc.

I'm desperately looking for workarounds that don't involve refreshing every item in the DataGrid, even if those solutions involve extensive reflection, etc.

From Googling, it's possible that this is related: https://social.msdn.microsoft.com/Forums/vstudio/en-US/d5ce7ab8-cd40-4f81-8a34-08abe4e67c5a/looks-like-visualcalculatesubgraphboundsinnerspace-internal-method-has-a-serious-bug?forum=wpf

Unfortunately I have been unable to get the workaround mentioned in that thread to work using the RowDetailsVisibilityChanged event of the DataGrid.

EDIT: I believe there are also other examples of this issue here (scroll down to the "Expander and VisualBrush" header - there's no archived link directly to the post, unfortunately, but the whole post is there) and here.

Actual behavior:
Row with collapsed row details reports previous boundaries.

Expected behavior:
Row with collapsed row details reports correct boundaries.

Minimal repro:
https://github.com/Metritutus/VisualTreeCollapseIssue001/

Repro instructions:

1. Run the repro project and click the CalculateRow1DescendantBounds button. Observe the value (for example: "Bounds: 0,0,783.6,29)")
2. Expand the first row by clicking that row's expander button to its left.
3. Click the CalculateRow1DescendantBounds button again. Observe the value (eg "Bounds: 0,0,783.6,37.92)")
4. Collapse the first row by clicking that row's expander button to its left.
5. Click the CalculateRow1DescendantBounds button again. Observe the value (eg "Bounds: 0,0,783.6,37.92)"). Note that the height returned is unchanged, where I would have expected it to be 29 again.
6. Click the RefreshItems button.
7. Click the CalculateRow1DescendantBounds button again. Observe the value (eg "Bounds: 0,0,783.6,29)").

[Metritutus]

I have managed to come up with a workaround to the issue (seen here on the workaround branch).

Now that I have investigated what's going on, I am uncertain if this is a bug or a feature. VisualTreeHelper.GetDescendantBounds() merely returns Visual.VisualDescendantBounds. The real work happens in the getter for the VisualDescendantBounds property of Visual. When the CalculateSubgraphBoundsInnerSpace() method loops through visual children, it does no checking for if those children occupy visible space.

Visibility is a property of UIElement, however I have found that the Visual class does have its own means of checking if a Visual would be visible. The Visual class contains a _flags field of type VisualFlags. Two of these flags, VisibilityCache_Visible and VisibilityCache_TakesSpace can be used to determine if the Visual will actually occupy space.

The workaround I have created lies in VisualTreeDescendantBoundsHelper.cs. The key change is here.

I've obviously made numerous changes due to extensive and horrifying reflection usage in order to access non-public types, members, etc and thus minimise the amount of code I needed to copy across.

If I were making changes in the repository itself, I'd simply replace this:

wpf/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Visual.cs

Line 493 in 83b9af1

if (child != null)

with this:

if (child != null && (CheckFlagsAnd(VisualFlags.VisibilityCache_Visible) || CheckFlagsAnd(VisualFlags.VisibilityCache_TakesSpace))

If the VisualDescendantBounds property of Visual is being calculated correctly as it currently stands, would a feature request be considered for the addition of another property, ie VisibleVisualDescendantBounds, along with a new method for VisualTreeHelper called GetVisibleDescendantBounds() which would look at said property? The names here are just provisional. It's possible another name variation would be better to avoid being misleading (as Hidden still occupies space despite not being Visible, so would be correctly included in bounds calculations).

Finally, even if the VisualDescendantBounds property of Visual is being calculated incorrectly, I assume fixing it would be a breaking change that might not be permitted.

Assuming that my analysis and understanding of all this is correct, I'd be willing to put together a pull request to add in the aforementioned helper method and Visual property if it would be permitted.