Refactor ShellTest to allow for different ShellTestPlatformViews #15972
Conversation
gw280
force-pushed
the
gwright-shell-test-platformview
branch
from
539eb9a to
3be386d
Compare
chinmaygarde
left a comment
chinmaygarde
left a comment
There was a problem hiding this comment.
The primary guidance is decoupling the client rendering API from the platform. This would allow us to test the using said client rendering API on all platforms where it is available (or can be emulated).
|
|
||
| #include "flutter/shell/common/shell_test_platform_view.h" | ||
|
|
||
| #if OS_FUCHSIA |
There was a problem hiding this comment.
I'd suggest adding a layer of abstraction over the platform and the client rendering API. In this instance, there is no reason Vulkan needs to be tied to Fuchsia. In fact, @iskakaushik expressed interest in setting up the test harness to use Vulkan on the host (using SwiftShader just like we test OpenGL in a cross platform manner) . This will allow us to test Vulkan functionality on the host in a cross platform manner with Fuchsia being just one of the users of Vulkan.
| namespace flutter { | ||
| namespace testing { | ||
|
|
||
| std::unique_ptr<ShellTestPlatformView> ShellTestPlatformView::Create( |
There was a problem hiding this comment.
We should really aim for all our tests to be as cross platform as possible. Towards this end, there is no reason that Vulkan can only be tested on Fuchsia (since it is either available or can be emulated everywhere). I suggest making this configurable via an enum that the test can select at runtime (or skip if not available). I was undertaking just such an effort that would collide with this one so I have closed it. But feel free to refer to #14023 for a guide on my thought process.
There was a problem hiding this comment.
So @iskakaushik suggested I remove the Vulkan stubs entirely for now, land the changes to the rest of the ShellTestPlatformView, and iterate on the rendering API runtime changes in a future PR.
My plan was to have Vulkan not tied to Fuchsia and this was more of a stepping stone towards getting there, but after Kaushik's suggestion I agree it'd be better to land this in distinct pieces so we don't end up with behemoth patches.
by different rendering APIs (OpenGL, Vulkan).
gw280
force-pushed
the
gwright-shell-test-platformview
branch
from
3be386d to
69125a9
Compare
|
Removed the Vulkan code for now, will iterate on those with @chinmaygarde's suggestions in a future PR. |
flutter/engine@51a7964...4218f80 git log 51a7964..4218f80 --first-parent --oneline 2020-01-25 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from 7fqYj... to 35pbn... (flutter/engine#15984) 2020-01-25 nurhan@google.com updating the versions of the browsers for flutter web engine unit tests (flutter/engine#15977) 2020-01-25 chris@bracken.jp Eliminate unused import in Android embedding (flutter/engine#15975) 2020-01-24 matthew-carroll@users.noreply.github.com Prevent duplicate plugin registration in FlutterEnginePluginRegistry. (#49365) (flutter/engine#15956) 2020-01-24 dnfield@google.com retry logic for another cipd upload (flutter/engine#15974) 2020-01-24 gw280@google.com Ensure GetFixturesPath works on Fuchsia (flutter/engine#15978) 2020-01-24 nurhan@google.com [web] Calling platform message callback after copy (flutter/engine#15950) 2020-01-24 iska.kaushik@gmail.com [fuchsia] Expose view_ref as part of dart:fuchsia initialization (flutter/engine#15958) 2020-01-24 gw280@google.com Refactor ShellTest to allow for different ShellTestPlatformViews (flutter/engine#15972) 2020-01-24 yjbanov@google.com Cache computed window.physicalSize in a FrameReference (flutter/engine#15955) 2020-01-24 dnfield@google.com the the fix (flutter/engine#15973) 2020-01-24 ferhat@gmail.com Optimize drawRRect to use dom_canvas (flutter/engine#15970) 2020-01-24 ferhat@gmail.com Remove paint apply in draw image (flutter/engine#15969) 2020-01-24 skia-flutter-autoroll@skia.org Roll fuchsia/sdk/core/mac-amd64 from 6_pZp... to 7fqYj... (flutter/engine#15966) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC aaclarke@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
…tter#15972) This paves the way for us to have shell_unittests backed by different rendering APIs (e.g. OpenGL, Vulkan, Metal).
…ews (flutter#15972)" This reverts commit f6056d9.
3 participants
This paves the way for shell_unittests to be backed by different rendering APIs (OpenGL, Vulkan).
Stub out a Vulkan implementation of ShellTestPlatformView.