-
Notifications
You must be signed in to change notification settings - Fork 6k
Configure contexts to reduce shader variations. #27016
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "flutter/shell/common/context_options.h" | ||
|
|
||
| #include "flutter/common/graphics/persistent_cache.h" | ||
|
|
||
| namespace flutter { | ||
|
|
||
| GrContextOptions MakeDefaultContextOptions(ContextType type, | ||
| std::optional<GrBackendApi> api) { | ||
| GrContextOptions options; | ||
|
|
||
| if (PersistentCache::cache_sksl()) { | ||
| options.fShaderCacheStrategy = GrContextOptions::ShaderCacheStrategy::kSkSL; | ||
| } | ||
| PersistentCache::MarkStrategySet(); | ||
| options.fPersistentCache = PersistentCache::GetCacheForProcess(); | ||
|
|
||
| if (api.has_value() && api.value() == GrBackendApi::kOpenGL) { | ||
| options.fAvoidStencilBuffers = true; | ||
|
|
||
| // To get video playback on the widest range of devices, we limit Skia to | ||
| // ES2 shading language when the ES3 external image extension is missing. | ||
| options.fPreferExternalImagesOverES3 = true; | ||
| } | ||
|
|
||
| // TODO(goderbauer): remove option when skbug.com/7523 is fixed. | ||
| options.fDisableGpuYUVConversion = true; | ||
|
|
||
| options.fReduceOpsTaskSplitting = GrContextOptions::Enable::kNo; | ||
|
|
||
| options.fReducedShaderVariations = true; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guessing this might be the cause of the failure in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I updated the test fixture. I could observe no noticeable differences in the results.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I did verify that its just because of the shader variations. That is, it was not a side effect of me moving the context option setup into a common translation unit. The test pass if I disable reduced shaders variants. |
||
|
|
||
| return options; | ||
| }; | ||
|
|
||
| } // namespace flutter | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #ifndef FLUTTER_SHELL_COMMON_CONTEXT_OPTIONS_H_ | ||
| #define FLUTTER_SHELL_COMMON_CONTEXT_OPTIONS_H_ | ||
|
|
||
| #include <optional> | ||
|
|
||
| #include "flutter/fml/macros.h" | ||
| #include "third_party/skia/include/gpu/GrContextOptions.h" | ||
|
|
||
| namespace flutter { | ||
|
|
||
| enum class ContextType { | ||
| /// The context is used to render to a texture or renderbuffer. | ||
| kRender, | ||
| /// The context will only be used to transfer resources to and from device | ||
| /// memory. No rendering will be performed using this context. | ||
| kResource, | ||
| }; | ||
|
|
||
| //------------------------------------------------------------------------------ | ||
| /// @brief Initializes GrContextOptions with values suitable for Flutter. | ||
| /// The options can be further tweaked before a GrContext is created | ||
| /// from these options. | ||
| /// | ||
| /// @param[in] type The type of context that will be created using these | ||
| /// options. | ||
| /// @param[in] type The client rendering API that will be wrapped using a | ||
| /// context with these options. This argument is only required | ||
| /// if the context is going to be used with a particular | ||
| /// client rendering API. | ||
| /// | ||
| /// @return The default graphics context options. | ||
| /// | ||
| GrContextOptions MakeDefaultContextOptions( | ||
| ContextType type, | ||
| std::optional<GrBackendApi> api = std::nullopt); | ||
|
|
||
| } // namespace flutter | ||
|
|
||
| #endif // FLUTTER_SHELL_COMMON_CONTEXT_OPTIONS_H_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While moving this, I followed up on the linked bug. It has been marked as closed. I believe we can remove this option and resolve the TODO. I didn't do it here because I wanted to keep the contents of the patch on topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just came across this too and think we should remove it, but do we have a test case to show that it's safe now?