Skip to content

Keep wrapped editor rows inside their own box - #441

Merged
Eli Pinkerton (wallstop) merged 9 commits into
masterfrom
fix/wrapped-editor-rows-and-open-issues
Aug 22, 2026
Merged

Keep wrapped editor rows inside their own box#441
Eli Pinkerton (wallstop) merged 9 commits into
masterfrom
fix/wrapped-editor-rows-and-open-issues

Conversation

@wallstop

@wallstop Eli Pinkerton (wallstop) commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Closes #440. Advances #336 and #410. Measurements posted to #330, #406, #307, #349, #439.

Wrapped editor rows drew outside their own box (#440)

Why. On Unity 2021.3 a wrapping row keeps the height of one line. The lines its children wrap
onto draw outside the row, on top of the block beneath it. The Flow Graph details pane and the
Message Monitor have twelve such rows, so a long type name or a deep hierarchy path can hide the
next block.

A second defect came out of testing it, and this one is live on every editor version. Wrapping a
scroll view's own content container makes Unity size that container to the viewport. Measured on
6000.4.6f1 with 24 filter tokens in a 72px scroll view: the content needed 105px, the container
reported 72px, and the scroller had nothing to scroll to. Every token past the first few lines was
unreachable.

What changed.

  • All twelve wrapping containers go through DxMessagingEditorTheme.ApplyContentSizedWrap. It
    measures the children and raises the container to fit them. An editor that already sizes the
    container writes no inline style at all.
  • The Monitor's lane pills and filter tokens wrap in a row the window owns, inside the scroll view
    rather than as the scroll view's content container. The same 24 tokens now give a 105px row and a
    working scroller.
  • A source scan fails if any editor file turns wrapping on by hand again.

How we know. A probe pins a wrapping row to one line's height, which is what 2021.3 does on its
own. The control probe leaves its children outside the row; the fixed probe does not. Tree-walk
tests assert every wrapping container in both windows contains its children at the smallest window
size. That is the assertion that reports the defect on the 2021.3 leg.

Run Pass Fail
EditMode, Tests.Editor + Tests.Editor.Allocations 1077 0
PlayMode, Tests.Runtime 1133 0
npm test 446 0

An MSVC compiler that is present but cannot start (#336)

Why. The IL2CPP gate checked that cl.exe exists. A cl.exe that is there but cannot start,
from a failed update or a missing sibling DLL, passed. The leg then took a Unity licence seat and
the build lock and failed twenty minutes later with a message naming neither the host nor the cause.

What changed. The gate now starts the compiler and reports a third verdict,
compiler-unusable, that names the symptom and the runner. A newer broken toolset no longer hides
an older working one.

The launch verdict is advisory, not blocking. This gate runs before the organization build lock, so
a probe that is wrong about a healthy host would block every IL2CPP leg on that runner, and step 1
of #336 is still open. Enforcing it is one field change.

How we know. Both cases run off Windows through the injected probe: 24 assertions, all passing.
Exit code 0 is not the test, because cl.exe with no input files exits non-zero on a healthy
toolchain, so a pure function reads the launch outcome from throws and NTSTATUS exit codes and is
tested directly.

Nothing watched how long a CI step takes (#410)

Why. A change added 78 seconds to the EditMode step on every editor leg and stayed green for
two days. The suite already measures itself and already warns past a 60s soft budget, but only into
the Unity log, which nobody opens on a green run.

What changed. run-ci-tests.ps1 lifts that one line into the job summary for every leg and
turns a soft breach into a workflow warning. No new script and no new workflow. It does not compare
against history; that is the option #410 calls most at odds with the tooling philosophy.

How we know. Six tests cover under budget, over budget, the once-per-job table header, a log
with no line, a missing log, and the shared line shape between the C# producer and the PowerShell
consumer. It would have flagged the regression above on its first run: 113.8s against 60s.

Pull request writing instructions

Why. Nothing said how to write a pull request, so descriptions grew long and full of jargon.

What changed. Added .llm/skills/pull-request-writing, referenced it from the Core Delivery
Rules, and rewrote .github/pull_request_template.md to the same shape: why, what changed, how we
know. Short Simplified Technical English, one idea per sentence.

Also found

PowerShell parses an unsuffixed hex literal as a signed integer, so 0xC0000000 is -1073741824.
The first launch probe compared an exit code against a bare NTSTATUS literal and called every
healthy compiler broken. Two tests caught it. Recorded in the shell-and-powershell skill.


Note

Medium Risk
Touches editor layout for Flow Graph and Message Monitor and the IL2CPP MSVC preflight. The new compiler-launch check is advisory so it should not block healthy runners, but a bad wrap-height measurement could still distort those windows.

Overview
Keeps wrapping rows in Flow Graph and Message Monitor inside their own box on Unity 2021.3, where a wrap row stayed one line tall and extra lines painted over the next block. All wrapping containers now go through ApplyContentSizedWrap, which measures children and raises min-height only when the box is too short. Monitor lane pills and filter tokens wrap in a window-owned row inside the scroll view so overflow can actually scroll.

Adds editor tests that pin the 2021.3 height, re-measure when a child grows, ignore unbounded layout sentinels, and fail if any editor source or USS turns wrap on by hand.

Also advances CI: the MSVC gate now tries to start cl.exe and reports advisory compiler-unusable without blocking; suite wall-clock lines from Unity logs are lifted into the job summary with a soft-budget warning. Adds a pull-request-writing agent skill and rewrites the PR template to Why / What changed / How we know.

Reviewed by Cursor Bugbot for commit 11c8251. Bugbot is set up for automated code reviews on this repo. Configure here.

On Unity 2021.3 a wrapping row keeps the height of one line, so the extra
lines draw on top of the block beneath it. The Flow Graph details pane and
the Message Monitor have twelve such rows.

All twelve now go through DxMessagingEditorTheme.ApplyContentSizedWrap. It
measures the children and gives the container that height. An editor that
already sizes the container is left alone.

The Monitor's lane pills and filter tokens now wrap in a row the window
owns. Unity sizes a scroll view's own content container to the viewport, so
wrapping it clipped the extra lines and left the scroller with nothing to
scroll to, on every editor version.

Adds the pull-request-writing skill and points the PR template at it.

Closes #440.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The IL2CPP gate checked that cl.exe exists. A cl.exe that is there but
cannot start, from a failed update or a missing sibling DLL, passed the
gate. The leg then took a Unity licence seat and the build lock and failed
twenty minutes later with a message that named neither the host nor the
cause.

The gate now also starts the compiler and reports a third verdict,
compiler-unusable, that names the symptom and the runner.

The launch verdict is advisory, not blocking. This gate runs before the
organization build lock, so a probe that is wrong about a healthy host
would block every IL2CPP leg on that runner, and no Windows host has yet
shown the probe is clean on a healthy toolchain. Enforcing it is one field
change once someone demonstrates that.

Both cases run off Windows through the injected probe. A pure function
reads the launch outcome, so the NTSTATUS exit codes are covered too.

Advances #336.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A change added 78 seconds to the EditMode step on every editor leg and
stayed green for two days. Nothing in CI reads how long a step takes, so
nobody saw it.

The suite already measures itself and already warns past a 60s soft budget,
but only into the Unity log, which nobody opens on a green run. The CI
harness now lifts that one line into the job summary and turns the soft
breach into a workflow warning.

No new script and no new workflow, and no comparison against history. That
keeps the cheapest option in issue #410 and leaves out the one the issue
calls most at odds with the tooling philosophy.

The suite formats its number with the invariant culture so the harness
parse does not depend on the runner locale. A test pins that shared line
shape from both sides.

Advances #410.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 22, 2026 06:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Three findings from reviewing this branch:

The wrap helper measured absolutely positioned children. Those are out of
flow and create no wrapped line, so one would have dragged the container's
height with it. They are skipped now.

The MSVC launch probe read its outcome outside its own try block. This
verdict is advisory, so an unexpected throw there would have failed the leg
the gate exists to protect. Everything is inside the try now.

The CI job summary header used an in-process flag, but the workflow runs
the harness once per test mode, so each leg is its own process and the
header printed three times per job. It is looked up in the summary file
now, and the test spawns one process per leg to prove it.

The wrap drift guard also scans the stylesheets. A stylesheet can turn
wrapping on just as well as a source file can.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 22, 2026 06:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 22, 2026 06:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The lane panel is built twice, once for message-type lanes and once for
context lanes, so a shared constant put the same element name in the tree
twice. The name is derived from the scroll view's own name now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 22, 2026 06:24
Found by applying the skill to this branch's own pull request, which closes
several issues and so runs past 400 words in total.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 22, 2026 06:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Performance Numbers

Measured commit: 11c8251
Trusted reporting code (PR base): a8db2b1
Workflow run: 32558940345 attempt 1

Benchmark or harness paths changed, so historical deltas are non-comparable:

  • scripts/unity/run-ci-tests.ps1

Historical Standalone delta

Delta direction: + is better; - is worse.

Historical delta omitted because benchmark or harness code changed in this pull request.

Current TargetMap evidence

These raw rows are current-run diagnostics. Use fresh bracketed controls before accepting a micro-optimization.

[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_16_Churn keyCount=16 operation=Churn totalOperations=2110000 operationsPerSecond=421414.958 wallClockMs=5006.941 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=16 targetMapCapacity=32 observedInvocations=2130000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_16_Hit keyCount=16 operation=Hit totalOperations=53160000 operationsPerSecond=10629573.906 wallClockMs=5001.141 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=16 targetMapCapacity=32 observedInvocations=53180000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_16_Miss keyCount=16 operation=Miss totalOperations=58530000 operationsPerSecond=11705666.623 wallClockMs=5000.142 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=16 targetMapCapacity=32 observedInvocations=0
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_1_Churn keyCount=1 operation=Churn totalOperations=2240000 operationsPerSecond=447306.559 wallClockMs=5007.751 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=1 targetMapCapacity=4 observedInvocations=2260000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_1_Hit keyCount=1 operation=Hit totalOperations=53060000 operationsPerSecond=10611395.999 wallClockMs=5000.285 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=1 targetMapCapacity=4 observedInvocations=53080000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_1_Miss keyCount=1 operation=Miss totalOperations=60990000 operationsPerSecond=12196877.887 wallClockMs=5000.460 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=1 targetMapCapacity=4 observedInvocations=0
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_256_Churn keyCount=256 operation=Churn totalOperations=1970000 operationsPerSecond=392287.407 wallClockMs=5021.828 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=256 targetMapCapacity=512 observedInvocations=1990000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_256_Hit keyCount=256 operation=Hit totalOperations=44590000 operationsPerSecond=8916666.067 wallClockMs=5000.748 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=256 targetMapCapacity=512 observedInvocations=44610000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_256_Miss keyCount=256 operation=Miss totalOperations=54170000 operationsPerSecond=10833678.023 wallClockMs=5000.149 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=256 targetMapCapacity=512 observedInvocations=0
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_4096_Churn keyCount=4096 operation=Churn totalOperations=1540000 operationsPerSecond=307017.115 wallClockMs=5016.007 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=4096 targetMapCapacity=8192 observedInvocations=1560000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_4096_Hit keyCount=4096 operation=Hit totalOperations=41450000 operationsPerSecond=8287702.317 wallClockMs=5001.386 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=4096 targetMapCapacity=8192 observedInvocations=41470000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_4096_Miss keyCount=4096 operation=Miss totalOperations=60100000 operationsPerSecond=12019355.763 wallClockMs=5000.268 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=4096 targetMapCapacity=8192 observedInvocations=0
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_4_Churn keyCount=4 operation=Churn totalOperations=2110000 operationsPerSecond=421750.012 wallClockMs=5002.964 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=4 targetMapCapacity=8 observedInvocations=2130000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_4_Hit keyCount=4 operation=Hit totalOperations=53490000 operationsPerSecond=10696348.270 wallClockMs=5000.772 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=4 targetMapCapacity=8 observedInvocations=53510000
[Standalone] DXM_TARGET_MAP_BENCHMARK scenario=TargetMap_4_Miss keyCount=4 operation=Miss totalOperations=56260000 operationsPerSecond=11248815.460 wallClockMs=5001.416 gcAllocations=-1 gcAllocatedBytes=-1 targetMapEntries=4 targetMapCapacity=8 observedInvocations=0
[Standalone] DXM_TARGET_MAP_CONSTRUCTION keyCount=1 wallClockMs=0.000109 operationsPerSecond=9149130.833 gcAllocations=-1 gcAllocatedBytes=-1 targetMapCapacity=4
[Standalone] DXM_TARGET_MAP_CONSTRUCTION keyCount=16 wallClockMs=0.000608 operationsPerSecond=1645244.216 gcAllocations=-1 gcAllocatedBytes=-1 targetMapCapacity=32
[Standalone] DXM_TARGET_MAP_CONSTRUCTION keyCount=256 wallClockMs=0.007650 operationsPerSecond=130718.954 gcAllocations=-1 gcAllocatedBytes=-1 targetMapCapacity=512
[Standalone] DXM_TARGET_MAP_CONSTRUCTION keyCount=4 wallClockMs=0.000228 operationsPerSecond=4385964.912 gcAllocations=-1 gcAllocatedBytes=-1 targetMapCapacity=8
[Standalone] DXM_TARGET_MAP_CONSTRUCTION keyCount=4096 wallClockMs=0.124500 operationsPerSecond=8032.129 gcAllocations=-1 gcAllocatedBytes=-1 targetMapCapacity=8192

… children

CI on Unity 2021.3, 2022.3 and 6000.3 found two real defects that the host
editor does not show.

A child laid out with no bound reports Unity's largest length, 8388608.
The helper treated that as a measurement and asked for a box eight million
pixels tall, which is where the 8388608 and 41943040 numbers in those legs
came from. Such a measurement is now ignored.

The helper also could not hear its own content grow. Unity reports a
geometry change to an element only when that element's own box changes, and
never reports a child's change to the parent, in either propagation phase.
So a container already held at a height never learned that its text had
finished measuring and needed more room. That left a Flow Graph details
header ten pixels short on 6000.3. Each child is now watched directly.

Both are covered by tests that fail without the fix.

Test-side, the layout settle now runs until the tree stops changing instead
of a fixed two passes, and the unset-inline-style assertion accepts both
keywords Unity uses for it: Null on 6000.4, Undefined on 2021.3 and 2022.3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 22, 2026 07:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@wallstop
Eli Pinkerton (wallstop) merged commit 00c881b into master Aug 22, 2026
44 of 45 checks passed
@wallstop
Eli Pinkerton (wallstop) deleted the fix/wrapped-editor-rows-and-open-issues branch August 22, 2026 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: wrapping containers in the Flow Graph details pane can render outside their box on Unity 2021.3

2 participants