[Impeller] when mips are disabled, also disable from sampler options. #161765

jonahwilliams · 2025-01-16T18:52:23Z

If we have disabled mipmap generation on a platform due to GPU driver bugs, make sure that all sampling options used declare that only the base mip level should be read. Otherwise we can end up sampling from unpopulated mip levels.

gaaclarke

lgtm

gaaclarke · 2025-01-16T21:01:17Z

engine/src/flutter/impeller/renderer/backend/vulkan/context_vk.cc

      std::make_unique<DriverInfoVK>(device_holder->physical_device);
  workarounds_ = GetWorkaroundsFromDriverInfo(*driver_info);
  caps->ApplyWorkarounds(workarounds_);
+  sampler_library->SetWorkarounds(workarounds_);


Should this be named ApplyWorkarounds?

gaaclarke · 2025-01-16T21:03:38Z

engine/src/flutter/impeller/renderer/backend/vulkan/sampler_library_vk.h

+  explicit SamplerLibraryVK(const std::weak_ptr<DeviceHolderVK>& device_holder);
+
+  void SetWorkarounds(const WorkaroundsVK& workarounds);


Nit: instead of having a setter you could provide workarounds in the constructor and then then could be const instance variables.

There is sort of a chicken and egg here because we create the sampler library before we do the workarounds. Its probably possible to untangle but its a huge PITA

…options. (flutter/flutter#161765)

flutter/flutter@5517cc9...b9e86a5 2025-01-18 [email protected] Fix link to Linux custom embedder artifacts (flutter/flutter#161699) 2025-01-18 [email protected] Remove tests, GLFW examples, and non-artifact builds from `linux_host_engine`. (flutter/flutter#161786) 2025-01-18 [email protected] [Impeller] when mips are disabled, also disable from sampler options. (flutter/flutter#161765) 2025-01-18 [email protected] revert removing Twitter, retain BlueSky (flutter/flutter#161803) 2025-01-17 [email protected] fix reorderable_list_test.dart (flutter/flutter#161836) 2025-01-17 [email protected] [Release] Update the cherry-pick process. (flutter/flutter#161771) 2025-01-17 [email protected] Reland "#143249 Autocomplete options width" (flutter/flutter#161695) 2025-01-17 [email protected] Roll Dart to Version 3.8.0-1.0.dev (flutter/flutter#161781) 2025-01-17 [email protected] [Impeller] use 3 fences to synchronize AHB swapchains (like we do for KHR). (flutter/flutter#161767) 2025-01-17 [email protected] [Impeller] remove Adreno denylist entries. (flutter/flutter#161740) 2025-01-17 [email protected] Refactor event redispatching (flutter/flutter#161701) 2025-01-17 [email protected] [Impellerc] correctly pad arrays of vec3s in reflector. (flutter/flutter#161697) 2025-01-17 [email protected] Initialize dartLoader.rootDirectories so the Web stack trace mapper can convert package source paths (flutter/flutter#160383) 2025-01-16 [email protected] Make fl_keyboard_manager_handle_event async (flutter/flutter#161637) 2025-01-16 [email protected] Update social links in readme (flutter/flutter#161778) 2025-01-16 [email protected] Remove some stray printf debugging (flutter/flutter#161706) 2025-01-16 [email protected] Set meta tag in default index (flutter/flutter#161493) 2025-01-16 [email protected] remove usage of `Usage` from build system (flutter/flutter#160663) 2025-01-16 [email protected] route CLI command usage information through the logger instead of using `print` (flutter/flutter#161533) 2025-01-16 [email protected] Enable duplicate `linux_host_engine_test`. (flutter/flutter#161613) 2025-01-16 [email protected] Do not block vertical drag gestures in CupertinoSheetRoute body (flutter/flutter#161696) 2025-01-16 [email protected] [Impeller] Update partial repaint to use a fullsize onscreen. (flutter/flutter#161626) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

flutteractionsbot · 2025-01-22T19:47:59Z

Failed to create CP due to merge conflicts.
You will need to create the PR manually. See the cherrypick wiki for more info.

…flutter#161765) If we have disabled mipmap generation on a platform due to GPU driver bugs, make sure that all sampling options used declare that only the base mip level should be read. Otherwise we can end up sampling from unpopulated mip levels.

…ons (#161765) (#162193) If we have disabled mipmap generation on a platform due to GPU driver bugs, make sure that all sampling options used declare that only the base mip level should be read. Otherwise we can end up sampling from unpopulated mip levels.

…options. (flutter/flutter#161765)

flutter/flutter@5517cc9...b9e86a5 2025-01-18 [email protected] Fix link to Linux custom embedder artifacts (flutter/flutter#161699) 2025-01-18 [email protected] Remove tests, GLFW examples, and non-artifact builds from `linux_host_engine`. (flutter/flutter#161786) 2025-01-18 [email protected] [Impeller] when mips are disabled, also disable from sampler options. (flutter/flutter#161765) 2025-01-18 [email protected] revert removing Twitter, retain BlueSky (flutter/flutter#161803) 2025-01-17 [email protected] fix reorderable_list_test.dart (flutter/flutter#161836) 2025-01-17 [email protected] [Release] Update the cherry-pick process. (flutter/flutter#161771) 2025-01-17 [email protected] Reland "#143249 Autocomplete options width" (flutter/flutter#161695) 2025-01-17 [email protected] Roll Dart to Version 3.8.0-1.0.dev (flutter/flutter#161781) 2025-01-17 [email protected] [Impeller] use 3 fences to synchronize AHB swapchains (like we do for KHR). (flutter/flutter#161767) 2025-01-17 [email protected] [Impeller] remove Adreno denylist entries. (flutter/flutter#161740) 2025-01-17 [email protected] Refactor event redispatching (flutter/flutter#161701) 2025-01-17 [email protected] [Impellerc] correctly pad arrays of vec3s in reflector. (flutter/flutter#161697) 2025-01-17 [email protected] Initialize dartLoader.rootDirectories so the Web stack trace mapper can convert package source paths (flutter/flutter#160383) 2025-01-16 [email protected] Make fl_keyboard_manager_handle_event async (flutter/flutter#161637) 2025-01-16 [email protected] Update social links in readme (flutter/flutter#161778) 2025-01-16 [email protected] Remove some stray printf debugging (flutter/flutter#161706) 2025-01-16 [email protected] Set meta tag in default index (flutter/flutter#161493) 2025-01-16 [email protected] remove usage of `Usage` from build system (flutter/flutter#160663) 2025-01-16 [email protected] route CLI command usage information through the logger instead of using `print` (flutter/flutter#161533) 2025-01-16 [email protected] Enable duplicate `linux_host_engine_test`. (flutter/flutter#161613) 2025-01-16 [email protected] Do not block vertical drag gestures in CupertinoSheetRoute body (flutter/flutter#161696) 2025-01-16 [email protected] [Impeller] Update partial repaint to use a fullsize onscreen. (flutter/flutter#161626) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

jonahwilliams added 2 commits January 16, 2025 10:49

[Impeller] when mips are disabled, also disable from sampler options.

334d7c0

Add back deleted code.

3515cd8

jonahwilliams requested a review from matanlurey January 16, 2025 18:52

github-actions bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Jan 16, 2025

jonahwilliams requested a review from gaaclarke January 16, 2025 20:20

gaaclarke approved these changes Jan 16, 2025

View reviewed changes

jonahwilliams added 4 commits January 16, 2025 13:43

rename to apply workarounds.

d4498cf

Update sampler_library_vk_unittests.cc

013cb3a

Merge branch 'master' into disable_mip_in_sample_properties

83f6e7b

Merge branch 'master' into disable_mip_in_sample_properties

908877a

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 17, 2025

auto-submit bot added this pull request to the merge queue Jan 18, 2025

Merged via the queue into flutter:master with commit fe257cb Jan 18, 2025
180 of 181 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 18, 2025

engine-flutter-autoroll mentioned this pull request Jan 18, 2025

Roll Flutter from 5517cc9b3b3b to b9e86a565f92 (22 revisions) flutter/packages#8457

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 18, 2025

fe257cb [Impeller] when mips are disabled, also disable from sampler …

56b2c15

…options. (flutter/flutter#161765)

engine-flutter-autoroll mentioned this pull request Jan 18, 2025

Roll Flutter from 5517cc9b3b3b to b9e86a565f92 (22 revisions) flutter/packages#8458

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 18, 2025

fe257cb [Impeller] when mips are disabled, also disable from sampler …

b5b0564

…options. (flutter/flutter#161765)

engine-flutter-autoroll mentioned this pull request Jan 19, 2025

Roll Flutter from 5517cc9b3b3b to b9e86a565f92 (22 revisions) flutter/packages#8459

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 19, 2025

fe257cb [Impeller] when mips are disabled, also disable from sampler …

9631293

…options. (flutter/flutter#161765)

jonahwilliams added the cp: beta cherry pick this pull request to beta release candidate branch label Jan 22, 2025

reidbaker mentioned this pull request Jan 30, 2025

3.29 engine bump and dart rev #162471

Merged

9 tasks

WyvernPirate mentioned this pull request Apr 3, 2025

Updates Flutter to 3.29.2 stable release WyvernPirate/flutter#1

Merged

reidbaker mentioned this pull request Apr 4, 2025

Unbreak presubmit for stable #166594

Merged

9 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

fe257cb [Impeller] when mips are disabled, also disable from sampler …

8ef3036

…options. (flutter/flutter#161765)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

fe257cb [Impeller] when mips are disabled, also disable from sampler …

2cb0c66

…options. (flutter/flutter#161765)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

fe257cb [Impeller] when mips are disabled, also disable from sampler …

96cc614

…options. (flutter/flutter#161765)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Impeller] when mips are disabled, also disable from sampler options. #161765

[Impeller] when mips are disabled, also disable from sampler options. #161765

Uh oh!

jonahwilliams commented Jan 16, 2025

Uh oh!

gaaclarke left a comment

Uh oh!

gaaclarke Jan 16, 2025

Uh oh!

jonahwilliams Jan 16, 2025

Uh oh!

gaaclarke Jan 16, 2025

Uh oh!

jonahwilliams Jan 16, 2025

Uh oh!

Uh oh!

flutteractionsbot commented Jan 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		explicit SamplerLibraryVK(const std::weak_ptr<DeviceHolderVK>& device_holder);

		void SetWorkarounds(const WorkaroundsVK& workarounds);

[Impeller] when mips are disabled, also disable from sampler options. #161765

[Impeller] when mips are disabled, also disable from sampler options. #161765

Uh oh!

Conversation

jonahwilliams commented Jan 16, 2025

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

jonahwilliams Jan 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flutteractionsbot commented Jan 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants