Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] opt float/sampler into relaxed precision for gles #37828

Merged
merged 2 commits into from
Nov 22, 2022

Conversation

jonahwilliams
Copy link
Member

Simplified version of #37823

Part of flutter/flutter#115044

Introduce float16_t, f16vec2, et cetera types (but don't necessarily use them). Opt gles into relaxed precision.

We can't really use the float16 types, because they currently 1) are unsupported by reflector.cc 2) even if they were supported would lead to different codegen based on the target platform.

Until that is resolved, we can only use float16 types within the shader itself, which means explicit conversions are required. Because these aren't free, its less straightforward to apply.

Why can't precision mediump float compile to have on MSL? In short, because mediump/lowp are just treated as hints to the driver that compiles the shader - they can and will be discarded. As a result, there are no conversions and such. Whereas in MSL half and float are distinct types and conversions must be explict. functions like max or mix are generic and can't be invoked with a mix of half/float values.

To correctly generate MSL with half types, we need to use types such as float_16t, f16vec2 et cetera.

See also:

@@ -5,6 +5,24 @@
#ifndef TYPES_GLSL_
#define TYPES_GLSL_

#ifdef IMPELLER_TARGET_METAL
#extension GL_AMD_gpu_shader_half_float : enable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Looks like this line can be pulled out of both sides of the ifdef, simplifying to only the #ifndef IMPELLER_TARGET_METAL side?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 22, 2022
@dnfield
Copy link
Contributor

dnfield commented Nov 22, 2022

neato

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 22, 2022
@auto-submit
Copy link
Contributor

auto-submit bot commented Nov 22, 2022

auto label is removed for flutter/engine, pr: 37828, due to - The status or check suite Mac Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 22, 2022
@auto-submit auto-submit bot merged commit c05e7ff into flutter:main Nov 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 22, 2022
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 23, 2022
…115881)

* c05e7ff9b [Impeller] opt float/sampler into relaxed precision for gles (flutter/engine#37828)

* 256dc78f6 Reland "[web] Avoid returning int from js interop classes." (flutter/engine#37627)

* 5ab29c95a [web] Move unicode properties to third_party (flutter/engine#37440)

* 10da1a1f0 Remove setSampler from FragmentShader (flutter/engine#37839)

* a800e7650 [Impeller] make VerticesGeometry delegate to the DL class (flutter/engine#37835)

* 1067cd2b7 Roll Skia from 3b2d9e4bf668 to 3bd2fe46f6d2 (2 revisions) (flutter/engine#37848)

* 2d8e53925 change cloneImageElement() to return clone every time (flutter/engine#37811)

* cefb954e0 fix pixel ratio (flutter/engine#37268)

* c6b2ced1e Set nested clip nodes (flutter/engine#37850)

* c7ecca866 Roll Skia from 3bd2fe46f6d2 to c098e3c5d932 (2 revisions) (flutter/engine#37851)

* afac22d6c Made platform message responses threadsafe for macos. (flutter/engine#37607)

* a805efffb fix docs analysis error on setImageSampler (flutter/engine#37852)
@jonahwilliams jonahwilliams deleted the explict_types branch November 23, 2022 21:20
shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022
…lutter#115881)

* c05e7ff9b [Impeller] opt float/sampler into relaxed precision for gles (flutter/engine#37828)

* 256dc78f6 Reland "[web] Avoid returning int from js interop classes." (flutter/engine#37627)

* 5ab29c95a [web] Move unicode properties to third_party (flutter/engine#37440)

* 10da1a1f0 Remove setSampler from FragmentShader (flutter/engine#37839)

* a800e7650 [Impeller] make VerticesGeometry delegate to the DL class (flutter/engine#37835)

* 1067cd2b7 Roll Skia from 3b2d9e4bf668 to 3bd2fe46f6d2 (2 revisions) (flutter/engine#37848)

* 2d8e53925 change cloneImageElement() to return clone every time (flutter/engine#37811)

* cefb954e0 fix pixel ratio (flutter/engine#37268)

* c6b2ced1e Set nested clip nodes (flutter/engine#37850)

* c7ecca866 Roll Skia from 3bd2fe46f6d2 to c098e3c5d932 (2 revisions) (flutter/engine#37851)

* afac22d6c Made platform message responses threadsafe for macos. (flutter/engine#37607)

* a805efffb fix docs analysis error on setImageSampler (flutter/engine#37852)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
…lutter#115881)

* c05e7ff9b [Impeller] opt float/sampler into relaxed precision for gles (flutter/engine#37828)

* 256dc78f6 Reland "[web] Avoid returning int from js interop classes." (flutter/engine#37627)

* 5ab29c95a [web] Move unicode properties to third_party (flutter/engine#37440)

* 10da1a1f0 Remove setSampler from FragmentShader (flutter/engine#37839)

* a800e7650 [Impeller] make VerticesGeometry delegate to the DL class (flutter/engine#37835)

* 1067cd2b7 Roll Skia from 3b2d9e4bf668 to 3bd2fe46f6d2 (2 revisions) (flutter/engine#37848)

* 2d8e53925 change cloneImageElement() to return clone every time (flutter/engine#37811)

* cefb954e0 fix pixel ratio (flutter/engine#37268)

* c6b2ced1e Set nested clip nodes (flutter/engine#37850)

* c7ecca866 Roll Skia from 3bd2fe46f6d2 to c098e3c5d932 (2 revisions) (flutter/engine#37851)

* afac22d6c Made platform message responses threadsafe for macos. (flutter/engine#37607)

* a805efffb fix docs analysis error on setImageSampler (flutter/engine#37852)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
No open projects
Archived in project
4 participants