Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Fix: Complete Documentation for RasterStatus::kSkipAndRetry #44880

Merged
merged 6 commits into from
Aug 25, 2023

Conversation

yatendra2001
Copy link
Contributor

Description

This PR completes the documentation for RasterStatus::kSkipAndRetry in the compositor_context.h file. The previous comment ended abruptly, leaving ambiguity regarding the behavior when the thread merger is disabled.

Changes:

  • Added clarification on the behavior when the thread merger is disabled.
  • Explained the potential performance implications when threads are not merged.

Related Issues

flutter/flutter#131814

Tests

No tests were added as this PR only involves documentation changes.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@dkwingsmt
Copy link
Contributor

Hi, can you fix the linting errors as shown in the checks, so that this PR can land?

@yatendra2001
Copy link
Contributor Author

Hi, can you fix the linting errors as shown in the checks, so that this PR can land?

yeah sure

@yatendra2001
Copy link
Contributor Author

Hi, can you fix the linting errors as shown in the checks, so that this PR can land?

hey @dkwingsmt, I can't understand where's the linting error, If you check my file changes everything seems fine.

@@ -40,9 +40,11 @@ enum class RasterStatus {
// This is currently used to wait for the thread merger to merge
// the raster and platform threads.
//
// Since the thread merger may be disabled,
// Since the thread merger may be disabled, the system will proceed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Since the thread merger may be disabled, the system will proceed
// Since the thread merger may be disabled, the system will proceed

@@ -40,9 +40,11 @@ enum class RasterStatus {
// This is currently used to wait for the thread merger to merge
// the raster and platform threads.
//
// Since the thread merger may be disabled,
// Since the thread merger may be disabled, the system will proceed
// with separate threads for rasterization and platform tasks,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// with separate threads for rasterization and platform tasks,
// with separate threads for rasterization and platform tasks,

@dkwingsmt
Copy link
Contributor

Two trailing spaces. (I'd suggest checking out if your code editor has an option to automatically remove trailing white spaces on saving.)

@yatendra2001
Copy link
Contributor Author

Two trailing spaces. (I'd suggest checking out if your code editor has an option to automatically remove trailing white spaces on saving.)

omfg, that's such a silly error. sure i use vscode, i'll rn enable this functionality. thanks for the help

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 25, 2023
@auto-submit auto-submit bot merged commit faaa2ff into flutter:main Aug 25, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 25, 2023
…133355)

flutter/engine@1471967...1ec7b89

2023-08-25 zanderso@users.noreply.github.com Remove --enable-software-rendering from iOS scenario tests (flutter/engine#45093)
2023-08-25 skia-flutter-autoroll@skia.org Roll Skia from 56bb647a49ac to 76672468e8d7 (3 revisions) (flutter/engine#45121)
2023-08-25 yatendraroria2001@gmail.com Fix: Complete Documentation for RasterStatus::kSkipAndRetry (flutter/engine#44880)
2023-08-25 30870216+gaaclarke@users.noreply.github.com [Impeller]  Updated TextureSourceVK docs and deleted unused ivars (flutter/engine#45123)
2023-08-25 zanderso@users.noreply.github.com Revert "Fix global tests doing nothing." (flutter/engine#45125)
2023-08-25 skia-flutter-autoroll@skia.org Roll Skia from ba7c5258d2b4 to 56bb647a49ac (3 revisions) (flutter/engine#45118)

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 jimgraham@google.com,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
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…44880)

## Description

This PR completes the documentation for `RasterStatus::kSkipAndRetry` in the `compositor_context.h` file. The previous comment ended abruptly, leaving ambiguity regarding the behavior when the thread merger is disabled.

### Changes:
- Added clarification on the behavior when the thread merger is disabled.
- Explained the potential performance implications when threads are not merged.

## Related Issues

flutter/flutter#131814

## Tests

No tests were added as this PR only involves documentation changes.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants