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

Remove System.Drawing code that doesn't work #54245

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

eerhardt
Copy link
Member

This code only runs on macOS, and expects mono's System.Windows.Forms assembly is loaded into the process. This is causing trim warnings, and it is easier just to delete this code than try to make ILLink happy.

This code is only called from Graphics.FromHwnd, which doesn't work because the Carbon.framework no longer exists on macOS. Issue #22221 is tracking the test failures that use FromHwnd.

Tagging the mono runtime devs I think might be interested here: @lambdageek @vargaz @marek-safar @steveisok @lewing @SamMonoRT

This code only runs on macOS, and expects mono's System.Windows.Forms assembly is loaded into the process. This is causing trim warnings, and it is easier just to delete this code than try to make ILLink happy.

This code is only called from Graphics.FromHwnd, which doesn't work because the Carbon.framework no longer exists on macOS. Issue dotnet#22221 is tracking the test failures that use FromHwnd.
@ghost
Copy link

ghost commented Jun 15, 2021

Tagging subscribers to this area: @safern, @tarekgh
See info in area-owners.md if you want to be subscribed.

Issue Details

This code only runs on macOS, and expects mono's System.Windows.Forms assembly is loaded into the process. This is causing trim warnings, and it is easier just to delete this code than try to make ILLink happy.

This code is only called from Graphics.FromHwnd, which doesn't work because the Carbon.framework no longer exists on macOS. Issue #22221 is tracking the test failures that use FromHwnd.

Tagging the mono runtime devs I think might be interested here: @lambdageek @vargaz @marek-safar @steveisok @lewing @SamMonoRT

Author: eerhardt
Assignees: -
Labels:

area-System.Drawing

Milestone: -

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

LGTM. If we ever want to support this we needed to rewrite it anyway because Carbon framework is no longer available in our supported macOS platforms. We would need to use Cocoa.

@eerhardt
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@eerhardt
Copy link
Member Author

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@eerhardt
Copy link
Member Author

Test failure is #54301.

@eerhardt eerhardt merged commit c70e2e4 into dotnet:main Jun 16, 2021
@eerhardt eerhardt deleted the RemoveDeadDrawingCode branch June 16, 2021 22:48
@akoeplinger
Copy link
Member

@safern FYI the Carbon.framework still exists on an M1 Big Sur, not sure where you saw it being removed.

@eerhardt
Copy link
Member Author

Our code doesn't appear to work on my mac:

Console.WriteLine(Graphics.FromHwnd(IntPtr.Zero));

Causes the following exception:

System.EntryPointNotFoundException has been thrown

"Unable to find an entry point named 'GetQDGlobalsThePort' in shared library '/System/Library/Frameworks/Carbon.framework/Versions/Current/Carbon'."

This seems to be similar: https://bugzilla.xamarin.com/56/56613/bug.html

@safern
Copy link
Member

safern commented Jun 17, 2021

I followed up with @akoeplinger and it is not like that the framework was removed but a lot of the APIs were removed, so some entry points we use no longer exist in the shared library.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants