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

Use Clang for Windows (m79) #1006

Merged
merged 4 commits into from
Nov 11, 2019
Merged

Use Clang for Windows (m79) #1006

merged 4 commits into from
Nov 11, 2019

Conversation

mattleibow
Copy link
Contributor

@mattleibow mattleibow commented Nov 11, 2019

Description of Change

Describe your changes here.

Bugs Fixed

Provide links to issues here. Ensure that a GitHub issue was created for your feature or bug fix before sending PR.

API Changes

List all API changes here (or just put None), example:

Added:

  • string Class.Property { get; set; }
  • void Class.Method();

Changed:

  • object Cell.OldPropertyName => object Cell.NewPropertyName

Behavioral Changes

Describe any non-bug related behavioral changes that may change how users app behaves when upgrading to this version of the codebase.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Updated documentation

@mattleibow
Copy link
Contributor Author

This is very weird. I got a 32-bit build locally, now I can't. Seems to have issues with 64-bit and the linker. CI has all sorts of weird issues, and now I have those same issues. How did I get a working build locally? I know it was a real build because I saw a massive change in the performance of drawing. NICE!

@Gillibald
Copy link
Contributor

Maybe we should check out the original build file https://github.com/google/skia/blob/master/gn/BUILD.gn

@mattleibow
Copy link
Contributor Author

I don't think I changed anything around clang - I have been trying to make sure I make minimal changes. I updated to Clang 9 and it seemed to build locally. Just updating my agents to that and we shall see. I did a test a local build and ran the benchmarks for #758 - I saw MASSIVE improvements. Instead of taking 50000us, it took 900us - and the unrotated canvas was 200us. I think we got ourselves a winner. As soon as CI is green, then I'll post the binaries here for testing - it is the new version though. And, I'll see what it takes to get the currently released version using clang so we can get these same improvements there.

@mattleibow mattleibow changed the title Use Clang for Windows Use Clang for Windows (m79) Nov 11, 2019
@mattleibow
Copy link
Contributor Author

Trying to get m68 (the current version) using clang in #1007

@mattleibow mattleibow marked this pull request as ready for review November 11, 2019 20:16
@mattleibow mattleibow merged commit 4127a72 into dev/update-skia Nov 11, 2019
@mattleibow mattleibow deleted the dev/use-clang-cl branch November 11, 2019 20:17
@mattleibow
Copy link
Contributor Author

Clang works, so merging all the PRs into branches. 🎉

@mattleibow mattleibow added this to the v2.80.0 milestone Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants