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

Migrate System.Drawing.Common from dotnet/runtime into winforms #8633

Merged
merged 804 commits into from
Mar 13, 2023

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Feb 13, 2023

An announcement for the code move will be following soon.

Important: All commits except the last two are migrated from dotnet/runtime and shouldn't be reviewed.

Commits are migrated on 2/13/2023. Here's the script that I created to migrate them: https://gist.github.com/ViktorHofer/2be5314439e75860326a60cee6523bef

Microsoft Reviewers: Open in CodeFlow

qmfrederik and others added 30 commits March 18, 2020 17:44
* Update System.Drawing to reflect GDI+ changes

- Add support ValueTypePointer encoder parameters
- Support ColorSpace, ImageItems and SaveAsCmyk encoders

* Skip test on NetFx

* Add XML documentation

Commit migrated from dotnet/runtime@e3fd0e0
…et/runtime#34354)

* Fix filename typo

* Disable Bitmap round-tripping tests for old libgdiplus versions

Old libgdiplus versions have uninitialized stack variable bug that makes Bitmap round-tripping unreliable. The bug causes Bitmap.Flags, Bitmap.HorizontalResolution and Bitmap.VerticalResolution properties to be set to bogus values if the stack happens to contain certain bit patterns. This bug was fixed for libgdiplus 6 by https://github.com/mono/libgdiplus/commit/dotnet/runtime@81e45a1d5a3ac3cf035bcc3fabb2859818b6cc04#diff-c96a8261ecb168c12b44248208da21c0R118.

* Fix and simplify gdiplus library loading

Commit migrated from dotnet/runtime@e1fa5d7
…runtime#34656)

* Rename CoreFx.Private.TestUtilities to TestUtilities

* Add TestUtilities reference to slns

Commit migrated from dotnet/runtime@d0dc66b
* Enable analyzer rule xUnit1024

* Remove redundant test in System.Text.Json

The very next method is a Theory with inline data that tests this
exact scenario.

* Rename test methods in System.Text.Json

* Rename test methods in System.ComponentModel.Annotations

* Private methods -> local functions in System.Text.Json

* Rename test methods in System.Collections.Immutable

For RemoveNonExistingTest, just moved the logic from the single-use
helper into the method with the Fact attribute.

* Rename test methods in System.Globalization

* Rename test methods in System.Runtime.Extensions

In Math.cs, removed Round_Decimal_Digits Fact which manually tested
cases included in the MemberData of a Theory with the same name.

* Rename test methods in System.Private.Uri

* Rename test methods in System.Runtime.WindowsRuntime.UI.Xaml

* Rename test methods in System.Security.Cryptography.X509Certificates

* Rename test methods in System.Reflection

* Remove redundant test in System.IO.FileSystem.Watcher

Theory EndInit_ResumesPausedEnableRaisingEvents alredy tests
the same scenario as the removed Fact of the same name.

* Rename test methods in System.IO.FileSystem.Watcher

* Rename test methods in System.Composition.Runtime

* Rename test methods in System.Drawing.Primitives

* Rename test methods in System.Reflection.Emit

* Rename test methods in System.Text.RegularExpressions

* Private methods -> local functions in System.Threading.Tasks.Parallel

* Rename test methods in System.Threading.Tasks.Parallel

* Rename test methods in System.Threading.Tasks

* Simplify tests in System.Threading

* Rename test methods in System.Threading

* Rename test methods in Microsoft.VisualBasic.Core

* Rename test methods in System.DirectoryServices.AccountManagement

* Private methods -> local functions in System.Text.Encoding

* Rename test methods in System.Text.Encoding

* Rename test methods in System.Security.Cryptography.Primitives

* Rename test methods in System.Runtime.Numeric

* Rename test methods in System.CodeDom.Tests

* Rename test methods in System.Reflection.Emit.ILGeneration

* Rename test methods in System.Composition.AttributedModel

* Rename test methods in System.Data.DataSetExtensions

* Rename test methods in System.Runtime

* Rename test methods in System.Runtime.InteropServices

* Remove redundant test in System.ServiceModel.Syndication

* Rename test methods in System.ServiceModel.Syndication

* Rename test methods in System.Data.Common

* Rename test methods in System.Security.Cryptography.Xml

* Private methods -> local functions in System.Security.Cryptography.Xml

* Simplify tests in System.Security.Cryptography.ProtectedData

* Replace private methods with local functions...

In System.Security.Cryptography.Algorithms

* Rename test methods in System.Security.Cryptography.Algorithms

* Simplify tests code in System.Collections.Concurrent

* Rename test methods in System.Drawing.Common

* Rename test methods in System.Data.OleDb

* Rename test methods in System.ComponentModel.TypeConverter

* Rename test methods in System.Web.HttpUtility

* Rename test methods in System.Linq

* Private method -> local function in System.Linq

* Rename test methods in System.Memory

* Rename test methods in System.IO.IsolatedStorage

* Rename test methods in System.DirectoryServices.Protocols

* Rename test methods in System.Globalization.Extensions

* Private methods -> local functions in System.Globalization.Extensions

* Rename test methods in System.IO.Compression

* Rename test methods in System.Security.Cryptography.Encoding

* Rename test methods in Microsoft.Win32.Registry

* Rename test methods in System.IO.UnmanagedMemoryStream

* Rename test methods in System.Collections

* Rename test methods in System.Collections.Specialized

* Rename test methods in System.Private.Xml

* Rename test methods in System.IO.FileSystem

* Rename test methods in System.Diagnostics.StackTrace

* Rename test methods in System.Diagnostics

* Improve some test method renames

* Minor nit fixing

* More improvements to test method names

Commit migrated from dotnet/runtime@61733e2
Changes:
- enabled `runtime (Libraries Test Run release mono Windows_NT x64 Debug)` CI lane (Windows Nano Server CI leg is disabled);
- enabled the tests that pass;
- marked failing test assemblies/classes/methods with `ActiveIssue` attribute using respective GH issues;
- removed CoreFX.issues_windows.rsp file and related mono rsp arguments.

Fixes dotnet/runtime#1980

Commit migrated from dotnet/runtime@ab6e225
…ntime#34651)

* Fix PrinterSettings.SupportsColor to use the right PInvoke

* PR Fedback

* Fix test

* PR Feedbak

Commit migrated from dotnet/runtime@4265a13
…tnet/runtime#34998)

* Provide better exception when saving to non-existent directory

When saving an image to a non-existent directory, throw a DirectoryNotFoundException
vs a System.Runtime.InteropServices.ExternalException.

Fix dotnet/runtime#31367

* Change exception message to be more intuitive

Use: The directory {0} of the filename {1} does not exist.
instead of: The target directory {0} of the targetPath {1} does not exist.

Fix dotnet/runtime#31367

* Fix test on unix by using built path.

We were using a hardcoded path with backslashes. That didn't match the built path for the Assert.

* Make Test conditional as it requires GDI+

* Put directory check closer to where it's used to ensure that we
capture it from all Save variations.

* Remove trailing whitespace

* Update test to ensure it doesn't run on the .NET Framework

* Change CheckIfDirectoryExists to ThrowIfDirectoryDoesntExist
to match naming conventions for throw-style methods.

Fix dotnet/runtime#31367

* Fixes typo in test name

Fix dotnet/runtime#31367

Commit migrated from dotnet/runtime@8981ad7
* Change <Link>...</Link> to Link="..."

* Format existing Link="..."

Commit migrated from dotnet/runtime@890fab0
* Use GDI+1.1 API set in Windows 8 and later

* PR Feedback, add meaningful test

* Update System.Drawing test package from runtime-assets

* PR Feedback and skip test on full framework

