Skip to content

Commit

Permalink
Revert "Add SPIR-V FragmentShader API to painting.dart (flutter#26996)…
Browse files Browse the repository at this point in the history
…" (flutter#28202)

This reverts commit 6d6ce34.
  • Loading branch information
zanderso authored Aug 19, 2021
1 parent a116cc0 commit b482f5a
Show file tree
Hide file tree
Showing 99 changed files with 36 additions and 1,987 deletions.
3 changes: 0 additions & 3 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,6 @@ group("flutter") {
"//flutter/flow:flow_unittests",
"//flutter/fml:fml_unittests",
"//flutter/lib/spirv/test/exception_shaders:spirv_compile_exception_shaders",
"//flutter/lib/spirv/test/general_shaders:spirv_compile_general_shaders",
"//flutter/lib/spirv/test/supported_glsl_op_shaders:spirv_compile_supported_glsl_shaders",
"//flutter/lib/spirv/test/supported_op_shaders:spirv_compile_supported_op_shaders",
"//flutter/lib/ui:ui_unittests",
"//flutter/runtime:no_dart_plugin_registrant_unittests",
"//flutter/runtime:runtime_unittests",
Expand Down
4 changes: 1 addition & 3 deletions ci/analyze.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ function follow_links() (
SCRIPT_DIR=$(follow_links "$(dirname -- "${BASH_SOURCE[0]}")")
SRC_DIR="$(cd "$SCRIPT_DIR/../.."; pwd -P)"
FLUTTER_DIR="$SRC_DIR/flutter"
SKY_ENGINE_DIR="$SRC_DIR/out/host_debug_unopt/gen/dart-pkg/sky_engine"
DART_BIN="$SRC_DIR/out/host_debug_unopt/dart-sdk/bin"
DART="$DART_BIN/dart"

Expand All @@ -47,8 +46,7 @@ echo "Using dart from $DART_BIN"
"$DART" --version
echo ""

(cd $SKY_ENGINE_DIR && "$DART" pub get --offline)
"$DART" analyze "$SKY_ENGINE_DIR/lib/ui/ui.dart"
"$DART" analyze "$FLUTTER_DIR/lib/ui"

"$DART" analyze "$FLUTTER_DIR/lib/spirv"

Expand Down
2 changes: 0 additions & 2 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -358,8 +358,6 @@ FILE: ../../../flutter/lib/ui/painting/color_filter.cc
FILE: ../../../flutter/lib/ui/painting/color_filter.h
FILE: ../../../flutter/lib/ui/painting/engine_layer.cc
FILE: ../../../flutter/lib/ui/painting/engine_layer.h
FILE: ../../../flutter/lib/ui/painting/fragment_shader.cc
FILE: ../../../flutter/lib/ui/painting/fragment_shader.h
FILE: ../../../flutter/lib/ui/painting/gradient.cc
FILE: ../../../flutter/lib/ui/painting/gradient.h
FILE: ../../../flutter/lib/ui/painting/image.cc
Expand Down
2 changes: 1 addition & 1 deletion ci/licenses_golden/tool_signature
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
Signature: e892e715f8184778851c5efcb6a1323a
Signature: 2b08c736bc3af2c5d770099c53112c83

3 changes: 0 additions & 3 deletions lib/snapshot/libraries.json
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,6 @@
"uri": "../../../third_party/dart/sdk/lib/typed_data/typed_data.dart",
"patches": "../../../third_party/dart/sdk/lib/_internal/vm/lib/typed_data_patch.dart"
},
"_spirv": {
"uri": "../../lib/spirv/lib/spirv.dart"
},
"ui": {
"uri": "../../lib/ui/ui.dart"
},
Expand Down
3 changes: 0 additions & 3 deletions lib/snapshot/libraries.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,6 @@ flutter:
uri: "../../../third_party/dart/sdk/lib/typed_data/typed_data.dart"
patches: "../../../third_party/dart/sdk/lib/_internal/vm/lib/typed_data_patch.dart"

_spirv:
uri: "../../lib/spirv/lib/spirv.dart"

ui:
uri: "../../lib/ui/ui.dart"

Expand Down
35 changes: 4 additions & 31 deletions lib/spirv/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,40 +33,13 @@ Support for textures, control flow, and structured types is planned, but not cur

## Testing

### Exception Tests
## Exception Tests

These tests rely on the `.spvasm` (SPIR-V Assembly) and `.glsl` files contained under `test/exception_shaders` in this directory. They are compiled to binary SPIR-V using `spirv-asm`, from the SwiftShader dependency. They are tested by testing/dart/spirv_exception_test.dart as part of the normal suite of dart tests. The purpose of these tests is to exercise every explicit failure path for shader transpilation. Each `glsl` or `spvasm` file should include a comment describing the failure that it is testing. The given files should be valid apart from the single failure case they are testing.

To test the exception tests directly: `./testing/run_tests.py --type dart --dart-filter spirv_exception_test.dart`
## Pixel Tests

### Pixel Tests
Pixel test are not yet checked in, and should run as part of unit-testing for each implementation of `dart:ui`. These tests aim to validate the correctness of transpilation to each target language. Each shader should render the color green #00FF00 for a correct transpilation, and any other color for failure. They will be a combination of `.spvasm` files and more-readable GLSL files that are compiled to SPIR-V via `glslang`, provided by the SwiftShader dependency. Information for pixel tests will be expanded in a follow-up PR.

Pixel tests should run as part of unit-testing for each implementation of `dart:ui`. Currently, FragmentShader is only supported in C++. These tests aim to validate the correctness of transpilation to each target language. Each shader should render the color green for a correct transpilation, and any other color for failure. They will be a GLSL files that are compiled to SPIR-V via `shaderc`. Therefor, the `fragColor` should resolve to `vec4(0.0, 1.0, 0.0, 1.0)`
for all tests.
These tests will be able to be run alone by executing `./ui_unittests` in the build-output directory.

In each test, the uniform `a` is initialized with the value of 1.0.
This is important so that expressions are not simplified during GLSL to SPIR-V compilation, which may result in the removal of the op being tested.

To test the pixel tests directly: `./testing/run_tests.py --type dart --dart-filter fragment_shader_test.dart`

#### A Note on Test Isolation

Even the simplest GLSL program tests several instructions, so no test us completely isolated
to a single op. Also, some of the GLSL 450 op tests will use addition in subtraction, along with the
actual op being tested. However, the GLSL program for each test file is kept as simple as possible,
to satisfy these conditions: pass if the op works, and fail if the op does not work. In some tests,
it is sufficient to only call the GLSL op once, while other may need more calls to more completelty
test the op. Many ops support scalars, vectors, or a combination as parameters. Most tests default
to using scalars as params, but vec2, vec3, and vec4 parameters are also tested.

- vec2 is tested as a paramter in glsl_op_normalize.glsl
- vec3 is tested as a parameter in glsl_op_cross.glsl
- vec4 is tested as a parameter in glsl_op_length.glsl

### Adding New Tests

To add a new test, add a glsl (fragment shader tests) or spvasm (spirv exception tests) src file to a `lib/spirv/test/` subfolder, and add the file as a source to the corresponding `BUILD.gn`.

- New files in `exception_shaders` are automatically tested in `testing/dart/spirv_exception_test`.
- New files in `supported_op_shaders` and `supported_glsl_op_shaders` are automatically tested in `testing/dart/fragment_shader_test`.
- New files in `general_shaders` are not automatically tested and must add a new manual test case in `testing/dart/fragment_shader_test`.
20 changes: 4 additions & 16 deletions lib/spirv/lib/src/constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,6 @@ const int _decorationLocation = 30;
// Explicitly supported builtin types
const int _builtinFragCoord = 15;

// Ops that have no semantic meaning in output and can be safely ignored
const int _opSource = 3;
const int _opSourceExtension = 4;
const int _opName = 5;
const int _opMemberName = 6;
const int _opString = 7;
const int _opLine = 8;

// Supported instructions
const int _opExtInstImport = 11;
const int _opExtInst = 12;
Expand All @@ -51,21 +43,17 @@ const int _opExecutionMode = 16;
const int _opCapability = 17;
const int _opTypeVoid = 19;
const int _opTypeBool = 20;
const int _opTypeInt = 21;
const int _opTypeFloat = 22;
const int _opTypeVector = 23;
const int _opTypeMatrix = 24;
const int _opTypePointer = 32;
const int _opTypeFunction = 33;
const int _opConstantTrue = 41;
const int _opConstantFalse = 42;
const int _opConstant = 43;
const int _opConstantComposite = 44;
const int _opFunction = 54;
const int _opFunctionParameter = 55;
const int _opFunctionEnd = 56;
const int _opFunctionCall = 57;
const int _opFUnordNotEqual = 183;
const int _opVariable = 59;
const int _opLoad = 61;
const int _opStore = 62;
Expand All @@ -74,8 +62,6 @@ const int _opDecorate = 71;
const int _opVectorShuffle = 79;
const int _opCompositeConstruct = 80;
const int _opCompositeExtract = 81;
const int _opConvertFToS = 110;
const int _opConvertSToF = 111;
const int _opFNegate = 127;
const int _opFAdd = 129;
const int _opFSub = 131;
Expand All @@ -88,7 +74,6 @@ const int _opVectorTimesMatrix = 144;
const int _opMatrixTimesVector = 145;
const int _opMatrixTimesMatrix = 146;
const int _opDot = 148;
const int _opSelect = 169;
const int _opLabel = 248;
const int _opReturn = 253;
const int _opReturnValue = 254;
Expand All @@ -100,6 +85,7 @@ const int _opReturnValue = 254;
const String _glslStd450 = 'GLSL.std.450';

// Supported GLSL ops
const int _glslStd450Trunc = 3;
const int _glslStd450FAbs = 4;
const int _glslStd450FSign = 6;
const int _glslStd450Floor = 8;
Expand Down Expand Up @@ -135,6 +121,7 @@ const int _glslStd450FaceForward = 70;
const int _glslStd450Reflect = 71;

const Map<int, String> _glslStd450OpNames = <int, String>{
_glslStd450Trunc: 'trunc',
_glslStd450FAbs: 'abs',
_glslStd450FSign: 'sign',
_glslStd450Floor: 'floor',
Expand Down Expand Up @@ -171,6 +158,7 @@ const Map<int, String> _glslStd450OpNames = <int, String>{
};

const Map<int, int> _glslStd450OpArgc = <int, int>{
_glslStd450Trunc: 1,
_glslStd450FAbs: 1,
_glslStd450FSign: 1,
_glslStd450Floor: 1,
Expand All @@ -186,7 +174,7 @@ const Map<int, int> _glslStd450OpArgc = <int, int>{
_glslStd450Atan: 1,
_glslStd450Atan2: 2,
_glslStd450Pow: 2,
_glslStd450Exp: 1,
_glslStd450Exp: 2,
_glslStd450Log: 1,
_glslStd450Exp2: 1,
_glslStd450Log2: 1,
Expand Down
Loading

0 comments on commit b482f5a

Please sign in to comment.