ImageFiltered Render Issue #105674
Comments
|
It is the ImageFilter diff rect is calculated as an error value. |
|
Is this effect happen all the time? |
|
@rivella50 Yeah, it happens all time. you can try my demo code to reproduce it. |
|
Issue is reproducible on In my testing, I was only able to reproduce this on iOS. recordings
code sampleimport 'dart:ui';
import 'package:flutter/material.dart';
class MyCustomScrollBehavior extends MaterialScrollBehavior {
// Override behavior methods and getters like dragDevices
@override
Set<PointerDeviceKind> get dragDevices => {
PointerDeviceKind.touch,
PointerDeviceKind.mouse,
// etc.
};
}
void main() {
runApp(const MyApp());
}
class MyApp extends StatelessWidget {
const MyApp({Key? key}) : super(key: key);
@override
Widget build(BuildContext context) {
return MaterialApp(
scrollBehavior: MyCustomScrollBehavior(),
home: const Scaffold(
backgroundColor: Colors.black,
body: BugTest(),
),
);
}
}
class BugTest extends StatefulWidget {
const BugTest({Key? key}) : super(key: key);
@override
State<BugTest> createState() => _BugTestState();
}
class _BugTestState extends State<BugTest> {
final ScrollController _controller = ScrollController();
@override
Widget build(BuildContext context) {
return Scaffold(
backgroundColor: Colors.lightBlue,
body: ListView(
physics: AlwaysScrollableScrollPhysics(),
controller: _controller,
children: <Widget>[
ImageFiltered(
imageFilter: ImageFilter.blur(
sigmaX: 4,
sigmaY: 4,
),
child: Container(
width: 50,
height: 50,
color: Colors.red,
),
),
Container(
height: 800,
),
],
),
);
}
}
flutter doctor -v |
danagbemava-nc
added
platform-ios
|
I may know why. void ImageFilterLayer::Paint(PaintContext& context) const {
....
// This change `child_paint_bounds` to `paint_bounds`
Layer::AutoSaveLayer save_layer =
Layer::AutoSaveLayer::Create(context, paint_bounds(), &paint);
PaintChildren(context);
}This is behavior: 2022-06-09.17.24.55.movHello Jim, do you the why use child_paint_bounds? |
The filter is applied to the contents of the save layer, so it needs an input with those dimensions. paint_bounds is the expanded dimensions that include all of the output of the filter so it is usually too large. This isn't just an optimization, consider applying a blur filter to a content that is filled with a rectangle. The edge conditions of the blur need to be applied to the edge of that rectangle so the filter input needs to be the size of that rectangle. If we used paint_bounds then the input would have transparent pixel padding around that rectangle and thus the edge conditions would apply to the transparent pixels rather than the rectangle background. |
|
Is there an older version that doesn't have this issue? It could be something we've done with Diff bounds or various caching changes, but it could also be a Skia boundary condition issue. If there is an older version that doesn't have the problem, bisecting might help pin down when it was added. Maybe try a version 4-6 months ago before we made a lot of recent changes to bounds code. |
|
Shouldn't the diff method of ImageFilterLayer (and ColorFilterLayer for that matter) update the transform of the diff context to the pixel snapped version? OpacityLayer: https://github.com/flutter/engine/blob/main/flow/layers/opacity_layer.cc#L25-L28 ImageFilterLayer: https://github.com/flutter/engine/blob/main/flow/layers/image_filter_layer.cc#L14 |
@flar hi, a question that if I use the |
@jonahwilliams do you mean this: void ImageFilterLayer::Diff(DiffContext* context, const Layer* old_layer) {
...
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
context->SetTransform(
RasterCache::GetIntegralTransCTM(context->GetTransform()));
#endif
DiffChildren(context, prev);
....
} |
If this is the code in the original description then it should be just a large red rectangle because the default tile mode for the blur is clamp. The one on the Skia attempts to ensure proper edge conditions for blur filters, but they still have some bugs that sometimes crop up. |
That sounds like an issue. Paging @knopp |
|
@flar Yeah, you are right, the default blue tile mode is clamp. |
|
There seem to be a few things that are causing this:
|
Not in |
|
I think the Integral CTM probably matters more in cases where there might be a 1:1 pixel blit (opacity, dl_picture, sk_picture) but ImageFilter won't be blitting the children, it will be using them as an input. If a layer is caching itself it probably matters more as that cache draw won't apply any filters (potentially just an alpha in the case of OpacityLayer). |
The issue is that You can verify this by
Any of these will fix the issue. I'll work on a PR. |
My point was not about bounds agreement between the passes, I was saying that we care about exact pixel alignment in cache::draw otherwise we can leave a fair bit of performance on the table on lower end devices. Integer CTM was created to help with that performance consideration. The headaches about how to get all of the bounds in all of the passes to agree are just side effects of having to use this performance trick. Having said that, I'm not sure anyone has verified if the performance still matters (and there are potentially other ways to achieve the "pixel for pixel blit" without having to slam all of the CTMs). OpacityLayer ends up with a cache entry that can match pixels from the cache image to the destination surface corner-by-corner center-by-center exactly and so it needs the Integer CTM to ensure performance. Layers that mostly are used for blur operations only use the cache entry as an input to a filter and so they never measure the pixels of the cache entry against the pixels on the destination because there is a complicated filtering system that is in the way which will dice and slice the pixels anyway. Thus, Integer CTM mangling matters to OpacityLayer in terms of performance. But it doesn't so much matter to the other layers. Having said that, once we've chosen that a particular layer cares about Integer CTM, then we need to make sure it matches across the various passes and methods on the layer. |
|
I just noticed that Dl_layer and SkP_layer also use it just for rendering. I think that was a boon for trying to avoid AA rectangles all over the place, but I'm not sure again how much that matters. So, DL/SkP layers also inherit that practice for their cached blits should they be cached. |
I try to use the Integer CTM to fix the issue. And then this scenario is normal. 2022-06-11.19.33.47.mov |
|
I disagree with the solution. Whereas caching can vary the result subtly from uncached rendering, that can be fixed in ways other than slamming all CTMs to integer translations. |
|
But raster cache already does that. Every time you paint raster cached layer the translation is already slammed to integer. The only difference being is that when painting raster cached layer the integer coordinates are determined through |
|
Also if you don't raster snap before rasterizing layer you'll get subtly different cached layer depending on current scroll offset for example. Don't get me wrong, I'm all for removing the |
|
(Conversation was continued on flutter/engine#33981) |
danagbemava-nc
added
r: fixed
|
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
flutter-triage-bot
bot
added
P0
The scenario is like this:
this is the behavior:
2022-06-09.14.00.36.mov
The engine is the latest version.
And I found this issue may be because of
Partial repaintThe second scenario is like this:
2022-06-09.15.45.49.mov
The second scenario and the first scenario may not be the same problem.
Because the second scenario I set the RasterCahe is nullptr, and don't use the partial repaint
The text was updated successfully, but these errors were encountered: