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

Conversation

@chinmaygarde
Copy link
Member

Also adds a failing test for flutter/flutter#21398 that will be address in a subsequent patch.

Copy link
Contributor

@liyuqian liyuqian left a comment

Choose a reason for hiding this comment

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

RSLGTM

#include "flutter/shell/common/rasterizer.h"
#include "flutter/shell/common/shell.h"
#include "flutter/shell/common/thread_host.h"
#include "flutter/shell/gpu/gpu_surface_software.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

The following is a general comment on naming that does not need to be addressed in this PR:

The name of "gpu_surface_software" seems a little strange: if it's software, it's not a gpu surface...

This issue exists in many places such as "gpu thread" which may or may not involve gpu. Maybe at some point, we should substitute all "gpu" with "render" so they become "render_surface_software" and "render thread".

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. The support for various backends using different client rendering APIs was added much later and these naming patterns are an unfortunate relic of the old way of doing things. These APIs are not public however and so should be straightforward to rename to make more idiomatic.

FML_DISALLOW_COPY_AND_ASSIGN(TestPlatformView);
};

bool ValidateShell(Shell* shell) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: in Skia, we tend to put static before these functions (e.g., static bool ValidateShell) that are only used in this cc file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a pattern we follow in the engine too. I was just forgetful :(

@chinmaygarde chinmaygarde merged commit 10f9cab into flutter:master Oct 16, 2018
@chinmaygarde chinmaygarde deleted the deadlock branch October 16, 2018 21:30
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 17, 2018
flutter/engine@08272ee...04c860f

git log 08272ee..04c860f --no-merges --oneline
04c860f Revert "Roll Dart to ac6d4f7e653deba11d4836768376537893a9e9d6. (flutter#6549)" (flutter/engine#6568)
ef2ea12 Roll src/third_party/skia 5b90b84085cf..cb65ce7f77c9 (8 commits) (flutter/engine#6567)
065246e Roll src/third_party/skia 483536c242f3..5b90b84085cf (3 commits) (flutter/engine#6566)
b0008ca Roll src/third_party/skia 583161c09a3b..483536c242f3 (2 commits) (flutter/engine#6565)
e22da7c Roll src/third_party/skia 0a8faf3f557d..583161c09a3b (1 commits) (flutter/engine#6564)
915f5e0 Roll src/third_party/skia f831b64ed40e..0a8faf3f557d (5 commits) (flutter/engine#6563)
ad0fbd5 Roll src/third_party/skia c9092eb46754..f831b64ed40e (14 commits) (flutter/engine#6561)
6697d9d Update @animation dartdoc directives to current api (flutter/engine#6552)
10f9cab Ensure that the platform view is created and destroyed when running the shell unittests. (flutter/engine#6560)
df5f420 Roll src/third_party/skia 3d99b1e347be..c9092eb46754 (10 commits) (flutter/engine#6559)
215c146 Roll src/third_party/skia 1349ec4de78c..3d99b1e347be (3 commits) (flutter/engine#6556)
fc376dd Roll src/third_party/skia a583a06f831e..1349ec4de78c (1 commits) (flutter/engine#6555)
2dbbb2a Roll src/third_party/skia 198b87e5be20..a583a06f831e (5 commits) (flutter/engine#6554)
126e61d Roll src/third_party/skia b3067e8e1b6e..198b87e5be20 (2 commits) (flutter/engine#6553)
2cd79d1 Roll src/third_party/skia 4b7b2ceb4ad9..b3067e8e1b6e (1 commits) (flutter/engine#6551)
62cd86c Advertise iOS Observatory port over mDNS/Bonjour (flutter/engine#6533)
26f437f Fix crash when removeViewImmediate invoked from platform view (flutter/engine#6266)
aed6b8c Roll Dart to ac6d4f7e653deba11d4836768376537893a9e9d6. (flutter/engine#6549)
3ba6270 Roll src/third_party/skia 921ec976556c..4b7b2ceb4ad9 (14 commits) (flutter/engine#6550)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Oct 17, 2018
flutter/engine@08272ee...04c860f

git log 08272ee..04c860f --no-merges --oneline
04c860f Revert "Roll Dart to ac6d4f7e653deba11d4836768376537893a9e9d6. (#6549)" (flutter/engine#6568)
ef2ea12 Roll src/third_party/skia 5b90b84085cf..cb65ce7f77c9 (8 commits) (flutter/engine#6567)
065246e Roll src/third_party/skia 483536c242f3..5b90b84085cf (3 commits) (flutter/engine#6566)
b0008ca Roll src/third_party/skia 583161c09a3b..483536c242f3 (2 commits) (flutter/engine#6565)
e22da7c Roll src/third_party/skia 0a8faf3f557d..583161c09a3b (1 commits) (flutter/engine#6564)
915f5e0 Roll src/third_party/skia f831b64ed40e..0a8faf3f557d (5 commits) (flutter/engine#6563)
ad0fbd5 Roll src/third_party/skia c9092eb46754..f831b64ed40e (14 commits) (flutter/engine#6561)
6697d9d Update @animation dartdoc directives to current api (flutter/engine#6552)
10f9cab Ensure that the platform view is created and destroyed when running the shell unittests. (flutter/engine#6560)
df5f420 Roll src/third_party/skia 3d99b1e347be..c9092eb46754 (10 commits) (flutter/engine#6559)
215c146 Roll src/third_party/skia 1349ec4de78c..3d99b1e347be (3 commits) (flutter/engine#6556)
fc376dd Roll src/third_party/skia a583a06f831e..1349ec4de78c (1 commits) (flutter/engine#6555)
2dbbb2a Roll src/third_party/skia 198b87e5be20..a583a06f831e (5 commits) (flutter/engine#6554)
126e61d Roll src/third_party/skia b3067e8e1b6e..198b87e5be20 (2 commits) (flutter/engine#6553)
2cd79d1 Roll src/third_party/skia 4b7b2ceb4ad9..b3067e8e1b6e (1 commits) (flutter/engine#6551)
62cd86c Advertise iOS Observatory port over mDNS/Bonjour (flutter/engine#6533)
26f437f Fix crash when removeViewImmediate invoked from platform view (flutter/engine#6266)
aed6b8c Roll Dart to ac6d4f7e653deba11d4836768376537893a9e9d6. (flutter/engine#6549)
3ba6270 Roll src/third_party/skia 921ec976556c..4b7b2ceb4ad9 (14 commits) (flutter/engine#6550)


The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants