Comments (12)
gaaclarke
commented on July 22, 2024
1
I verified that this regressed as a result of flutter/engine#53261. I'm not sure if this use case has no golden coverage or if this is high dpi related which might not have good coverage in the golden test runner.
from flutter.
gaaclarke
commented on July 22, 2024
Can you please provide a complete code reproduction? Also a screenshot would be helpful.
from flutter.
gaaclarke
commented on July 22, 2024
Is this what you are seeing?
impeller
skia
from flutter.
mark8044
commented on July 22, 2024
@gaaclarke Very similar but not quite, mine looks like this with the code I have in my original post (might be different shadow setting then you have).
The difference is very stark when comparing the same exact code between iOS and Android, and iOS on master branch from yesterday compared to today (latest commit)
iOS - Flutter 3.23.0-13.0.pre.199 - Engine • revision c7fcbfce60 - Framework • revision 4afadeae3e
Using 'Inter' font and using the shadows listed above
I did flutter downgrade to goto an earlier version to show what it used to look like, it has taken me far down to a much earlier version unfortunately, but this is just to show the difference. On this earlier version iOS and Android look very similar
iOS - Flutter 3.22.0-42.0.pre - Engine • revision c89defa558 - Framework • revision e5217b670b
Using 'Inter' font and using the shadows listed above
from flutter.
mark8044
commented on July 22, 2024
Ok I worked backwards and did a git checkout on hash 966d2b92236b8d284db86b5e99e156e3f53e7bd3
which gives me:
Flutter 3.23.0-13.0.pre.190 • channel [user-branch] • unknown source
Framework • revision 966d2b9223 (12 hours ago) • 2024-06-11 22:07:23 -0700
Engine • revision 1cdbebee19
Tools • Dart 3.5.0 (build 3.5.0-236.0.dev) • DevTools 2.36.0
In that build its way better, but weirdly still not the same as the 3.22 screen shot:
But this is still more then acceptable and TBH this better matches what I would expect those blur settings to do (rather then the 3.22 version). Nevertheless it is different from 3.22 if it matters.
I hope thats somehow helpful
PS. Im happy to test this code out on other version hashes if it helps in any way
from flutter.
gaaclarke
commented on July 22, 2024
Hmm, before we even blur the text used for generating looks a bit off (The letters are jumbled and cropped at the top). I'm not sure if this is expected:
from flutter.
gaaclarke
commented on July 22, 2024
Here is the input of the gaussian blur with that PR reverted:
Notice that after that PR the input to the gaussian blur is 1/2 the size. That means we didn't account for high dpi correctly (we are missing the 2x effect transform).
from flutter.
gaaclarke
commented on July 22, 2024
I'm not quite sure how the PR that changed the gaussian blur is effecting the input to the gaussian blur. It may be the use of the filter's coverage before sending the input to the blur.
from flutter.
gaaclarke
commented on July 22, 2024
The following patch fixes the text shadow issue, but then regresses the behavior for the GaussianVsRRect tests:
--- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc
+++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc
@@ -417,11 +417,11 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
}
Entity snapshot_entity = entity.Clone();
- snapshot_entity.SetTransform(Matrix());
+ snapshot_entity.SetTransform(Matrix::MakeScale({2, 2, 1}));
std::optional<Rect> source_expanded_coverage_hint;
if (expanded_coverage_hint.has_value()) {
source_expanded_coverage_hint =
- expanded_coverage_hint->TransformBounds(entity.GetTransform().Invert());
+ expanded_coverage_hint->TransformBounds(Matrix::MakeScale({2, 2, 1}) * entity.GetTransform().Invert());
}
std::optional<Snapshot> input_snapshot =
@@ -563,8 +563,10 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
Entity blur_output_entity = Entity::FromSnapshot(
Snapshot{.texture = pass3_out.value().GetRenderTargetTexture(),
- .transform = entity.GetTransform() * input_snapshot->transform *
- padding_snapshot_adjustment *
+ .transform = entity.GetTransform() * //
+ Matrix::MakeScale({0.5, 0.5, 1}) * //
+ input_snapshot->transform * //
+ padding_snapshot_adjustment * //
Matrix::MakeScale(1 / effective_scalar),
.sampler_descriptor = sampler_desc,
.opacity = input_snapshot->opacity},
from flutter.
gaaclarke
commented on July 22, 2024
This patch fixes it, but it has the 2x dpi scalar hardcoded. I don't think there is a way we can query the dpi in impeller, looking into it.
--- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc
+++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc
@@ -390,7 +390,7 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
return std::nullopt;
}
- Vector2 scaled_sigma = (effect_transform.Basis() *
+ Vector2 scaled_sigma = (effect_transform.Basis() * Matrix::MakeScale({2, 2, 1}) *
Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_)))
.Abs();
scaled_sigma.x = std::min(scaled_sigma.x, kMaxSigma);
@@ -417,11 +417,11 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
}
Entity snapshot_entity = entity.Clone();
- snapshot_entity.SetTransform(Matrix());
+ snapshot_entity.SetTransform(Matrix::MakeScale({2, 2, 1}));
std::optional<Rect> source_expanded_coverage_hint;
if (expanded_coverage_hint.has_value()) {
source_expanded_coverage_hint =
- expanded_coverage_hint->TransformBounds(entity.GetTransform().Invert());
+ expanded_coverage_hint->TransformBounds(Matrix::MakeScale({2, 2, 1}) * entity.GetTransform().Invert());
}
std::optional<Snapshot> input_snapshot =
@@ -563,8 +563,10 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
Entity blur_output_entity = Entity::FromSnapshot(
Snapshot{.texture = pass3_out.value().GetRenderTargetTexture(),
- .transform = entity.GetTransform() * input_snapshot->transform *
- padding_snapshot_adjustment *
+ .transform = entity.GetTransform() * //
+ Matrix::MakeScale({0.5, 0.5, 1}) * //
+ input_snapshot->transform * //
+ padding_snapshot_adjustment * //
Matrix::MakeScale(1 / effective_scalar),
.sampler_descriptor = sampler_desc,
.opacity = input_snapshot->opacity},from flutter.
gaaclarke
commented on July 22, 2024
The following patch is a more appropriate fix in that it doesn't rely on some magic number. It however breaks the GaussianVsRRectBlur tests. If you comment out the line below it fixes those test, but breaks the text shadow (but also looks incorrect when scaling the object). I suspect this has to do with scaling the content but not scaling the size of a pixel?
I verified also that using the dpi scalar for the "source space" won't work when scaling text.
--- a/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc
+++ b/impeller/entity/contents/filters/gaussian_blur_filter_contents.cc
@@ -379,6 +379,14 @@ std::optional<Rect> GaussianBlurFilterContents::GetFilterCoverage(
return expanded_source_coverage.TransformBounds(entity.GetTransform());
}
+namespace {
+Vector2 ExtractScale(const Matrix& matrix) {
+ Vector2 entity_scale_x = matrix * Vector2(1.0, 0.0);
+ Vector2 entity_scale_y = matrix * Vector2(0.0, 1.0);
+ return Vector2(entity_scale_x.GetLength(), entity_scale_y.GetLength());
+}
+} // namespace
+
std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
const FilterInput::Vector& inputs,
const ContentContext& renderer,
@@ -390,7 +398,14 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
return std::nullopt;
}
+ const Vector2 source_space_scalar =
+ ExtractScale(entity.GetTransform().Basis());
+
Vector2 scaled_sigma = (effect_transform.Basis() *
+ // Uncomment the following line to break the text
+ // shadows, but fix GaussianVsRRectBlur tests.
+ // Matrix::MakeScale({source_space_scalar.x,
+ // source_space_scalar.y, 1}) * //
Vector2(ScaleSigma(sigma_x_), ScaleSigma(sigma_y_)))
.Abs();
scaled_sigma.x = std::min(scaled_sigma.x, kMaxSigma);
@@ -417,11 +432,13 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
}
Entity snapshot_entity = entity.Clone();
- snapshot_entity.SetTransform(Matrix());
+ snapshot_entity.SetTransform(
+ Matrix::MakeScale({source_space_scalar.x, source_space_scalar.y, 1}));
std::optional<Rect> source_expanded_coverage_hint;
if (expanded_coverage_hint.has_value()) {
- source_expanded_coverage_hint =
- expanded_coverage_hint->TransformBounds(entity.GetTransform().Invert());
+ source_expanded_coverage_hint = expanded_coverage_hint->TransformBounds(
+ Matrix::MakeScale({source_space_scalar.x, source_space_scalar.y, 1}) *
+ entity.GetTransform().Invert());
}
std::optional<Snapshot> input_snapshot =
@@ -436,7 +453,10 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
Entity result =
Entity::FromSnapshot(input_snapshot.value(),
entity.GetBlendMode()); // No blur to render.
- result.SetTransform(entity.GetTransform() * input_snapshot->transform);
+ result.SetTransform(entity.GetTransform() *
+ Matrix::MakeScale({1.f / source_space_scalar.x,
+ 1.f / source_space_scalar.y, 1}) *
+ input_snapshot->transform);
return result;
}
@@ -562,12 +582,16 @@ std::optional<Entity> GaussianBlurFilterContents::RenderFilter(
MinMagFilter::kLinear, SamplerAddressMode::kClampToEdge);
Entity blur_output_entity = Entity::FromSnapshot(
- Snapshot{.texture = pass3_out.value().GetRenderTargetTexture(),
- .transform = entity.GetTransform() * input_snapshot->transform *
- padding_snapshot_adjustment *
- Matrix::MakeScale(1 / effective_scalar),
- .sampler_descriptor = sampler_desc,
- .opacity = input_snapshot->opacity},
+ Snapshot{
+ .texture = pass3_out.value().GetRenderTargetTexture(),
+ .transform = entity.GetTransform() * //
+ Matrix::MakeScale({1.f / source_space_scalar.x,
+ 1.f / source_space_scalar.y, 1}) * //
+ input_snapshot->transform * //
+ padding_snapshot_adjustment * //
+ Matrix::MakeScale(1 / effective_scalar),
+ .sampler_descriptor = sampler_desc,
+ .opacity = input_snapshot->opacity},
entity.GetBlendMode());from flutter.
github-actions
commented on July 22, 2024
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 doctor -v and a minimal reproduction of the issue.
from flutter.
Related Issues (20)
- Missing `//flutter` in `//testing/scenario_app` causes a crash HOT 2
- FlexibleSpaceBar causing Null check operator used on a null value HOT 1
- kotlin gradle plugin failure only after installing tts in flutter app HOT 4
- Improve fidelity of `CupertinoRadio`
- [rfw] Goldens tests disabled due to blocking flutter->packages roll
- TextField becomes focused on navigator back (pop) action HOT 3
- Flutter GPU has analysis errors that probably (?) were just hidden before
- `ExpansionTileController` is unusable if the `ExpansionTile` changes HOT 2
- `TextPainter.getFullHeightForCaret` reports incorrect results for spans with different styles HOT 1
- mac-23 lost external connection phone device. HOT 1
- Dart on web: The integer negative zero formats as "-0.0" HOT 1
- Xcode Cloud CI Fails with File Write Error
- [tool_crash] ProcessException: Failed to find "/Users/chenguangshao/code/flutter_private/bin/cache/artifacts/engine/darwin-x64-release/gen_snapshot_arm64" in the search path. Command: /Users/chenguangshao/code/flutter_private/bin/cache/artifacts/engine/darwin-x64-release/gen_snapshot_arm64, OS error code: 0 HOT 1
- App Crashes while using iphone mirroring in MacOS sequoia and ios 18, both on beta HOT 1
- [CP] Fix TextPainter's caret position calculation for text containing full-width spaces HOT 2
- macOS app crashed HOT 2
- Webview (can not to load webview in android device) HOT 3
- [User] Flutter apps causing "critical" restarts on RMX3231 device HOT 2
- [tool_crash] FileSystemException: Cannot create link, OS Error: The system cannot find the path specified., errno = 3 HOT 1
- ./gradlew build fail HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from flutter.