Document and provide small cleanups to CommandPoolVk.
#45558
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| /// | ||
| /// @param[in] buffer The |vk::CommandBuffer| to collect. | ||
| /// | ||
| /// @note This method is a NOP if a different thread created the pool. |
There was a problem hiding this comment.
Nit: we use "noop" elsewhere, plus it's more fun to say.
|
test-exempt: documentation and code removal |
|
|
||
| #include "flutter/fml/macros.h" | ||
| #include "impeller/base/thread.h" | ||
| #include "impeller/renderer/backend/vulkan/context_vk.h" |
There was a problem hiding this comment.
The reason we generally do forward declarations is to avoid circular dependencies in header files (and sometimes to reduce compilation time but that is marginal). As long as it compiles that should be okay, but you might hit cases where you can't remove a forward reference.
There was a problem hiding this comment.
A good to know. I think these are all in the same source set but I'll have CI double check me :)
|
@chinmaygarde I'm going to merge, but if you have any post-submit comments or suggestions please leave then and I'll get to it in post! |
flutter/engine@47a7930...b2cb1d2 2023-09-08 skia-flutter-autoroll@skia.org Roll Skia from 5181fcf6184e to 948959b6b53e (4 revisions) (flutter/engine#45583) 2023-09-08 gspencergoog@users.noreply.github.com Remove usage of the Jazzy document formatter for Objective C, in favor of Doxygen. (flutter/engine#45561) 2023-09-08 skia-flutter-autoroll@skia.org Roll ANGLE from 83030fed595b to a507f31285b8 (1 revision) (flutter/engine#45582) 2023-09-08 skia-flutter-autoroll@skia.org Roll Skia from 5b1e40fc0548 to 5181fcf6184e (2 revisions) (flutter/engine#45580) 2023-09-08 matanlurey@users.noreply.github.com Document and provide small cleanups to `CommandPoolVk`. (flutter/engine#45558) 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 rmistry@google.com,zra@google.com 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
I was reviewing this code for the first time before implementing flutter/flutter#133198.
GetGraphicsCommandPool().If I made any mistakes, please feel free to point them out as this is how I'm learning how this stuff works 😄
/cc @jonahwilliams
Footnotes
Avoid using forward declarations where possible. Instead, include the headers you need. ↩