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

Properly apply translation and clipping in TextRenderer #4525

Merged
merged 1 commit into from
Feb 10, 2021

Conversation

JeremyKuhne
Copy link
Member

@JeremyKuhne JeremyKuhne commented Feb 4, 2021

Translation and clipping should only be applied from Graphics objects when they are explicitly asked for in the TextFormatFlags.

Adds regression tests.

Fixes #4524

Proposed changes

  • Apply graphics properties only when requested
  • Add regression tests

Customer Impact

  • This causes rendering to not work correctly when transforms and clipping are in play.
  • This shows up in 3rd party control libraries, such as Telerik's. It can cause text to "vanish" as it isn't rendered in the clipped area.
  • Workarounds are difficult, particularly when this shows up in 3rd party control libraries.

Regression?

  • Yes

Risk

  • Very low

Screenshots

Before

image

After

image

Test methodology

  • Test Telerik provided repro
  • Audit of legacy code
  • Add regression tests
Microsoft Reviewers: Open in CodeFlow

Translation and clipping should only be applied from Graphics objects when they are explicitly asked for in the TextFormatFlags.

Adds regression tests.

Fixes dotnet#4524
@JeremyKuhne JeremyKuhne added 💥 regression-preview Regression from a preview release Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Feb 4, 2021
@JeremyKuhne JeremyKuhne added this to the 5.0.3 milestone Feb 4, 2021
@JeremyKuhne JeremyKuhne self-assigned this Feb 4, 2021
@JeremyKuhne JeremyKuhne requested a review from a team as a code owner February 4, 2021 01:37
@RussKie RussKie linked an issue Feb 4, 2021 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #4525 (759449a) into release/5.0 (ba8884e) will decrease coverage by 0.00729%.
The diff coverage is 100.00000%.

@@                  Coverage Diff                  @@
##           release/5.0       #4525         +/-   ##
=====================================================
- Coverage     68.27969%   68.27240%   -0.00729%     
=====================================================
  Files             1421        1421                 
  Lines           510635      510713         +78     
  Branches         41577       41580          +3     
=====================================================
+ Hits            348660      348676         +16     
- Misses          155620      155679         +59     
- Partials          6355        6358          +3     
Flag Coverage Δ
Debug 68.27240% <100.00000%> (-0.00729%) ⬇️
production 37.60027% <100.00000%> (-0.01302%) ⬇️
test 97.90274% <100.00000%> (-0.00721%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@RussKie RussKie added 🚫 * NO-MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) Servicing-approved .NET Shiproom approved the PR for merge and removed Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Feb 4, 2021
@RussKie RussKie modified the milestones: 5.0.3, 5.0.4 Feb 4, 2021
@RussKie RussKie removed the 🚫 * NO-MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Feb 10, 2021
@RussKie RussKie merged commit b04dbad into dotnet:release/5.0 Feb 10, 2021
RussKie pushed a commit that referenced this pull request Feb 10, 2021
Translation and clipping should only be applied from Graphics objects when they are explicitly asked for in the TextFormatFlags.

Adds regression tests.

Fixes #4524

(cherry picked from commit b04dbad)
@RussKie
Copy link
Member

RussKie commented Feb 24, 2021

FYI this change has a caused a regression in the PropertyGrid, see #4593 for more details.

RussKie added a commit to RussKie/winforms that referenced this pull request Mar 2, 2021
The fix introduced in dotnet#4525 has redirected the `PropertyGrid` value
rendering through a different code path which only relies on flags passed
into `DrawTextInternal` completely side-stepping any modifications applied
to the `Graphics` object. However the `PropertyGrid` value rendering
routines do modify the `Graphics` object (applying clipping and
transformations) but do not pass the necessary flags to `DrawTextInternal`.

Fix by applying the necessary flags to preserve the `Graphics` modifications,
and add debug-time asserts to check that necessary flags are applies,
if the underlying `Graphics` object is modified.

Fixes dotnet#4593
RussKie added a commit that referenced this pull request Mar 2, 2021
The fix introduced in #4525 has redirected the `PropertyGrid` value
rendering through a different code path which only relies on flags passed
into `DrawTextInternal` completely side-stepping any modifications applied
to the `Graphics` object. However the `PropertyGrid` value rendering
routines do modify the `Graphics` object (applying clipping and
transformations) but do not pass the necessary flags to `DrawTextInternal`.

Fix by applying the necessary flags to preserve the `Graphics` modifications,
and add debug-time asserts to check that necessary flags are applies,
if the underlying `Graphics` object is modified.

Fixes #4593
RussKie added a commit to RussKie/winforms that referenced this pull request Mar 2, 2021
The fix introduced in dotnet#4525 has redirected the `PropertyGrid` value
rendering through a different code path which only relies on flags passed
into `DrawTextInternal` completely side-stepping any modifications applied
to the `Graphics` object. However the `PropertyGrid` value rendering
routines do modify the `Graphics` object (applying clipping and
transformations) but do not pass the necessary flags to `DrawTextInternal`.

Fix by applying the necessary flags to preserve the `Graphics` modifications,
and add debug-time asserts to check that necessary flags are applies,
if the underlying `Graphics` object is modified.

Fixes dotnet#4593

(cherry picked from commit 34cdcf6)
RussKie added a commit that referenced this pull request Mar 10, 2021
The fix introduced in #4525 has redirected the `PropertyGrid` value
rendering through a different code path which only relies on flags passed
into `DrawTextInternal` completely side-stepping any modifications applied
to the `Graphics` object. However the `PropertyGrid` value rendering
routines do modify the `Graphics` object (applying clipping and
transformations) but do not pass the necessary flags to `DrawTextInternal`.

Fix by applying the necessary flags to preserve the `Graphics` modifications,
and add debug-time asserts to check that necessary flags are applies,
if the underlying `Graphics` object is modified.

Fixes #4593

(cherry picked from commit 34cdcf6)
@ghost ghost locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
💥 regression-preview Regression from a preview release Servicing-approved .NET Shiproom approved the PR for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TextRenderer always applies Graphics transforms and clipping
2 participants