Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid reflow of shaped text #3566

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Conversation

SamBent
Copy link
Contributor

@SamBent SamBent commented Sep 24, 2020

Addresses #3099
This is a port of a servicing fix in .NET 4.7-4.8

Issue: Text that is narrower after shaping (e.g. applying ligatures and kerning) can reflow, yielding different line breaks than the original measure. Some words can disappear, and programmatic access can crash hard (FailFast).

Discussion:
The root cause turns out to be a faulty assumption in TextBlock's logic to choose a width to send to FormatLine in post-measure situations like render and hit-test. The intent is that FormatLine makes the same line-breaking decisions it did during measure. The faulty assumption is "Reflowing will not happen when Width is between _previousDesiredSize.Width and ReferenceWidth". [Comment quoted from TextBlock.CalcWrappingWidth. "Reflowing" means "different line-breaking decisions". Width is the width to be sent to FormatLine. ReferenceWidth and _previousDesiredSize.Width are the input and output widths, respectively, of the most recent measure.]

The assumption fails in cases where shaping (kerning, ligatures, etc.) decreases the width of the text. FormatLine uses unshaped glyph widths to determine how much text to consider in line-breaking decisions. But it reports the width of the lines it produces using shaped glyph widths. Thus choosing width = _previousDesiredSize.Width (shaped width) may cause FormatLine to consider less of the original text than it did at measure time, which can lead to reflowing.

In the repro, the text "ABCDE IAATA Corp." contains the sequence "ATA", which gets kerned closer together, making the shaped width of the first two words (83.3167) about 2.6 pixels less than its unshaped width (85.96). Measure calls FormatLine(115), which considers the full string (unshaped width 121.3633) and breaks after the second word, so ReferenceWidth = 115 and prevDesiredWidth = 83.3167. Then Render calls FormatLine(83.3167), which only considers the first two words (unshaped width 85.96) and breaks after the first word. This leads to the missing text and crashing text pointers.

The fix is to give FormatLine a width large enough to avoid reflowing. If the original width reflows, we do a binary search to discover the smallest width that doesn't reflow. It may change the result of rendering (and hit-testing), but only in the cases where reflowing occurred, i.e. only when the old results had missing text and were vulnerable to FailFast crashes. Those cases will pay a (slight) perf cost for the search, and the text may extend (slightly) outside the arrange-rect; this is preferable to crashing and losing text completely.

@SamBent SamBent requested a review from a team as a code owner September 24, 2020 00:50
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Sep 24, 2020
@SamBent SamBent changed the base branch from release/5.0-rc2 to release/5.0 October 7, 2020 00:28
@SamBent SamBent merged commit 45f3f33 into dotnet:release/5.0 Oct 7, 2020
@SamBent SamBent deleted the TextReflow branch October 7, 2020 21:54
dotnet-maestro bot added a commit that referenced this pull request Oct 16, 2020
[master] Update dependencies from dotnet/winforms
- Coherency Updates:
  - Microsoft.Win32.Registry: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.CodeDom: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Configuration.ConfigurationManager: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Diagnostics.EventLog: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.DirectoryServices: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Drawing.Common: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Reflection.MetadataLoadContext: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Security.AccessControl: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Security.Cryptography.Xml: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Security.Permissions: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Security.Principal.Windows: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Windows.Extensions: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.Platforms: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.IO.Packaging: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.ILDAsm: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.ILAsm: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - System.Resources.Extensions: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.App.Internal: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.App.Ref: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)
  - Microsoft.NETCore.App.Runtime.win-x64: from 6.0.0-alpha.1.20428.2 to 6.0.0-alpha.1.20515.8 (parent: Microsoft.Private.Winforms)

 - .NET Core WPF Build error on custom BaseIntermediateOutputPath #1718 (#3120)

* Use more robust relative path calculation in markup code generation

 - Do not reflow lines after measure

 - disconnect HostVisual on its own thread

 - Fix three scrolling hangs

 - remove useless Assert

 - handle re-entrant request to close ToolTip

 - Move StackTrace to error branch

 - FixedPage SOM bugs

 - Don't add null item peers

 - Use correct lock object for predefined packages

 - repair bad copy/paste

 - fix build break

 - remove AppContext switch

 - DataGrid.Copy: fail silently if clipboard is locked

 - update_intellisense_artifacts

 - Remove or replace Policheck violations in code comments (#3606)

 - Merge pull request #3542 from dotnet/rc2-askmode-BaseIntermediateOutputPath

Ask-Mode: [release/5.0-rc2] Custom intermediate output paths shouldn't break markup compilation

 - Merge pull request #3564 from SamBent/VSPFreeze

Vsp freeze

 - Merge pull request #3565 from SamBent/AutomationPeerNull

Avoid null item AutomationPeers

 - Merge pull request #3566 from SamBent/TextReflow

Avoid reflow of shaped text

 - Merge pull request #3567 from SamBent/HostVisualThreading

HostVisual threading

 - Merge pull request #3568 from SamBent/PopupReentrancy

Reentrancy when closing ToolTip

 - Merge pull request #3570 from SamBent/ClearTypeAntiAliasing

ClearType anti-aliasing

 - Merge pull request #3574 from SamBent/NoisyStackTrace

Move StackTrace to error branch

 - Merge pull request #3575 from SamBent/FixedPageOverlap

FixedPage SOM bugs

 - Merge pull request #3576 from SamBent/LockedClipboard

DataGrid.Copy: fail silently if clipboard is locked

 - Merge pull request #3577 from SamBent/PredefinedPackageSynch

App resource threading issue

 - Revert "Merge pull request #3542 from dotnet/rc2-askmode-BaseIntermediateOutputPath"

This reverts commit 3d4e91c, reversing
changes made to ab840c4.

 - Merge branch 'release/5.0' of https://github.com/dotnet/wpf into release/5.0

 - update intellisense version

 - missed a version

 - 1228498 [ wpf ][ PoliCheck ] - Defect : Term "nuked"

 - Merge pull request #3637 from dotnet/revertPath

[release/5.0] Revert path change to markup compiler

 - Merge pull request #3642 from dotnet/policheck2

1228498 [ wpf ][ PoliCheck ] - Defect : Term "nuked"

 - Update arcade

 - Merge remote-tracking branch 'upstream/release/5.0' into darc-master-f9327a78-0358-4132-8625-923a71a6b121

 - Update wpf-int dependencies

 - Replace null comparisons on non-nullable types that now cause compilation errors, due to the .NET SDK update.

 - PR feedback

 - PR feedback

 - Merge pull request #3640 from AdamYoblick/release/5.0

Update intellisense version

 - Merge remote-tracking branch 'upstream/release/5.0' into darc-master-f9327a78-0358-4132-8625-923a71a6b121

 - Merge branch 'darc-master-f9327a78-0358-4132-8625-923a71a6b121' of https://github.com/dotnet/wpf into darc-master-f9327a78-0358-4132-8625-923a71a6b121

 - Suppress obsolete API warnings. These must be fixed for 6.0.0.

 - Merge pull request #3659 from dotnet/fixdarcupdate

Temporarily suppress obsolete API errors to get WPF master building

 - Merge branch 'master' into darc-master-f9327a78-0358-4132-8625-923a71a6b121
@ghost ghost locked as resolved and limited conversation to collaborators Apr 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants