Add manual test for color space rendering behavior #10000
Conversation
Update from upstream
| body: new ListView( | ||
| padding: new EdgeInsets.all(5.0), | ||
| children: <Widget>[ | ||
| new Image.asset('packages/flutter_gallery_assets/ali_connors.jpg', fit: BoxFit.contain), |
There was a problem hiding this comment.
These are placeholders for the actual images that I want to use. Can you instruct on how to add images to flutter_gallery_assets?
|
This manual test should really be a standalone app in dev/manual_tests, not a page in the gallery demo. |
|
Got it thanks, I'll move this to a test app. |
|
@HansMuller Are you able to explain how to run the files in the dev/manual_tests? |
|
|
I'm running in to an error: msarett@msarett1:~/flutter/flutter/dev/manual_tests$ ../../bin/flutter run page_view.dart https://github.com/flutter/flutter/wiki/Upgrading-Flutter-projects-to-build-with-gradle for details on how to upgrade the project. |
|
very exciting. |
|
@mattsarett do you have more than one copy of flutter installed? That message is very old, and would suggest that you have a newer copy of |
|
@eseidelGoogle I'm no longer working in Flutter gallery because I'm hoping to move my test code into dev/manual_tests. Would that make a difference? I'll go ahead and see if I seem to be out of date anywhere. |
|
Ah, dev/manual_tests is probably out of date, yes. @jason-simmons or @yjbanov could probably help us upgrade that for you. |
|
Ah got it thanks, I appreciate the help! |
|
FYI: #10057 is fixing the upgrade issue |
|
Thanks Jason and Yegor! This should be ready after #10057 lands. |
|
@HansMuller please take a look. I've moved the test into dev/manual_tests. |
|
We shouldn't check in graphics assets into this repo. Can you use the flutter_gallery_assets package instead? See |
|
Thanks for the example. Looks like I'll want to land a change to the flutter_gallery_assets repo first? |
|
@Hixie Is it ok to push directly to that repo or is there a review system? |
|
I'm afraid you have entered a dark place. :-) You want to submit a review on https://flutter-review.googlesource.com/ (the repo itself is https://flutter.googlesource.com/gallery-assets/). You'll want to make sure you describe the license for each file in the README. To actually submit the code review you'll want to follow the esoteric steps described here: I am really sorry about the complexity here. If this all seems like far too much red tape to contribute to a project then please don't hesitate to just leave this PR open and eventually we'll deal with the images ourselves. (In that case, please do leave a comment here describing the license situation for each image and also maybe the technical details of each image file, for example what is important about each image such as what color spaces each image must use and what colors it must include and so forth, so that if we "optimize" them by converting to JPEG or using |
|
No problem, thanks for the detail. I'll follow up if I have questions. |
This is for <flutter/flutter#10000>. I considered using the Gallery assets repo, but it seems wrong to include these assets in the gallery app, so I decided to use this repo instead.
|
Ok the images are now available at: https://flutter.github.io/assets-for-api-docs/tests/colors/gbr.png I recommend changing the app to use [Image.network] and just fetch them by URL, rather than trying to put them into a package. |
|
Thanks very much Ian! I think the latest version follows your suggestion. I'll need to verify the behavior when I'm back in the office tomorrow. |
|
This needs a test (next to https://github.com/flutter/flutter/blob/master/dev/manual_tests/test/card_collection_test.dart), but other than that LGTM. |
|
Got it. I've added a very basic test. |
|
LGTM. Will land when build is green. |
The following content should render with noticeable differences depending on whether Flutter is using color correct Skia.