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

Port one-off line drawing fixes to RC2 #3961

Closed

Conversation

JeremyKuhne
Copy link
Member

@JeremyKuhne JeremyKuhne commented Sep 21, 2020

Proposing porting the changes that fix the class of off by one bugs for the end point of lines that were switched from GDI+ to GDI (for performance and memory usage). Note that the majority of the code in these commits are test code.

Fixes #3953
Fixes #3945
Fixes #3931
Fixes #3944

Proposed changes

  • Adjust end points one over to render correctly
  • Remove unreachable code
  • Move misplaced return statement

Customer Impact

  • Control vendors controls render incorrectly as they build on our helper code
  • Customer UI has gaps in numerous controls and strange artifacts in the NumericUpDown and DomainUpDown controls.

Regression?

  • Yes

Risk

  • Very low

Screenshots

UpDownControl example, before and after:

image
image

Good grab handle is on left, bad is on right: (while the right may look more appealing out of context making it "centered" would require all users to upgrade their code to get their existing behavior)

image

Good DrawBorder is on the left, bad on the right. The dotted/dashed are ok as that always draws with GDI+.

image

Bad DrawBorder on left, good on right.

image

Test methodology

  • Manual pixel validation between 3.1 and 5.0
  • Wrote automated rendering regression tests
  • Look at internal callers of helper methods to ensure they are using the APIs the way they used to
Microsoft Reviewers: Open in CodeFlow

…or the end point of lines that were switched from GDI+ to GDI (for performance and memory usage). Note that the majority of the code in these commits are test code.

Fixes dotnet#3953
Fixes dotnet#3945
Fixes dotnet#3931
Fixes dotnet#3944
@JeremyKuhne JeremyKuhne added the Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria label Sep 21, 2020
@JeremyKuhne JeremyKuhne added this to the 5.0 RC2 milestone Sep 21, 2020
@JeremyKuhne JeremyKuhne requested a review from a team as a code owner September 21, 2020 04:21
@ghost ghost assigned JeremyKuhne Sep 21, 2020
@codecov
Copy link

codecov bot commented Sep 21, 2020

Codecov Report

Merging #3961 into release/5.0-rc2 will increase coverage by 30.45333%.
The diff coverage is n/a.

@@                    Coverage Diff                     @@
##           release/5.0-rc2       #3961          +/-   ##
==========================================================
+ Coverage         67.64830%   98.10163%   +30.45332%     
==========================================================
  Files                 1410         491         -919     
  Lines               507284      256536      -250748     
  Branches             41180        4322       -36858     
==========================================================
- Hits                343169      251666       -91503     
+ Misses              158124        4134      -153990     
+ Partials              5991         736        -5255     
Flag Coverage Δ
#Debug 98.10163% <ø> (+30.45332%) ⬆️
#production ?
#test 98.10163% <ø> (-0.00615%) ⬇️

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

@merriemcgaw merriemcgaw removed the Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria label Sep 23, 2020
@merriemcgaw
Copy link
Member

Taking #3956 instead

@RussKie RussKie removed this from the 5.0 RC2 milestone Sep 25, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants