fix(rendering): correct PerformanceOverlay optionsMask checks and add tests by jihanurrahman33 · Pull Request #174814 · flutter/flutter

jihanurrahman33 · 2025-09-02T07:17:13Z

Fix bitmask checks in RenderPerformanceOverlay._intrinsicHeight:

Use bitwise AND (&) and compare against 0 instead of bitwise OR (|).
This ensures the intrinsic height is 0 when no options are enabled, 80.0 for one graph,
and 160.0 when both rasterizer and engine graphs are enabled.

Add tests in rendering/performance_overlay_test.dart to verify intrinsic height for
various masks.

… tests

gemini-code-assist

Code Review

This pull request correctly fixes a bug in RenderPerformanceOverlay where bitwise OR was used instead of bitwise AND for checking the optionsMask, which could lead to incorrect intrinsic height calculation. The change is sound and the addition of tests is a great improvement. I've provided a suggestion to refactor the bitmask checking logic for better readability and another to expand the test coverage to include more combinations of the performance overlay options.

gemini-code-assist · 2025-09-02T07:18:38Z

+    if ((optionsMask & (1 << PerformanceOverlayOption.displayRasterizerStatistics.index)) != 0 ||
+        (optionsMask & (1 << PerformanceOverlayOption.visualizeRasterizerStatistics.index)) != 0) {
      result += kDefaultGraphHeight;
    }
-    if ((optionsMask | (1 << PerformanceOverlayOption.displayEngineStatistics.index) > 0) ||
-        (optionsMask | (1 << PerformanceOverlayOption.visualizeEngineStatistics.index) > 0)) {
+    if ((optionsMask & (1 << PerformanceOverlayOption.displayEngineStatistics.index)) != 0 ||
+        (optionsMask & (1 << PerformanceOverlayOption.visualizeEngineStatistics.index)) != 0) {
      result += kDefaultGraphHeight;
    }


For improved readability and slightly better performance, you can combine the bitmasks for each graph type before checking against optionsMask. This avoids multiple bitwise operations within the if condition.¹

const int rasterizerBits = (1 << PerformanceOverlayOption.displayRasterizerStatistics.index) | (1 << PerformanceOverlayOption.visualizeRasterizerStatistics.index); if ((optionsMask & rasterizerBits) != 0) { result += kDefaultGraphHeight; } const int engineBits = (1 << PerformanceOverlayOption.displayEngineStatistics.index) | (1 << PerformanceOverlayOption.visualizeEngineStatistics.index); if ((optionsMask & engineBits) != 0) { result += kDefaultGraphHeight; }

Style Guide References

Footnotes

↩

jihanurrahman33 · 2025-09-02T07:36:20Z

Thanks for the suggestion — updated the implementation to combine the bitmasks into rasterizerBits and engineBits and check against optionsMask (commit eb0b722). The tests still pass. Happy to expand coverage further if you want additional combinations.

dkwingsmt

This was such a silly mistake. Thank you so much for catching it!

dkwingsmt

Fixed some analyzer issues for you

justinmc

LGTM 👍, thanks for catching this.

Looks like there is an analyzer failure, but it's just a formatting thing.

justinmc · 2025-10-13T22:52:41Z

+    final RenderPerformanceOverlay r = RenderPerformanceOverlay();
+    expect(r.computeMinIntrinsicHeight(100.0), 0.0);
+
+    // One engine stat enabled


Nit: Period at the end of these comments.

justinmc · 2025-10-14T22:26:41Z

@jihanurrahman33 Looks like the analyzer is failing.

justinmc · 2025-10-28T22:12:55Z

@jihanurrahman33 Friendly reminder on the analyzer failure.

flutter-dashboard · 2025-11-17T22:14:03Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

justinmc · 2025-11-18T23:05:27Z

@jihanurrahman33 Do you still have time to fix the analyzer failure here? I'd love to land this and it's otherwise ready to go!

Piinks · 2025-12-09T23:21:02Z

Hey @jihanurrahman33 thanks for contributing! Would you like to continue working on this change?

@jihanurrahman33

This re-stages the changes from #174814 That PR had some outstanding feedback to resolve, this is now done and we can get this change in, with thanks to @jihanurrahman33 ! From the original PR: > Fix bitmask checks in RenderPerformanceOverlay._intrinsicHeight: > > Use bitwise AND (&) and compare against 0 instead of bitwise OR (|). > This ensures the intrinsic height is 0 when no options are enabled, 80.0 for one graph, > and 160.0 when both rasterizer and engine graphs are enabled. > Add tests in rendering/performance_overlay_test.dart to verify intrinsic height for > various masks. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

@jihanurrahman33

…182309) This re-stages the changes from flutter#174814 That PR had some outstanding feedback to resolve, this is now done and we can get this change in, with thanks to @jihanurrahman33 ! From the original PR: > Fix bitmask checks in RenderPerformanceOverlay._intrinsicHeight: > > Use bitwise AND (&) and compare against 0 instead of bitwise OR (|). > This ensures the intrinsic height is 0 when no options are enabled, 80.0 for one graph, > and 160.0 when both rasterizer and engine graphs are enabled. > Add tests in rendering/performance_overlay_test.dart to verify intrinsic height for > various masks. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

