-
Notifications
You must be signed in to change notification settings - Fork 6k
Configure contexts to reduce shader variations. #27016
Configure contexts to reduce shader variations. #27016
Conversation
Context creation options for each backend were spread across multiple translation units. This makes setting options common across all backends hard to configure. I have moved the creation of such common options into a separate translation unit. Fixes flutter/flutter#84213
| } | ||
|
|
||
| // TODO(goderbauer): remove option when skbug.com/7523 is fixed. | ||
| options.fDisableGpuYUVConversion = true; |
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?
|
|
||
| options.fReduceOpsTaskSplitting = GrContextOptions::Enable::kNo; | ||
|
|
||
| options.fReducedShaderVariations = true; |
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.
Guessing this might be the cause of the failure in EmbedderTest.CanRenderGradientWithCompositor and others?
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.
Yes, I updated the test fixture. I could observe no noticeable differences in the results.
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.
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.
zanderso
left a comment
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.
lgtm!
Let's keep an eye on the framework roll and the FRoB for this PR.
|
Yeah, this one might be painful. We try to be diligent about not having pixel tests in the engine that are susceptible to minor changes and aliasing artifacts. But the couple of fixtures that were susceptible did trip. |
* ce489a5 Roll Skia from 0f1ac21185fe to b1590f15a32b (1 revision) (flutter/engine#27000) * 92e24d8 Record raster end as soon as frame is submitted (flutter/engine#26970) * 1f35cce Remove presubmit flake reporting instructions from issue template. (flutter/engine#26997) * f5bc55e Roll Skia from b1590f15a32b to eef5b0e933e3 (2 revisions) (flutter/engine#27001) * 8f57dfe Roll Fuchsia Mac SDK from z6trYeCMx... to R1ENSI-od... (flutter/engine#27002) * 360d2d8 Surface frame number identifier through window (flutter/engine#26785) * ed87f7c Roll Dart SDK from d2025958e351 to eca780278d49 (1 revision) (flutter/engine#27003) * f558e2a Roll Fuchsia Linux SDK from TqViQQzJo... to 4udsaggtH... (flutter/engine#27006) * 8ae3ff7 Fix Fuchsia build on Mac (flutter/engine#27007) * b259ea6 Revert "[Engine] Support for Android Fullscreen Modes (#25785)" (flutter/engine#27014) * 6e105c9 Roll Skia from eef5b0e933e3 to e5766b808045 (12 revisions) (flutter/engine#27011) * cfea27e Allow fuchsia_archive to accept a cml file and cmx file (flutter/engine#27012) * 9ef94a2 [web] Librarify paragraph/text files (flutter/engine#26888) * cc2c361 Roll Dart SDK from eca780278d49 to bbd701b4ba76 (1 revision) (flutter/engine#27015) * f6fcab4 Roll CanvasKit to 0.28.1 (flutter/engine#27013) * db8ed9e38 Configure contexts to reduce shader variations. (flutter/engine#27016) * 94876ed Roll Skia from e5766b808045 to 661abd0f8d64 (7 revisions) (flutter/engine#27020) * 35e297d Update ci.yaml documentation link (flutter/engine#26922) * 6a87e0b [web] fix actions flags in SemanticsTester (flutter/engine#26992) * 53ac32f Re-land Android fullscreen support (flutter/engine#27018) * 1417a82 [web] make analysis options delta of root options (flutter/engine#26991) * b84ebe5 Roll Skia from 661abd0f8d64 to 1bddd42b9897 (3 revisions) (flutter/engine#27023) * 4f3d88e [web] Librarify keyboard files (flutter/engine#26917) * 9d09594 Roll Dart SDK from bbd701b4ba76 to fff3a3747a18 (1 revision) (flutter/engine#27024)
Context creation options for each backend were spread across multiple translation units. This makes setting options common across all backends hard to configure. I have moved the creation of such common options into a separate translation unit. Fixes flutter/flutter#84213
Context creation options for each backend were spread across multiple translation units. This makes setting options common across all backends hard to configure. I have moved the creation of such common options into a separate translation unit. Fixes flutter/flutter#84213
Context creation options for each backend were spread across multiple
translation units. This makes setting options common across all backends hard to
configure. I have moved the creation of such common options into a separate
translation unit.
Fixes flutter/flutter#84213.