Commit migrated from dotnet/runtime@7bd9e4f
* feedbacl from previous time

* remove TargetsNet* from ItemGroup

* removing targetFramework from .targets

Commit migrated from dotnet/runtime@0ac72ac
…in arm/arm64 (dotnet/runtime#35690)

* Reduce usage of Win7 and Win8 queues, move to CI and add CI runs in Win arm/arm62

* Disable tests blocking clean rolling build

* Disable more failing tests

* Disable more failing tests

Commit migrated from dotnet/runtime@8b38643
* Change netcoreapp5.0 to net5.0

* Remove tfm downgrades

* Rename S.R.CS.Unsfae include folder

Commit migrated from dotnet/runtime@000046f
…ime#35956)

* Use TargetFramework condition consistently in libs

* Add docs section to describe the guideline

* Update Microsoft.Extensions.DependencyInjection.csproj

* Outline TargetFramework DesingTimeBuild issue

* Update Microsoft.Extensions.DependencyInjection.csproj

Commit migrated from dotnet/runtime@2cda903
Now that we have a Split overload that takes a single char, the previous optimization to cache an array and reuse that array with the array-based overload isn't needed.

Commit migrated from dotnet/runtime@c3d7041
* Fix argument exception warnings on runtime

* Apply feedback

* Addressing feedback and test fix

* Fix test failures

* Fix tests for NetFx run

* Fix suppressed warning for socket.SendAsyn(e) and fix corresponding tests

* Applied feedback

Commit migrated from dotnet/runtime@ee0aadc
…ime#36443)

* Chckecing strings against length seem to have better perf

* Apply suggestions from code review

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>

* Apply suggestions from code review

Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>

* Update src/libraries/System.Drawing.Common/src/System/Drawing/BitmapSelector.cs

Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>

* Apply suggestions from code review

* Update src/libraries/System.Private.CoreLib/src/System/Reflection/AssemblyNameFormatter.cs

* fix build error

* Update SmtpClient.cs

* Update AssemblyNameFormatter.cs

* Update LoggingEventSource.cs

* Fix build error

* Apply suggestions from code review

* Update src/libraries/System.ServiceModel.Syndication/src/System/ServiceModel/Syndication/SyndicationContent.cs

* Fix build error

* Apply suggestions from code review

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>

Commit migrated from dotnet/runtime@1c048bd
…runtime#36805)

* Ensure Image.Save can handle non readable / seekable Streams

* Address feedback

* Fix Image.Save on Unix for write only non-seekable stream

* Remove test which uses bogus handle value

Commit migrated from dotnet/runtime@00685a7
…me#36910)

* Add linux arm/arm64 libraries checked coreclr test runs

* Add linux_musl_arm64

* Disable BitmapTests on linux checked coreclr

Commit migrated from dotnet/runtime@344085b
This change enables the interpreter on CI as well as providing an option for the local test run.

Commit migrated from dotnet/runtime@93618ad
* Exclude MemberNotNull from various tools

* Use [MemberNotNull] to reduce use of `= null!;`

Commit migrated from dotnet/runtime@5f43e51
… platforms (dotnet/runtime#37479)

RemoteExecutor doesn't work on platforms that can't spawn processes like iOS/Android/WebAssembly.

Commit migrated from dotnet/runtime@da10ba8
* Change Activity Default IdFormat to W3C

* Remove the prefix from the switch and fix a test

* address some of the feedback

* Scope the breaking change to .NET 5

* Fix running the test on NetFX

* Changed the define NET5 to W3C_DEFAULT_ID_FORMAT

* Add net5 and remove netstandard2.0 configuration in the package

* Return back netstandard 2.0 targetting

When removing it, caused a  CI source build failure

Commit migrated from dotnet/runtime@c5edc00
…me#35162)

* add DefaultPropertyAttribute and DefaultEventAttribute to PrintDocument class and add references to System.ComponentModel.TypeConverter

* add the DefaultPropertyAttribute and DefaultEventAttribute to PrintDocument in the ref

Add DefaultEventAttribute and DefaultPropertyAttribute to PrintDocument in PrintDoument.Unix.cs

Commit migrated from dotnet/runtime@9de1a7e
…8410)

This has three primary benefits:
1. In some cases, in particular for static fields where other static fields have non-default initialization, the C# compiler can't elide the default initialization, resulting in more IL.
2. It highlights places where the field is otherwise unused and can be deleted, but where the presence of the initialization was silencing the C# compiler's warnings about the unused field.
3. It increases consistency and decreases maintenance / code size / etc.

Commit migrated from dotnet/runtime@ed78fad
…otnet/runtime#38578)

* Turn on argument exception analyzer on runtime, fix failures found

Commit migrated from dotnet/runtime@06fa941
@JeremyKuhne
Copy link
Member

@ViktorHofer - this is looking good! The changes so far make sense to me.

@elachlan
Copy link
Contributor

Are we blocked by microsoft/win32metadata#1287?

@ViktorHofer
Copy link
Member Author

I verified that the produced package contains the correct assemblies, placeholder files, msbuild targets files and intellisense documentation files. I'm verifying right now if the produced Microsoft.Private.Winforms transport package includes System.Drawing.Common assets.

@ViktorHofer ViktorHofer changed the title [WIP] Migrate System.Drawing.Common from dotnet/runtime into winforms Migrate System.Drawing.Common from dotnet/runtime into winforms Mar 13, 2023
@ViktorHofer ViktorHofer marked this pull request as ready for review March 13, 2023 16:08
@ViktorHofer ViktorHofer requested a review from a team as a code owner March 13, 2023 16:08
@ViktorHofer
Copy link
Member Author

The PR is now ready to be reviewed and merged. @JeremyKuhne can you please take a look?

Summary of the follow-up work post-merge:

  1. Make sure that WindowsDesktop.Ref picks up the right System.Drawing.Common assemblies.
  2. Verify that packages flow correctly and that we have access to the new System.Drawing.Common nuget package in the feeds.
  3. Send a follow-up PR with the changes that went into S.D.C in runtime meanwhile.
  4. Remove System.Drawing.Common from runtime and use the new package instead.
  5. Migrate all the issues over to winforms
  6. Publish the announcement issues

Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

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

This looks good- thanks for all the work here @ViktorHofer!

@ViktorHofer
Copy link
Member Author

We don't want to squash the commits. Can someone create a merge commit please?

@ViktorHofer ViktorHofer removed the draft draft PR label Mar 13, 2023
@dreddy-work dreddy-work merged commit f941e88 into main Mar 13, 2023
@dreddy-work dreddy-work deleted the DrawingMigration branch March 13, 2023 18:43
@ghost ghost added this to the 8.0 Preview3 milestone Mar 13, 2023
@dreddy-work
Copy link
Member

We don't want to squash the commits. Can someone create a merge commit please?

Merged without squash.

@dreddy-work
Copy link
Member

@ViktorHofer, we are dealing with flaky tests in this repo but i will follow-up CI flows into WPF->Desktop.

ViktorHofer added a commit to dotnet/runtime that referenced this pull request Mar 13, 2023
System.Drawing.Common was merged into winforms with
dotnet/winforms#8633.

Removing its sources and using PackageReferences to reference it.
carlossanlop pushed a commit to dotnet/runtime that referenced this pull request Mar 16, 2023
* Remove System.Drawing.Common from runtime

System.Drawing.Common was merged into winforms with
dotnet/winforms#8633.

Removing its sources and using PackageReferences to reference it.

* Remove dependency and fix tests

* Fix shim S.D.C reference

* Remove runtime test which causes failures
@ghost ghost locked as resolved and limited conversation to collaborators Apr 12, 2023
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.