This repository was archived by the owner on Feb 25, 2025. It is now read-only.
[Impeller] Cache entire render target and not just allocations. #50990
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonahwilliams
changed the title
jonahwilliams
commented
Feb 27, 2024
|
|
||
| private: | ||
| std::shared_ptr<Allocator> allocator_; | ||
| struct RenderTargetConfig { |
Contributor
Author
There was a problem hiding this comment.
This is a subset of the render target properties that we care about for caching (at the entity level).
Contributor
Author
There was a problem hiding this comment.
Its defined here for ... reasons, but I could move this up to the entity cache so its more obvious
dnfield
approved these changes
Feb 27, 2024
Contributor
dnfield
left a comment
dnfield
left a comment
There was a problem hiding this comment.
Was there a DISABLED test you can re-enable now?
jonahwilliams
commented
Feb 27, 2024
| namespace testing { | ||
|
|
||
| // | ||
| using RendererTest = PlaygroundTest; |
Contributor
Author
There was a problem hiding this comment.
@dnfield See here. For some reasonable using DISABLE_ caused a problem with the test harness so I just commented it out.
jonahwilliams
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Feb 29, 2024
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Feb 29, 2024
…144355) flutter/engine@10331db...61510db 2024-02-28 [email protected] Roll Skia from 27171d6a9205 to f0a60bfe98e0 (4 revisions) (flutter/engine#51068) 2024-02-28 [email protected] Prefix flutter in flutter_vma.h import (flutter/engine#51065) 2024-02-28 [email protected] [Impeller] Cache entire render target and not just allocations. (flutter/engine#50990) 2024-02-28 [email protected] Test the `SurfaceTextureSurfaceProducer`-branch in the Android `scenario_app` (flutter/engine#51061) 2024-02-28 [email protected] Revert "[skwasm] Clip pictures if they go beyond the bounds of the window. (#50887)" (flutter/engine#51067) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This may be required to fix flutter/flutter#144116 , and in general the safety of the render pass caches.
Because the RenderPass / Framebuffer objects refer to specific attachments, the caches are placed on the resolve texture, which is "real". The stencil/depth and or MSAA textures may be different but that didn't seem to matter on many Android devices. It doesn't seem like its actually valid though, whether or not there is a validation error to catch it.
I can confirm this fixes flutter/flutter#144116 locally.
This change moves the cache one level up to cover the properties of the RenderTarget we care about, and ensures that we always use the same attachment combinations.
Fixes flutter/flutter#141750
Maybe flutter/flutter#144255 ?