Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Run Flutter on iOS and Android with color correct Skia #3743

Merged
mattsarett merged 3 commits into from
Jun 9, 2017
Merged

Run Flutter on iOS and Android with color correct Skia #3743

mattsarett merged 3 commits into from
Jun 9, 2017

Conversation

@mattsarett
Copy link
Contributor

@mattsarett mattsarett commented Jun 6, 2017 

***Turns on color correct rendering for Android and iOS
***Communicates dst color space to raster cache
***Turns on color space aware image decoding

Test:
***color_testing_demo on Pixel XL
***flutter_gallery on iPad Mini and iPad Pro (haven't figured out how to run manual_tests on iOS)

TODO:
I needed to split up this CL somewhere. These are follow-up tasks.
***Make desktop backends color correct
***Make debugging tools (ex: encoding frames to png) preserve color space
***Investigate using UIKit API to allow iOS to fine tune color space of rendered content

mattsarett added 2 commits June 6, 2017 09:45
@mattsarett
Run Flutter on iOS and Android with color correct Skia (flutter#3716)
700a229 
***Turns on color correct rendering for Android and iOS
***Communicates dst color space to raster cache
***Turns on color space aware image decoding

Test:
***color_testing_demo on Pixel XL
***flutter_gallery on iPad Mini and iPad Pro (haven't figured out how to run manual_tests on iOS)

TODO:
I needed to split up this CL somewhere. These are follow-up tasks.
***Make desktop backends color correct
***Make debugging tools (ex: encoding frames to png) preserve color space
***Investigate using UIKit API to allow iOS to fine tune color space of rendered content
@mattsarett
Check for null canvas in layer tree
eab243d
@mattsarett mattsarett requested a review from chinmaygarde June 6, 2017 13:49
@mattsarett
Copy link
Contributor Author

mattsarett commented Jun 6, 2017

Alright let's try this again :).

I've added a null check for canvas in layer_tree, I think this should fix the crash in the software rasterizer.

@brianosman @liyuqian

@mattsarett
Copy link
Contributor Author

mattsarett commented Jun 6, 2017

@chinmaygarde

chinmaygarde
chinmaygarde approved these changes Jun 6, 2017
View reviewed changes
Copy link
Member

@chinmaygarde chinmaygarde left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm. Before we land this though, our buildbots will need to be ready with iOS 10. This is being tracked by @cbracken in flutter/flutter#10510.

@mattsarett
Copy link
Contributor Author

mattsarett commented Jun 6, 2017

Ahh ok, thanks for the update.

@mattsarett
Copy link
Contributor Author

mattsarett commented Jun 7, 2017

I've prepared another branch that handles some of cases I left as TODOs:
*Color correct Android software backend
*Color correct mac desktop backend
*Respecting color space when converting frames to PNG
*Minor Vulkan change

I think it makes sense to consider this as a follow-up (since this CL is already not small), but we also have the option to include it in this PR.
https://github.com/mattsarett/engine/tree/srgb2

@chinmaygarde
Copy link
Member

chinmaygarde commented Jun 7, 2017

Just an update. Steady progress is being on flutter/flutter#10510 to enable this patch to be built by our buildbots.

@mattsarett
Copy link
Contributor Author

mattsarett commented Jun 8, 2017

Thanks! I appreciate your and Chris' help with this.

@cbracken
Copy link
Member

cbracken commented Jun 9, 2017 •
edited
Loading

FYI flutter/flutter#10510 is now resolved: engine builds, framework with the new engine builds and passes.

You should be good to land this. Thanks for your patience! :)

@mattsarett mattsarett merged commit ffe8181 into flutter:master Jun 9, 2017
@cbracken cbracken mentioned this pull request Jun 13, 2017
Closed
goderbauer added a commit to goderbauer/engine that referenced this pull request Jun 14, 2017
@goderbauer
Revert "Run Flutter on iOS and Android with color correct Skia (flutt…
482b6f1 
…er#3743)"

This reverts commit ffe8181.
@Hixie
Copy link
Contributor

Hixie commented Jun 14, 2017

This patch is being reverted in #3775.

It caused a device hard crash on the flutter_gallery_ios__transition_perf benchmark on certain iOS devices (notably iPod Touch 6th gen with iOS 10.3.1 and iPhone 6 with iOS 10.3.2). We suspect it also caused a high-profile regression with Android emulators (flutter/flutter#10617).

goderbauer added a commit that referenced this pull request Jun 14, 2017
@goderbauer
Revert "Run Flutter on iOS and Android with color correct Skia (#3743)…
cfe70e0 
…" (#3775)

This reverts commit ffe8181.
@Hixie Hixie mentioned this pull request Jun 14, 2017
Closed
mattsarett added a commit to mattsarett/engine that referenced this pull request Jun 15, 2017
@mattsarett
Revert "Revert "Run Flutter on iOS and Android with color correct Skia (
128f43a 
flutter#3743)" (flutter#3775)"

This reverts commit cfe70e0.
brianosman added a commit that referenced this pull request Jun 24, 2017
@brianosman
Reland "Run Flutter on iOS and Android with color correct Skia" (#3818)
2650f52 
* Revert "Revert "Run Flutter on iOS and Android with color correct Skia (#3743)" (#3775)"

This reverts commit cfe70e0.

* Enable sRGB on IO thread, too

* Add 4444 as a fallback rendering mode

* Use bare ptr to SkColorSpace (not sk_sp) in PrerollContext
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@chinmaygarde chinmaygarde chinmaygarde approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mattsarett @chinmaygarde @cbracken @Hixie @googlebot