Skip to content

Move all getUniformX tests to web_ui/test.#180910

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
walley892:web-shader-tests
Jan 13, 2026
Merged

Move all getUniformX tests to web_ui/test.#180910
auto-submit[bot] merged 3 commits into
flutter:masterfrom
walley892:web-shader-tests

Conversation

@walley892

@walley892 walley892 commented Jan 13, 2026

Copy link
Copy Markdown
Contributor
  • Moves all the existing web tests for getUniformX to web_ui/tests.

  • Commits shaders to github in addition to the impellerc generated string. Had to manually translate them from the dumped SKSL string to GLSL and recompile, there may be small differences.

  • Updates the impellerc generated strings

Does not fix the web testing build system to have this happen in a non-brittle way. That should happen at some point.

@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically e: impeller Impeller rendering backend issues and features requests labels Jan 13, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the testing for shader uniforms by moving existing web tests for getUniformX to web_ui/tests. It also improves maintainability by committing the raw shader source files (.frag) to the repository and updating the generated string representations in the Dart test files. My feedback includes a suggestion to update a comment to ensure all dependent test files are listed for a shader, and a refactoring suggestion for the new tests to group related tests and reduce code duplication.

// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// If updating this file, also update engine/src/flutter/lib/web_ui/test/canvaskit/fragment_program_test.dart

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This comment is helpful, but it's missing a reference to engine/src/flutter/lib/web_ui/test/ui/fragment_shader_test.dart, which also uses the generated output from this shader. To ensure all dependent files are updated when this shader changes, it's best to list all of them, similar to how it's done in many_arrays.frag.

// If updating this file, also update engine/src/flutter/lib/web_ui/test/canvaskit/fragment_program_test.dart
// and engine/src/flutter/lib/web_ui/test/ui/fragment_shader_test.dart

Comment thread engine/src/flutter/lib/web_ui/test/ui/fragment_shader_test.dart

@gaaclarke gaaclarke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, this is an improvement. It's nice having the version numbers in there to help people if the checked in compiled shaders get stale.

@harryterkelsen harryterkelsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@walley892 walley892 added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2026
@auto-submit

auto-submit Bot commented Jan 13, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/180910, because - The status or check suite Linux web_skwasm_tests_1 has failed. Please fix the issues identified (or deflake) before re-applying this label.

@walley892 walley892 added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Jan 13, 2026
Merged via the queue into flutter:master with commit 264816f Jan 13, 2026
183 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 13, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 14, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 16, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 16, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 16, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 16, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 16, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 17, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 17, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 17, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 20, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 20, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 20, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 20, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 20, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 21, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 21, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 21, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 22, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 22, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 22, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels. platform-web Web applications specifically

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants