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

If the rasterizer has a valid surface context, use that to create a snapshotting render target.#4979

Merged
chinmaygarde merged 1 commit intoflutter:masterfrom
chinmaygarde:master
Apr 13, 2018
Merged

If the rasterizer has a valid surface context, use that to create a snapshotting render target.#4979
chinmaygarde merged 1 commit intoflutter:masterfrom
chinmaygarde:master

Conversation

@chinmaygarde
Copy link
Contributor

@chinmaygarde chinmaygarde commented Apr 12, 2018

Fixes snapshotting of textures uploaded on the IO thread and resident only on the GPU. Does not fix snapshotting of scenes with external textures.

Fixes flutter/flutter#16412. The engine refactor removed all the various places the engine used to screenshot layer trees and moved them to a single spot on the rasterizer. This patch fixes that call to handle IO thread uploaded textures.

@chinmaygarde
Copy link
Contributor Author

chinmaygarde commented Apr 12, 2018

Verified on iOS. The snapshot is on the top left.
1

jason-simmons
jason-simmons approved these changes Apr 12, 2018
View reviewed changes
shell/common/rasterizer.cc
frame->Raster(*tree, true);
canvas->flush();

// Prepare an image from the surface, this image may potentially be on th GPU.
Copy link
Member

@jason-simmons jason-simmons Apr 12, 2018

Choose a reason for hiding this comment

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

typo: "the"

@chinmaygarde chinmaygarde mentioned this pull request Apr 12, 2018
Closed
@cbracken
Copy link
Member

cbracken commented Apr 12, 2018

lgtm

@chinmaygarde
Copy link
Contributor Author

chinmaygarde commented Apr 13, 2018

Tree is green after the shell refactor. So I am assuming it stuck the landing this time. That unblocks this patch.

@chinmaygarde chinmaygarde merged commit 91dee59 into flutter:master Apr 13, 2018
@tvolkert
Copy link
Contributor

tvolkert commented Apr 17, 2018

This has been identified as the culprit of a Windows crash on AppVeyor trying to roll this into flutter/flutter.

https://ci.appveyor.com/project/flutter/flutter/build/1.0.11811

RUNNING: cd dev\automated_tests; ..\..\bin\flutter.bat test test_smoke_test\pass_test.dart
00:00 +0 -1: loading C:\projects\flutter sdk\dev\automated_tests\test_smoke_test\pass_test.dart [E]
  Failed to load "C:\projects\flutter sdk\dev\automated_tests\test_smoke_test\pass_test.dart":
  Shell subprocess crashed with unexpected exit code -1073741819 before connecting to test harness.
  Test: C:\projects\flutter sdk\dev\automated_tests\test_smoke_test\pass_test.dart
  Shell: C:\projects\flutter sdk\bin\cache\artifacts\engine\windows-x64\flutter_tester.exe

@tvolkert
Copy link
Contributor

tvolkert commented Apr 17, 2018

Scratch that - sounds like the culprit may be a different PR

@andreidiaconu
Copy link
Contributor

andreidiaconu commented May 18, 2018

@chinmaygarde While trying to screenshot images, sometimes they do not render. This seems to address a similar issue. Perhaps they are related? Opened issue: flutter/flutter#17687

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@brianosman brianosman Awaiting requested review from brianosman

2 more reviewers

@cbracken cbracken cbracken approved these changes

@jason-simmons jason-simmons jason-simmons 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.

Android:FlutterView.getBitmap can't capture Image&Videos.

6 participants

@chinmaygarde @cbracken @tvolkert @andreidiaconu @jason-simmons @googlebot