A division by zero happens if the shadow color is fully transparent, and
the NaNs are getting assigned to the RGB values of the paint color being
handed to the rrect shadow draw. This doesn't cause a problem when wide
gamut is off because NaN output ends up being interpreted as zero (thus
making the shadow output fully transparent black, which happens to be
the expected and correct result). However, when a wide gamut attachment
is present, the NaN output ends up being interpreted as a negative
value.
Reproduction app:
```dart
import 'package:flutter/material.dart';
void main() => runApp(const GeneralDialogApp());
class EvilPainter extends CustomPainter {
@override
void paint(Canvas canvas, Size size) {
final Rect rect = Offset.zero & size;
canvas.drawPaint(Paint()..color = Colors.white);
canvas.saveLayer(null, Paint()..blendMode = BlendMode.srcOver);
canvas.drawShadow(Path()..addRect(Rect.fromLTRB(100, 100, 300, 300)),
Colors.black54, 15, false);
canvas.drawShadow(Path()..addRect(Rect.fromLTRB(100, 100, 300, 300)),
Colors.black54, 15, false);
canvas.drawShadow(Path()..addRect(Rect.fromLTRB(100, 100, 300, 300)),
Colors.transparent, 15, false);
canvas.restore();
}
@override
bool shouldRepaint(EvilPainter oldDelegate) => false;
@override
bool shouldRebuildSemantics(EvilPainter oldDelegate) => false;
}
class GeneralDialogApp extends StatelessWidget {
const GeneralDialogApp({super.key});
@override
Widget build(BuildContext context) {
return MaterialApp(
restorationScopeId: 'app',
home: CustomPaint(painter: EvilPainter()),
);
}
}
```
Before:
After:
These keep turning up, so I did a little bash-fu to find them all.
```bash
grep -lL "#pragma once" $(grep -lL "#ifndef .*_H_" $(find . | grep "\.h$")) | cut -c 3-
```
Makes it much easier to work out where things are going wrong in filter
chains.
For example, when capturing the app in
https://github.com/flutter/flutter/issues/124956, the chain of
RenderPasses gets labeled as follows:
1. Contents to ForegroundPorterDuffBlend Filter Snapshot
2. Contents to GaussianBlur Filter Snapshot
3. Directional Gaussian Blur Filter
4. Directional Gaussian Blur Filter
...and so it's easy to see that the filter chain is
ForegroundPorterDuffBlend->DirectionalGaussianBlur->DirectionalGaussianBlur,
and the former two are triggering a non-collapsed snapshot of their
inputs.
Resolves https://github.com/flutter/flutter/issues/124956.
Fixes some recent transform issues that mainly apply to chained filters
(like when both a color + image filter has been set).
- For all filters, the returned entity should just be an identity matrix
because the snapshot transform gets fully absorbed.
- For all filters, given entity's transform should be applied in the
coverage proc (although this currently makes no difference because of
the way coverage is queried when snapshotting).
- For filters drawing with coverage + transforming UVs, the transform
handed to the contents of the deferred entity needs to be applied.
Otherwise the entity won't get repositioned correctly while
snapshotting.
No change in functionality. I've just renamed the TU's to replace `display_list_` to `dl_`. I am sure there are chances to make the naming better. For instance, the op receiver still being named a dispatcher.
Changes since last dart roll
```
1f224df52be Version 3.1.0-15.0.dev
3c284a0c95c Revert "Reland "[VM] Begin supporting Perfetto file recorder""
754aca00499 Bump github/codeql-action from 2.2.11 to 2.2.12
7c8e61104f6 Bump actions/checkout from 3.5.0 to 3.5.2
f9f1030959d (origin/base) [infra] DEPS and test dart-lang/native packages
b04b0844067 [vm/win7] Dynamically load functions for unwinding instructions.
a6ebd6fa8e9 Bump tools to 545d7e1c73ce21b8c91f638021f9d487d324a501
c4795a75508 [vm, compiler] Don't do safepoint transitions in generated code under TSAN.
2fcf9754032 Version 3.1.0-14.0.dev
393b1016292 Version 3.1.0-13.0.dev
39fc15a0b4c (origin/lkgr) [analysis_server] Add missing declaration semantic token modifier on classes/constructors
609db33dc7a Issue 51985. Quick fix to add missing pattern case to switch expression/pattern.
8fa08ae80b6 Add 'Convert to if-case statement' quick assist.
17d92f90d0b offer wildcard suggestions for destructured record fields
3149f813bbf [dartdev] Update --enable-analytics to re-enable unified analytics
eee509140dd Add dependency overrides for pkg/analyzer
3069c4f3e2b [analysis_server] Add missing "constructor" semantic token modifiers on constructor declarations
9834f87c542 [dart2js] Fix noSuchMethod override handling.
639a6f06ef9 Simplify several non-terminals to use regex-like grammar operators
f88e3bd77c5 Issue 51903. Test fixes for ParserErrorCode.VAR_AND_TYPE
f4d42303ad2 [analysis_server] Handle completion in whitespace after string values
70762f0de0e [ANTLR] Transform recursive definition of onParts to use iteration ...
453647022f6 [analysis_server] Don't fail on invalid/complex setters in LSP code completion
a565638b910 Issue 51689. Report when 'super' is used as a standalone expression, not as a receiver of an invocation.
b78d89ce0c7 Fix DynamicTypeImpl to InterfaceTypeImpl cast exception in AddTypeAnnotation, when ambiguous SetOrMapLiteral.
bd9c7a46e2b Roll zlib to 14dd4c4455602c9b71a1a89b5cafd1f4030d2e3f
18b5ecdfc99 [vm] Cleanup dead code around TypeArgumentClassFinder
9d4d48949c0 Add a top-level variable strict-inference test case
8df1b888771 [VM/Service] Create JSONBase64String class
b4aa83b0b7f [VM/Service] Rename JSONStream::AppendSerializedObject to JSONStream::AppendBytes
4cd9c9c6666 Reland "[VM] Begin supporting Perfetto file recorder"
fa3a72fa7e5 [cfe/ffi] Error on compound constructors
```
Part of https://github.com/flutter/flutter/issues/125017.
Pulls ColorSource construction out of the dispatcher and into Aiks.
Cleans up a few design warts we've accumulated over time:
- Removes special exceptions for solid colors in `Paint`.
- Makes `SolidColorContents` a `ColorSourceContents`.
- Removes paint ref capture for kImage.
Remove unnecessary instrumentation from picture. `UniqueRef`
automatically instruments all allocations, disposals, and garbage
collections, which covers `CkPicture`.
Does two things:
- Exposes a string about the GPU model on `impeller::Context`
- passes that string on to Skia gold when we add a test image.
This should help reduce noise/flakiness in golden images.
fixes https://github.com/flutter/flutter/issues/124996
This is the best I could come up with easily. It doesn't give as good of a result as I'd hoped. I measured it with a simple microbenchmark derived from one of the unit tests and it gave a 6% decrease in time ( 241.314us vs 257.626us) on macos release builds (run with rosetta).
Improvements:
1) Removed the copying of the std::set to an std::vector
1) Uses references instead of copying FontGlyphPairs out of collections in a few places
1) Holds new glyphs as a vector of references to the set instead of copying all the FontGlyphPairs
1) Deletes more lines of code than it adds
## the benchmark
```diff
diff --git a/impeller/typographer/typographer_unittests.cc b/impeller/typographer/typographer_unittests.cc
index 01a11d494c..1b99afa699 100644
--- a/impeller/typographer/typographer_unittests.cc
+++ b/impeller/typographer/typographer_unittests.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <chrono>
#include "flutter/testing/testing.h"
#include "impeller/playground/playground_test.h"
#include "impeller/typographer/backends/skia/text_frame_skia.h"
@@ -149,23 +150,29 @@ TEST_P(TypographerTest, GlyphAtlasWithLotsOfdUniqueGlyphSize) {
sk_font);
ASSERT_TRUE(blob);
- TextFrame frame;
- size_t count = 0;
- TextRenderContext::FrameIterator iterator = [&]() -> const TextFrame* {
- if (count < 8) {
- count++;
- frame = TextFrameFromTextBlob(blob, 0.6 * count);
- return &frame;
- }
- return nullptr;
- };
- auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap,
- atlas_context, iterator);
- ASSERT_NE(atlas, nullptr);
- ASSERT_NE(atlas->GetTexture(), nullptr);
-
- ASSERT_EQ(atlas->GetTexture()->GetSize().width * 2,
- atlas->GetTexture()->GetSize().height);
+ auto beg = std::chrono::high_resolution_clock::now();
+ int count = 10000;
+ for (int i = 0; i < count; ++i) {
+ TextFrame frame;
+ size_t count = 0;
+ TextRenderContext::FrameIterator iterator = [&]() -> const TextFrame* {
+ if (count < 8) {
+ count++;
+ frame = TextFrameFromTextBlob(blob, 0.6 * count);
+ return &frame;
+ }
+ return nullptr;
+ };
+ auto atlas = context->CreateGlyphAtlas(GlyphAtlas::Type::kAlphaBitmap,
+ atlas_context, iterator);
+ ASSERT_NE(atlas, nullptr);
+ ASSERT_NE(atlas->GetTexture(), nullptr);
+ ASSERT_EQ(atlas->GetTexture()->GetSize().width * 2,
+ atlas->GetTexture()->GetSize().height);
+ }
+ auto end = std::chrono::high_resolution_clock::now();
+ auto duration = std::chrono::duration_cast<std::chrono::microseconds>(end - beg);
+ FML_LOG(ERROR) << "Elapsed Time: " << static_cast<double>(duration.count())/count << "us";
}
```
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
The artifacts have been used in prod for a few weeks already and this change is just migrating the generation of the artifacts to engine_v2 builds and moving the legacy recipe to staging.
[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
After having run a memory audit on Flutter Gallery I believe we can turn
on wide gamut support by default for impeller. Here are the
ramifications of this change:
1) If an opaque surface is used and no transparent wide gamut images are
used, there should be no change in performance or memory usage.
1) If a transparent surface is used; the surface will take up 2x memory
(ex 4MB on iPhone 7) and will have a runtime performance cost.
1) If wide gamut **transparent** images are used, they will take up 2x
memory. Opaque wide-gamut images will take up the same amount of memory.
## Flutter gallery test results
### Test 1
Steps:
* Tap "Material"
* Scroll down and back to "Cards"
* Tap "Cards"
* Scroll to bottom of "Cards" example
Notice that the card images are wide gamut, AdobeRGB.
**Memory increase:** 10.401 MB (+8.7%)
### Test 2
Steps:
* Just launch the gallery
Notice that there shouldn't be any wide gamut images.
**Memory Increase:** 1.22 MB (+1%, this is probably a fluke of
measurement)
## Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
