-
Notifications
You must be signed in to change notification settings - Fork 6k
Added class docstrings for classes inside of shell/common. #9303
Conversation
shell/common/animator.h
Outdated
| /// Executor of animations on a |LayerTree|. | ||
| /// | ||
| /// This typically happens after the Layout and Paint passes in the Dart code | ||
| /// and before the LayerTree is Rasterized. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The functions of this component are completely divorced from the inner working of user and framework code in Dart. Also, stuff like RequestFrame happens before any of those phases in Dart code. So the final statement is inaccurate.
Something like "In conjunction with the system vsync waiter, allows callers (typically Dart code) to schedule work that ends up generating a layer tree. In cases where callers (Dart Code) end up specifying too much work to the layer tree consumer (the renderer), a backoff mechanism is implemented (with reschduled workloads still lining up with system vsync intervals)" is more accurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Animator::Render is called on the UI thread after flushLayout and flushPaint as a result of the compositeFrame method. I took out those details because i think class docstrings should talk about what the class is, not what it does.
shell/common/pipeline.h
Outdated
|
|
||
| size_t GetNextPipelineTraceID(); | ||
|
|
||
| /// A thread-safe queue of resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"A single producer, single consumer resource queue which allows for book-keeping of frame workloads across multiple threads."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
shell/common/vsync_waiter_fallback.h
Outdated
|
|
||
| namespace flutter { | ||
|
|
||
| /// A fake |VsyncWaiter| that will fire at 60 fps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"... using a timer that has nothing to do system vsync". This is used by default on platforms that don't provide a system vsync waiter or when the system vsync waiter is temporarily disengaged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
shell/common/engine.h
Outdated
|
|
||
| namespace flutter { | ||
|
|
||
| /// Runner of the Dart code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Manages the root isolate of a Flutter application. Is responsible for setting it up the isolate, setting up dart::ui binding, relaying platform messages, and ensuring cold-restarts are managed in development.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
shell/common/persistent_cache.h
Outdated
|
|
||
| namespace flutter { | ||
|
|
||
| /// A cache of shaders that gets stored to disk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't only have to be shaders. Also, all stores are atomic and serviced on an optional worker thread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The files saved to disk are named "shader_dump_x.skp"
engine/shell/common/persistent_cache.cc
Line 172 in 7bb5b9a
| name_stream << "shader_dump_" << std::to_string(ticks) << ".skp"; |
Also store always sets "stored_new_shaders_ = true;"
If it stores more than shaders there is other code in here that is out of date.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't only have to be shaders.
Its just shaders for now. But the cache directly is also written to by Dart. If you think this mentioning shaders is clearer, then go for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Added Shaders in details and out of brief.
chinmaygarde
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. Incorrectly approved.
shell/common/persistent_cache.h
Outdated
|
|
||
| namespace flutter { | ||
|
|
||
| /// A cache of shaders that gets stored to disk. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't only have to be shaders.
Its just shaders for now. But the cache directly is also written to by Dart. If you think this mentioning shaders is clearer, then go for it.
flutter/engine@f1d821d...6f5347c git log f1d821d..6f5347c --no-merges --oneline 6f5347c MessageLoopTaskQueue schedules Wakes (flutter/engine#9316) b9c790e Roll src/third_party/skia 1127c0b273ea..4c4945a25248 (9 commits) (flutter/engine#9323) 6aaf5b3 Build the GLFW shell on Linux host builds but not target builds (flutter/engine#9320) 1121fda Added class docstrings for classes inside of shell/common. (flutter/engine#9303) 96a592b [macos] Adds clipboard string read/write support to macOS (flutter/engine#9313) 1cf980f Roll src/third_party/skia 569f12f0e503..1127c0b273ea (13 commits) (flutter/engine#9317) 14b0414 Update the Dart version to 1d8b81283c1dee38f1dd87b71b16aa1648b01155 (flutter/engine#9318) 54ff3c5 Avoid using std::unary_function (flutter/engine#9314) 031c2dc Only build embedder unit tests for host builds (flutter/engine#9315) 219c1cd Roll src/third_party/skia 084fa1b52f30..569f12f0e503 (3 commits) (flutter/engine#9311) 1caff8d Decorate UIApplicationDelegate wrappers with matching UIKit deprecation (flutter/engine#9304) 0df44e9 [scene_host] Expose Opacity and remove ExportNode (flutter/engine#9297) fe7e444 Refactor: move Task Queue to its own class (flutter/engine#9301) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.
In response to: flutter/flutter#33358