[Impeller] support half precision uniforms and half precision samplers #40590

jonahwilliams · 2023-03-24T05:10:16Z

Exploits implicit conversions to use different uniform types on gles vs metal

flutter-dashboard · 2023-03-24T05:10:19Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

jonahwilliams · 2023-03-24T05:11:23Z

Requires no changes in the solid_fill shader, on metal we implicitly convert to a HalfVector4. On gles, we don't do the conversion - so it should just work.

dnfield · 2023-03-24T16:08:21Z

impeller/entity/shaders/solid_fill.frag

@@ -5,11 +5,11 @@
 #include <impeller/types.glsl>

 uniform FragInfo {
-  vec4 color;
+  f16vec4 color;


Doesn't this require #extension GL_EXT_shader_16bit_storage : require up top?

It does not, I'm not actually using a f16vec4 on glsl, here is the compiled shader:

#version 100 precision mediump float; precision highp int; struct FragInfo { vec4 color; }; uniform FragInfo frag_info; void main() { gl_FragData[0] = frag_info.color; }

oh - but this is all in types.glsl too

dnfield · 2023-03-24T16:14:22Z

impeller/geometry/scalar.h

+/// @brief Convert a scalar to a half precision float.
+///
+/// Adapted from
+/// https://developer.android.com/games/optimize/vertex-data-management .


We should document the behavior of this and how it handles precision loss, and add some tests that assert it.

Specifically:

What happens when the f is greater than the max finite value or less than the lowest finite value of a half float?

What happens when f is infinity/-infinity/nan?

What happens to very tiny values of f?

chinmaygarde · 2023-03-24T16:49:18Z

impeller/geometry/vector.h

@@ -288,6 +288,43 @@ struct Vector4 {
 static_assert(sizeof(Vector3) == 3 * sizeof(Scalar));
 static_assert(sizeof(Vector4) == 4 * sizeof(Scalar));

+struct HalfVector4 {


We could convert Vector3 and others to be TVector3 with template types for floats or half-floats. Then using Vector3 = TVector<Scalar>; and using HalfVector3 = TVector<Half>. Will significantly dry up this code.

Ahh good idea

one argument against doing this is that we don't really want to use half precision anywhere except for in the shaders.

chinmaygarde · 2023-03-24T16:54:55Z

impeller/geometry/scalar.h

@@ -13,6 +14,7 @@
 namespace impeller {

 using Scalar = float;
+using Half = uint16_t;


How is this different from the _Float16 C11 extension? I also see __fp16.

impeller/geometry/scalar.cc

jonahwilliams · 2023-03-24T20:08:42Z

impeller/geometry/vector.h

@@ -14,27 +15,46 @@

 namespace impeller {

-struct Vector3 {
+template <class T, class U>


As it exists this is a really dangerous implementation because TVector a = TVector.SomeMethod() will truncate to zero. We need to replace all of the static cast calls everywhere with Cast

jonahwilliams · 2023-03-24T21:02:06Z

I've removed the TVector specializations. In practice, you can't really do any arithmetic operations on the CPU with half types. And you can't actually use them since I'm exploiting implicit conversions to deal with metal and gles headers being different.

…vec4

jonahwilliams · 2023-03-25T00:21:44Z

I really need to test the half conversion. I think I'm going to see if there is some table somewhere I can reference for smoke testing.

jonahwilliams · 2023-03-25T00:56:28Z

Need to enable #40616 for some shaders to get float16sampler2d working.

jonahwilliams · 2023-03-25T18:07:22Z

Updated to use hardware conversion and a _Float16 type, which nicely enough has a literal too. Found in https://clang.llvm.org/docs/LanguageExtensions.html and seems to be supported on all relevant platforms

jonahwilliams · 2023-03-25T18:41:31Z

oh windows. Thankfully we only need this on apple platforms for now

jonahwilliams · 2023-03-25T20:15:13Z

From some investigation the loss of precision on the solid fill shader is visible in the wonderous app. need to dig deeper.

jonahwilliams · 2023-03-25T21:17:48Z

nvm typo

impeller/geometry/half.h

…nto support-half-vec4

gaaclarke

lgtm, modulo dan being happy too =)

gaaclarke · 2023-03-27T21:00:35Z

impeller/geometry/half.cc

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "half.h"


(this should be based from the project root too)

…nto support-half-vec4

jonahwilliams · 2023-03-28T16:26:19Z

Friendly ping:) @dnfield @chinmaygarde

chinmaygarde

Some nits and I genuinely didn't understand the member width. Its got to be right if the headers are right. But otherwise lgtm.

chinmaygarde · 2023-03-28T19:44:06Z

impeller/geometry/geometry_unittests.cc

@@ -2090,5 +2092,38 @@ TEST(GeometryTest, Gradient) {
  }
 }

+TEST(GeometryTest, HalfConversions) {
+#ifdef FML_OS_WIN
+  GTEST_SKIP() << "The reason this doesn't work on Windows";


Perhaps "Half-precision floats (IEEE 754) are not portable and unavailable on Windows."

We could write or use our own library (or a header rather) for doing this portably.

We could, one exists in an earlier commit. however we have as of today no use for it as this is only used on metal backends.

Comment fixed

chinmaygarde · 2023-03-28T19:46:45Z

impeller/geometry/half.h

+};
+
+/// @brief A storage only class for half precision floating point vector 4.
+struct HalfVector4 {


Did templating the existing Scalar type vectors not work? This is fine too but seems like we can dry this up.

I did this above and backed it out. The big caveat is that 1) We need implicit constructor conversion to make the headers work 2) we generally don't want to ever use these in the backend besides that as it implies we're dropping precision before we need to. There is no performance improvement to be gained from this. In addition, if we end up needing to emulate _Float16 support for windows we'll likely end up needing to using uint16_t as a storage type and then we'd have to define all of the operators to make that work.

Given all that, I'd really rather just leave them as storage only conversions.

chinmaygarde · 2023-03-28T19:47:43Z

impeller/geometry/half.cc

+
+namespace impeller {
+
+InternalHalf ScalarToHalf(Scalar f) {


Can this just be inline constexpr in the header?

ahh, this means I can't LOG if you try to use this on windows. Instead I'll make it always return 0 so that it is obviously wrong.

chinmaygarde · 2023-03-28T19:52:17Z

impeller/compiler/reflector.cc

@@ -767,6 +773,75 @@ std::vector<StructMember> Reflector::ReadStructMembers(
      continue;
    }

+    // Tightly packed half Point (vec2).
+    if (member.basetype == spirv_cross::SPIRType::BaseType::Half &&  //
+        member.width == sizeof(float) * 4 &&                         //


Can you double check this? The member width doesn't make sense for half base types. I may be misunderstanding.

ah, this was a hack I left in. Should be sizeof(Half) * 8

chinmaygarde · 2023-03-28T19:53:58Z

impeller/compiler/reflector.cc

+          GetArrayStride<sizeof(HalfVector2)>(struct_type, member, i);
+      uint32_t element_padding = stride - sizeof(HalfVector2);
+      result.emplace_back(StructMember{
+          "HalfVector2",                         // type


Not a function call, but with C++20, you can use aggregate initialization and do this without comments: {.type = "HalfVector2"}.

…nto support-half-vec4

…on samplers (flutter/engine#40590)

…123664) * 1d506dfd7 [Impeller] Allow metal shaders to compile through SPIR-V with openGL semantics. (flutter/engine#40616) * 3287e2b36 [Impeller] support half precision uniforms and half precision samplers (flutter/engine#40590) * 5c1c07fba Revert "Revert "Reland "Default the CanvasKit base URL to local artifacts. (#40293)" (#40470)" (#40700)" (flutter/engine#40717) * 9e4cedc81 Forward stdout and stderr from dart2wasm when verbose. (flutter/engine#40731) * 6941c1820 [Impeller] Allow toggling vulkan validation using a command line test flag. (flutter/engine#40728) * 6ef595829 Roll buildroot to build CanvasKit for speed instead of code size (flutter/engine#40737) * 5b8e02479 [Impeller] Gaussian blur: Add alpha mask specialization (flutter/engine#40707) * 6852bea71 Roll Skia from 7311e9220faf to e3eeabb14e9c (4 revisions) (flutter/engine#40745) * 235e42bea Add an option to malioc_diff.py to print a unified diff (flutter/engine#40732) * 38e6d772f Roll Dart SDK from 1acd1e649fb7 to 69867ba60bb7 (2 revisions) (flutter/engine#40738) * d81c4f2c7 [Impeller] migrate texture fill shaders to half precision. (flutter/engine#40735)

… samplers (#40590)" This reverts commit 3287e2b.

[Impeller] support f16vec4 in shaders and use in solid color uniform

02ccd58

flutter-dashboard bot added the needs tests label Mar 24, 2023

dnfield reviewed Mar 24, 2023

View reviewed changes

chinmaygarde reviewed Mar 24, 2023

View reviewed changes

jonahwilliams added 3 commits March 24, 2023 11:11

use compiler intrinsic

3c00839

++

43ff491

++

68b1009

jonahwilliams commented Mar 24, 2023

View reviewed changes

impeller/geometry/scalar.cc Outdated Show resolved Hide resolved

jonahwilliams commented Mar 24, 2023

View reviewed changes

++

b6e6f81

jonahwilliams added 3 commits March 24, 2023 14:10

Merge branch 'master' of github.com:flutter/engine into support-half-…

d739b58

…vec4

++

f1919b4

fix licenses

a98fa07

chinmaygarde added the e: impeller label Mar 25, 2023

chinmaygarde assigned jonahwilliams Mar 25, 2023

jonahwilliams added 2 commits March 25, 2023 10:44

make half a wrapper type

7dca490

use hardware supported conversion

9599b62

jonahwilliams changed the title ~~[Impeller] support f16vec4 in shaders and use in solid color uniform~~ [Impeller] support half precision uniforms and half precision samplers Mar 25, 2023

jonahwilliams commented Mar 25, 2023

View reviewed changes

impeller/geometry/half.h Outdated Show resolved Hide resolved

jonahwilliams mentioned this pull request Mar 25, 2023

[Impeller] use half precision in blending, blur, texture shaders #40632

Closed

jonahwilliams added 2 commits March 27, 2023 13:55

Merge branch 'support-half-vec4' of github.com:jonahwilliams/engine i…

c802c7c

…nto support-half-vec4

++

251564b

gaaclarke approved these changes Mar 27, 2023

View reviewed changes

jonahwilliams added 6 commits March 27, 2023 14:03

++

51d7bb7

Update geometry_unittests.cc

9001fe1

Update geometry_unittests.cc

11f0c9f

++

6998f76

Merge branch 'support-half-vec4' of github.com:jonahwilliams/engine i…

205db32

…nto support-half-vec4

make this unreachable

736a3e5

jonahwilliams requested a review from dnfield March 28, 2023 03:14

chinmaygarde approved these changes Mar 28, 2023

View reviewed changes

jonahwilliams added 5 commits March 28, 2023 13:46

++

0af6ab9

Merge branch 'main' into support-half-vec4

31cf48a

++

c9ffbc5

Merge branch 'support-half-vec4' of github.com:jonahwilliams/engine i…

b2996a8

…nto support-half-vec4

++

2f1f5c3

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 28, 2023

auto-submit bot merged commit 3287e2b into flutter:main Mar 28, 2023

engine-flutter-autoroll mentioned this pull request Mar 28, 2023

Roll Flutter Engine from 78f9c6897118 to 3287e2b362fc (2 revisions) flutter/flutter#123647

Closed

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

3287e2b36 [Impeller] support half precision uniforms and half precisi…

023445b

…on samplers (flutter/engine#40590)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 78f9c6897118 to 5b8e024794ee (7 revisions) flutter/flutter#123656

Closed

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

3287e2b36 [Impeller] support half precision uniforms and half precisi…

3bcd9d3

…on samplers (flutter/engine#40590)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 78f9c6897118 to 6852bea71eec (8 revisions) flutter/flutter#123659

Closed

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

3287e2b36 [Impeller] support half precision uniforms and half precisi…

5b3d196

…on samplers (flutter/engine#40590)

engine-flutter-autoroll mentioned this pull request Mar 29, 2023

Roll Flutter Engine from 78f9c6897118 to d81c4f2c759d (11 revisions) flutter/flutter#123664

Merged

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

3287e2b36 [Impeller] support half precision uniforms and half precisi…

697fa10

…on samplers (flutter/engine#40590)

CaseyHillers added a commit that referenced this pull request Mar 29, 2023

Revert "[Impeller] support half precision uniforms and half precision…

6c9ed0c

… samplers (#40590)" This reverts commit 3287e2b.

CaseyHillers mentioned this pull request Mar 29, 2023

Revert "[Impeller] support half precision uniforms and half precision samplers" #40766

Closed

[Impeller] support half precision uniforms and half precision samplers #40590

[Impeller] support half precision uniforms and half precision samplers #40590

Uh oh!

Conversation

jonahwilliams commented Mar 24, 2023

Uh oh!

flutter-dashboard bot commented Mar 24, 2023

Uh oh!

jonahwilliams commented Mar 24, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Mar 24, 2023

Uh oh!

jonahwilliams commented Mar 25, 2023

Uh oh!

jonahwilliams commented Mar 25, 2023

Uh oh!

jonahwilliams commented Mar 25, 2023

Uh oh!

jonahwilliams commented Mar 25, 2023

Uh oh!

jonahwilliams commented Mar 25, 2023

Uh oh!

jonahwilliams commented Mar 25, 2023

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Mar 28, 2023

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!