[Impeller] Added cache for command buffers in vulkan #42793
Conversation
| namespace impeller { | ||
|
|
||
| template <typename CommandBuffer> | ||
| class CommandBufferCache { |
There was a problem hiding this comment.
I would call this PassBindingsCache like the Metal backend. This class doesn't seem to cache command buffers at all.
There was a problem hiding this comment.
Done.
This class doesn't seem to cache command buffers at all.
"Pass bindings" isn't really accurate either since it is caching anything that is added to the command buffer, not just bindings. I agree, lets keep it consistent though.
|
|
||
| namespace impeller { | ||
|
|
||
| template <typename CommandBuffer> |
There was a problem hiding this comment.
The only reason this is templated is to serve the mock. This is unfortunate as it affects our ability to OOL this if needed and makes the code harder to read. Barring another way to test it that doesn't use templates, I'd rather leave the test out and make this simpler (and OOLed). If we mess up our bindings, I'm sure higher level tests will catch this.
gaaclarke
force-pushed
the
command_buffer_cache
branch
from
58b39b8 to
5117a6d
Compare
gaaclarke
force-pushed
the
command_buffer_cache
branch
from
5117a6d to
51c8625
Compare
chinmaygarde
left a comment
chinmaygarde
left a comment
There was a problem hiding this comment.
Minor nit about the case of method names. But otherwise LGTM.
|
|
||
| class PassBindingsCache { | ||
| public: | ||
| void bindPipeline(vk::CommandBuffer command_buffer, |
There was a problem hiding this comment.
I only did it so that they match verbatim the functions they are caching for (ie vk::CommandBuffer::bindPipeline).
There was a problem hiding this comment.
I like being consistent in newly authored code. And besides, the stuff in libc++ is underscore_case. So having different conventions for code and calls in different components is a thing already.
In any case, I don't have a strong opinion about this. But if you are ambivalent about it, I'd rather go with sticking to the style guide.
gaaclarke
added
the
autosubmit
…128841) flutter/engine@66a5761...727b441 2023-06-13 [email protected] Reland "[ios_platform_view] only recycle maskView when the view is applying mutators #42115" (flutter/engine#42823) 2023-06-13 [email protected] Roll Dart SDK from 41234fa4b22d to 2465228c0c21 (1 revision) (flutter/engine#42822) 2023-06-13 [email protected] [Impeller] Added cache for command buffers in vulkan (flutter/engine#42793) 2023-06-13 [email protected] setupDefultFontManager correctly clear out cache (flutter/engine#42178) 2023-06-13 [email protected] [Impeller] Reland attempt Vulkan setup and fallback to GLES. (flutter/engine#42820) 2023-06-13 [email protected] Added CI step for building with validation layers (flutter/engine#42724) 2023-06-13 [email protected] [Impeller] Null check for the device holder in the Vulkan context destructor (flutter/engine#42821) 2023-06-13 [email protected] Add missing includes (flutter/engine#42812) 2023-06-13 [email protected] Roll Dart SDK from 4dce1093ad94 to 41234fa4b22d (1 revision) (flutter/engine#42810) 2023-06-13 [email protected] [iOS][Keyboard] Wait vsync on UI thread and update viewport inset to avoid jitter. (flutter/engine#42312) 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
2 participants
issue flutter/flutter#118727
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.