@jihanurrahman33

…182309) This re-stages the changes from flutter#174814 That PR had some outstanding feedback to resolve, this is now done and we can get this change in, with thanks to @jihanurrahman33 ! From the original PR: > Fix bitmask checks in RenderPerformanceOverlay._intrinsicHeight: > > Use bitwise AND (&) and compare against 0 instead of bitwise OR (|). > This ensures the intrinsic height is 0 when no options are enabled, 80.0 for one graph, > and 160.0 when both rasterizer and engine graphs are enabled. > Add tests in rendering/performance_overlay_test.dart to verify intrinsic height for > various masks. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

fix(rendering): correct PerformanceOverlay optionsMask checks and add…

03f057d

… tests

github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 2, 2025

gemini-code-assist Bot reviewed Sep 2, 2025

View reviewed changes

refactor(rendering): combine overlay bitmask checks per review

eb0b722

justinmc requested a review from dkwingsmt September 16, 2025 22:10

dkwingsmt approved these changes Oct 1, 2025

View reviewed changes

dkwingsmt reviewed Oct 1, 2025

View reviewed changes

Comment thread packages/flutter/lib/src/rendering/performance_overlay.dart Outdated

Comment thread packages/flutter/lib/src/rendering/performance_overlay.dart Outdated

Comment thread packages/flutter/test/rendering/performance_overlay_test.dart Outdated

dkwingsmt added 2 commits September 30, 2025 22:05

Analyzer

3e3196b

Merge branch 'master' into fix/perf-overlay-mask-20250902-131551

c97fbad

dkwingsmt requested a review from justinmc October 1, 2025 05:05

justinmc approved these changes Oct 13, 2025

View reviewed changes

jihanurrahman33 closed this by deleting the head repository Jan 25, 2026

Piinks mentioned this pull request Feb 12, 2026

Correct PerformanceOverlay optionsMask checks and add tests #182309

Merged

9 tasks

Uh oh!

Conversation

jihanurrahman33 commented Sep 2, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Sep 2, 2025

Choose a reason for hiding this comment

Style Guide References

Footnotes

Uh oh!

jihanurrahman33 commented Sep 2, 2025

Uh oh!

dkwingsmt left a comment

Choose a reason for hiding this comment

Uh oh!

dkwingsmt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

justinmc commented Oct 14, 2025

Uh oh!

justinmc commented Oct 28, 2025

Uh oh!

flutter-dashboard Bot commented Nov 17, 2025

Uh oh!

justinmc commented Nov 18, 2025

Uh oh!

Piinks commented Dec 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants