[Impeller] Use 32 bit for positional stuff in the rrect blur #42797

bdero · 2023-06-13T01:48:48Z

16 bit floats aren't precise enough for positional computations, like computing SDFs in the case of the RRect blur.

Unfortunately, I haven't been able to repro this particular glitch in a playground -- it seems to be iOS-specific. In macOS desktop frame captures the problem seems to go away.

Before:

After:

bdero · 2023-06-13T02:00:10Z

This does also resolve an issue that would cause the blur to get screwed up and disappear as the blur nears 256, which was already resolved in the regular 2-pass blur. I can add a playground to catch that.

jonahwilliams · 2023-06-13T02:47:43Z

impeller/entity/shaders/rrect_blur.frag

-  f16vec2 rect_size;
-  float16_t blur_sigma;
-  float16_t corner_radius;
+  vec2 rect_size;


here and elsewhere, mark these as highp so the issue is fixed for Vulkan too.

err, not elsewhere, but like, in the declaration.

jonahwilliams

LGTM with nit

bdero · 2023-06-14T20:04:42Z

Mali json step is failing with no useful output, unfortunately. I'm assuming it doesn't like something syntactically. :/

FAILED: gen/malioc/flutter/impeller/entity/rrect_blur.frag.gles.Mali-G78.json
vpython3 ../../build/gn_run_binary.py ../../../../arm-tools/mali_offline_compiler/malioc --format json --fragment --core Mali-G78 --output /b/s/w/ir/cache/builder/src/out/android_debug_unopt/gen/malioc/flutter/impeller/entity/rrect_blur.frag.gles.Mali-G78.json /b/s/w/ir/cache/builder/src/out/android_debug_unopt/gen/flutter/impeller/entity/gles/rrect_blur.frag.gles

This reverts commit a6c0488.

bdero · 2023-06-18T04:54:04Z

malioc is seeing an undeclared variable, which is quite strange.

FAILED: gen/malioc/flutter/impeller/entity/rrect_blur.frag.gles.Mali-G78.json 
vpython3 ../../build/gn_run_malioc.py ../../../../arm-tools/mali_offline_compiler/malioc /b/s/w/ir/cache/builder/src/out/android_debug_unopt/gen/malioc/flutter/impeller/entity/rrect_blur.frag.gles.Mali-G78.json --format json --fragment --core Mali-G78 /b/s/w/ir/cache/builder/src/out/android_debug_unopt/gen/flutter/impeller/entity/gles/rrect_blur.frag.gles

malioc output:
{
  "producer": {
    "build": "aeadf0",
    "documentation": "https://developer.arm.com/tools-and-software/graphics-and-gaming/arm-mobile-studio/learn/mali-offline-compiler-documentation",
    "name": "Mali Offline Compiler",
    "version": [
      7,
      8,
      0
    ]
  },
  "schema": {
    "name": "error",
    "version": 0
  },
  "shaders": [
    {
      "driver": "r41p0-00rel0",
      "errors": [
        "0:60: L0002: Undeclared variable 'mp_copy_463_1_1'"
      ],
      "filename": "/b/s/w/ir/cache/builder/src/out/android_debug_unopt/gen/flutter/impeller/entity/gles/rrect_blur.frag.gles",
      "hardware": {
        "architecture": "Valhall",
        "core": "Mali-G78",
        "revision": "r1p1"
      },
      "shader": {
        "api": "OpenGL ES",
        "type": "Fragment"
      },
      "warnings": []
    }
  ]
}
[4737/5560] ACTION //flutter/impeller/entity:analyze_gles_entity_shaders_rrect_blur.frag_Mali-T880_malioc(//build/toolchain/android:clang_arm)
FAILED: gen/malioc/flutter/impeller/entity/rrect_blur.frag.gles.Mali-T880.json 
vpython3 ../../build/gn_run_malioc.py ../../../../arm-tools/mali_offline_compiler/malioc /b/s/w/ir/cache/builder/src/out/android_debug_unopt/gen/malioc/flutter/impeller/entity/rrect_blur.frag.gles.Mali-T880.json --format json --fragment --core Mali-T880 /b/s/w/ir/cache/builder/src/out/android_debug_unopt/gen/flutter/impeller/entity/gles/rrect_blur.frag.gles

malioc output:
{
  "producer": {
    "build": "aeadf0",
    "documentation": "https://developer.arm.com/tools-and-software/graphics-and-gaming/arm-mobile-studio/learn/mali-offline-compiler-documentation",
    "name": "Mali Offline Compiler",
    "version": [
      7,
      8,
      0
    ]
  },
  "schema": {
    "name": "error",
    "version": 0
  },
  "shaders": [
    {
      "driver": "r23p0-00rel0",
      "errors": [
        "0:60: L0002: Undeclared variable 'mp_copy_463_1_1'"
      ],
      "filename": "/b/s/w/ir/cache/builder/src/out/android_debug_unopt/gen/flutter/impeller/entity/gles/rrect_blur.frag.gles",
      "hardware": {
        "architecture": "Midgard",
        "core": "Mali-T880",
        "revision": "r2p0"
      },
      "shader": {
        "api": "OpenGL ES",
        "type": "Fragment"
      },
      "warnings": []
    }
  ]
}

bdero · 2023-06-18T05:56:56Z

I'm going to land this without the highp change for now and defer investigating the malioc Android GLES shader issues. For now, this fixes the bug in iOS. I unfortunately also had trouble creating a working repro case for a CI golden...

I'd also like to get to a point where shader authors are not forced to contend with 3 different type resolution behaviors; Metal and Vulkan can be combined once we add support for 16 bit uniform buffer access to the Vulkan backend.
(I started pulling on this thread in #42818)

…lutter/engine#42797)

flutter/engine@7ffa135...d298f0b 2023-06-18 jonahwilliams@google.com Fix prefer_final_in_foreach analysis errors (flutter/engine#42971) 2023-06-18 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from xS5RzQLfpMEK1rNEd... to fgS2avQGq4x_sbZHU... (flutter/engine#42973) 2023-06-18 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from aBEyk5N2HlwOqvpwo... to BuzniDS9u_hdghV5f... (flutter/engine#42970) 2023-06-18 skia-flutter-autoroll@skia.org Roll Skia from dc2e0ebbd045 to f489be63a30a (1 revision) (flutter/engine#42968) 2023-06-18 skia-flutter-autoroll@skia.org Roll Skia from 85b4788bc3f1 to dc2e0ebbd045 (1 revision) (flutter/engine#42967) 2023-06-18 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from -PsA0LDB2FdS_4_vK... to xS5RzQLfpMEK1rNEd... (flutter/engine#42966) 2023-06-18 bdero@google.com [Impeller] Use 32 bit for positional stuff in the rrect blur (flutter/engine#42797) 2023-06-18 jonahwilliams@google.com Revert "[Impeller] correct default PSO pixel format and sample count." (flutter/engine#42962) 2023-06-18 jonahwilliams@google.com [Impeller] dont use concurrent runner to decode images on Android. (flutter/engine#42944) 2023-06-18 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from I1G8Ll0KcxmAoMT8u... to aBEyk5N2HlwOqvpwo... (flutter/engine#42964) 2023-06-18 skia-flutter-autoroll@skia.org Roll Skia from c56f38d79fe0 to 85b4788bc3f1 (2 revisions) (flutter/engine#42963) 2023-06-17 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Au1gd3XLMuSt1Z4oh... to -PsA0LDB2FdS_4_vK... (flutter/engine#42959) 2023-06-17 skia-flutter-autoroll@skia.org Roll Dart SDK from d1faf69c8253 to 5a9e97eebbed (1 revision) (flutter/engine#42958) 2023-06-17 skia-flutter-autoroll@skia.org Roll Skia from 2a9e11fd9d37 to c56f38d79fe0 (1 revision) (flutter/engine#42955) 2023-06-17 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from JMStJe6efcLuWMT_I... to I1G8Ll0KcxmAoMT8u... (flutter/engine#42954) 2023-06-17 skia-flutter-autoroll@skia.org Roll Dart SDK from 1a04f451a934 to d1faf69c8253 (1 revision) (flutter/engine#42953) 2023-06-17 skia-flutter-autoroll@skia.org Roll Dart SDK from b3bcbdf7de8d to 1a04f451a934 (1 revision) (flutter/engine#42951) 2023-06-17 skia-flutter-autoroll@skia.org Roll Skia from dc11c7ddb33f to 2a9e11fd9d37 (1 revision) (flutter/engine#42950) 2023-06-17 bdero@google.com [Impeller] Print malioc errors on CI (flutter/engine#42906) 2023-06-17 skia-flutter-autoroll@skia.org Roll Dart SDK from 8eaed3382237 to b3bcbdf7de8d (10 revisions) (flutter/engine#42949) 2023-06-17 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from -NW1eatBbmjvLaIcV... to Au1gd3XLMuSt1Z4oh... (flutter/engine#42948) 2023-06-17 a-siva@users.noreply.github.com Fix analyzer warnings (prefer_final_in_for_each is now the default) (flutter/engine#42943) 2023-06-16 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from fXVcR5tdj5wSd_OUz... to JMStJe6efcLuWMT_I... (flutter/engine#42942) 2023-06-16 skia-flutter-autoroll@skia.org Roll Skia from 64fa632d3b01 to dc11c7ddb33f (3 revisions) (flutter/engine#42939) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from -NW1eatBbmjv to fgS2avQGq4x_ fuchsia/sdk/core/mac-amd64 from fXVcR5tdj5wS to BuzniDS9u_hd If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC chinmaygarde@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

bdero added the e: impeller label Jun 13, 2023

bdero requested review from chinmaygarde and jonahwilliams June 13, 2023 01:48

bdero self-assigned this Jun 13, 2023

jonahwilliams reviewed Jun 13, 2023

View reviewed changes

jonahwilliams approved these changes Jun 13, 2023

View reviewed changes

bdero force-pushed the bdero/rrect32 branch from 403f6b8 to 267b878 Compare June 14, 2023 15:39

bdero force-pushed the bdero/rrect32 branch 2 times, most recently from 18a9d3e to 997c85c Compare June 15, 2023 23:31

bdero mentioned this pull request Jun 16, 2023

[Impeller] Malioc analysis build failed with no useful output flutter/flutter#128989

Closed

bdero force-pushed the bdero/rrect32 branch 3 times, most recently from 53a69d4 to 687c06b Compare June 16, 2023 03:50

bdero added 3 commits June 17, 2023 21:46

[Impeller] Use 32 bit for positional stuff in the rrect blur

585ebdd

highp

a6c0488

Revert "highp"

6ac2ef4

This reverts commit a6c0488.

bdero force-pushed the bdero/rrect32 branch from 687c06b to 6ac2ef4 Compare June 18, 2023 04:47

bdero mentioned this pull request Jun 18, 2023

[Impeller] [iOS] PhysicalModel shadows are drawn with artifacts under circumstances flutter/flutter#128410

Closed

2 tasks

bdero merged commit df4aa98 into flutter:main Jun 18, 2023

engine-flutter-autoroll mentioned this pull request Jun 18, 2023

Roll Flutter Engine from 7ffa1355f718 to ae2e9ec8bd54 (21 revisions) flutter/flutter#129081

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2023

df4aa98 [Impeller] Use 32 bit for positional stuff in the rrect blur (f…

c703024

…lutter/engine#42797)

engine-flutter-autoroll mentioned this pull request Jun 18, 2023

Roll Flutter Engine from 7ffa1355f718 to b41f9f7138c3 (22 revisions) flutter/flutter#129084

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2023

df4aa98 [Impeller] Use 32 bit for positional stuff in the rrect blur (f…

e1bea13

…lutter/engine#42797)

engine-flutter-autoroll mentioned this pull request Jun 18, 2023

Roll Flutter Engine from 7ffa1355f718 to b41f9f7138c3 (22 revisions) flutter/flutter#129087

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2023

df4aa98 [Impeller] Use 32 bit for positional stuff in the rrect blur (f…

9738010

…lutter/engine#42797)

engine-flutter-autoroll mentioned this pull request Jun 18, 2023

Roll Flutter Engine from 7ffa1355f718 to b41f9f7138c3 (22 revisions) flutter/flutter#129090

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2023

df4aa98 [Impeller] Use 32 bit for positional stuff in the rrect blur (f…

b7c0093

…lutter/engine#42797)

engine-flutter-autoroll mentioned this pull request Jun 18, 2023

Roll Flutter Engine from 7ffa1355f718 to a16dd8d8607b (23 revisions) flutter/flutter#129091

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 18, 2023

df4aa98 [Impeller] Use 32 bit for positional stuff in the rrect blur (f…

793e4f9

…lutter/engine#42797)

engine-flutter-autoroll mentioned this pull request Jun 19, 2023

Roll Flutter Engine from 7ffa1355f718 to d298f0bf720c (24 revisions) flutter/flutter#129092

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 19, 2023

df4aa98 [Impeller] Use 32 bit for positional stuff in the rrect blur (f…

751c766

…lutter/engine#42797)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Impeller] Use 32 bit for positional stuff in the rrect blur #42797

[Impeller] Use 32 bit for positional stuff in the rrect blur #42797

Uh oh!

bdero commented Jun 13, 2023 •

edited

Loading

Uh oh!

bdero commented Jun 13, 2023

Uh oh!

jonahwilliams Jun 13, 2023

Uh oh!

jonahwilliams Jun 13, 2023

Uh oh!

jonahwilliams left a comment

Uh oh!

bdero commented Jun 14, 2023

Uh oh!

bdero commented Jun 18, 2023

Uh oh!

bdero commented Jun 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[Impeller] Use 32 bit for positional stuff in the rrect blur #42797

[Impeller] Use 32 bit for positional stuff in the rrect blur #42797

Uh oh!

Conversation

bdero commented Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdero commented Jun 13, 2023

Uh oh!

jonahwilliams Jun 13, 2023

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Jun 13, 2023

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

bdero commented Jun 14, 2023

Uh oh!

bdero commented Jun 18, 2023

Uh oh!

bdero commented Jun 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bdero commented Jun 13, 2023 •

edited

Loading