Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] Make storage sizes typed. #53700

Merged
merged 5 commits into from
Jul 3, 2024

Conversation

chinmaygarde
Copy link
Member

@chinmaygarde chinmaygarde commented Jul 2, 2024

This is similar to our handling of degrees and radians or really any of the stuff in chrono.

Storage is always in bytes. Before being displayed, it can be converted into any of unit that makes the most sense. Conversions are non-truncating and everything is constexpr safe.

This is similar to our handling of scalars and radians or really any of
the stuff in chrono.

Storage is always in bytes. Before being displayed, it can be converted
into any of unit that makes the most sense. Conversions are
non-truncating and everything is constexpr safe.
@chinmaygarde
Copy link
Member Author

MemoryBudgetUsageMB is probably incorrectly named and should ideally be renamed to MemoryBudgetUsageMiB for accuracy.

Copy link
Member

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

Neato


TEST(AllocationSizeTest, CanCreateTypedAllocationsWithLiterals) {
using namespace allocation_size_literals;
ASSERT_EQ((1024_bytes).GetByteSize(), 1024u);
Copy link
Member

Choose a reason for hiding this comment

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

oooo!


using KibiBytes = AllocationSize<1'024u>;
using MebiBytes = AllocationSize<1'024u * 1'024u>;
using GigiBytes = AllocationSize<1'024u * 1'024u * 1'024u>;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not gigibytes, but gibibytes :) You're probably going to need to change it across the PR, since the typo came up in a few places.

Copy link
Member Author

Choose a reason for hiding this comment

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

derp. Good catch. Fixed everywhere.

@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 3, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 3, 2024
Copy link
Contributor

auto-submit bot commented Jul 3, 2024

auto label is removed for flutter/engine/53700, due to - The status or check suite Linux linux_host_desktop_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

  • The status or check suite Linux linux_arm_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 3, 2024
@auto-submit auto-submit bot merged commit 1af5cef into flutter:main Jul 3, 2024
31 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 3, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 4, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 4, 2024
…151293)

flutter/engine@4190543...8e2d05f

2024-07-04 flar@google.com [Impeller] Re-enable fast blur path for elliptical rrects (flutter/engine#53704)
2024-07-03 skia-flutter-autoroll@skia.org Roll Skia from 9fd1dc779589 to d5f8dde714e4 (2 revisions) (flutter/engine#53721)
2024-07-03 yjbanov@google.com [web] ignore pointer events on plain text spans (flutter/engine#53694)
2024-07-03 mdebbar@google.com Add Semantics Property `linkUrl` (flutter/engine#53507)
2024-07-03 chinmaygarde@google.com [Embedder] Document incorrectly named field in FlutterOpenGLFramebuffer. (flutter/engine#53720)
2024-07-03 chinmaygarde@google.com [Impeller] Make storage sizes typed. (flutter/engine#53700)
2024-07-03 matanlurey@users.noreply.github.com Convert `run_ios_tests.sh` to `run_ios_tests.dart`. (flutter/engine#53645)
2024-07-03 matanlurey@users.noreply.github.com Move `//third_party/android_embedding_dependencies` to `//flutter/third_party`. (flutter/engine#53587)
2024-07-03 chinmaygarde@google.com [Impeller] Document how to debug/profile OpenGL ES on macOS. (flutter/engine#53671)
2024-07-03 kingtous@qq.com [Flutter Web(HTML)] fix: shader mask is painted incorrectly on shared offscreen canvas (flutter/engine#44998)
2024-07-03 kingtous@qq.com fix: mask disappeared when having nested mask filter on Flutter web HTML (flutter/engine#45166)
2024-07-03 skia-flutter-autoroll@skia.org Roll Skia from 86ee8cc61508 to 9fd1dc779589 (3 revisions) (flutter/engine#53715)
2024-07-03 skia-flutter-autoroll@skia.org Roll Skia from c14cce59222b to 86ee8cc61508 (1 revision) (flutter/engine#53713)

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 aaclarke@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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TahaTesser pushed a commit to TahaTesser/flutter that referenced this pull request Jul 8, 2024
…lutter#151293)

flutter/engine@4190543...8e2d05f

2024-07-04 flar@google.com [Impeller] Re-enable fast blur path for elliptical rrects (flutter/engine#53704)
2024-07-03 skia-flutter-autoroll@skia.org Roll Skia from 9fd1dc779589 to d5f8dde714e4 (2 revisions) (flutter/engine#53721)
2024-07-03 yjbanov@google.com [web] ignore pointer events on plain text spans (flutter/engine#53694)
2024-07-03 mdebbar@google.com Add Semantics Property `linkUrl` (flutter/engine#53507)
2024-07-03 chinmaygarde@google.com [Embedder] Document incorrectly named field in FlutterOpenGLFramebuffer. (flutter/engine#53720)
2024-07-03 chinmaygarde@google.com [Impeller] Make storage sizes typed. (flutter/engine#53700)
2024-07-03 matanlurey@users.noreply.github.com Convert `run_ios_tests.sh` to `run_ios_tests.dart`. (flutter/engine#53645)
2024-07-03 matanlurey@users.noreply.github.com Move `//third_party/android_embedding_dependencies` to `//flutter/third_party`. (flutter/engine#53587)
2024-07-03 chinmaygarde@google.com [Impeller] Document how to debug/profile OpenGL ES on macOS. (flutter/engine#53671)
2024-07-03 kingtous@qq.com [Flutter Web(HTML)] fix: shader mask is painted incorrectly on shared offscreen canvas (flutter/engine#44998)
2024-07-03 kingtous@qq.com fix: mask disappeared when having nested mask filter on Flutter web HTML (flutter/engine#45166)
2024-07-03 skia-flutter-autoroll@skia.org Roll Skia from 86ee8cc61508 to 9fd1dc779589 (3 revisions) (flutter/engine#53715)
2024-07-03 skia-flutter-autoroll@skia.org Roll Skia from c14cce59222b to 86ee8cc61508 (1 revision) (flutter/engine#53713)

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 aaclarke@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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jul 8, 2024
…lutter#151293)

flutter/engine@4190543...8e2d05f

2024-07-04 flar@google.com [Impeller] Re-enable fast blur path for elliptical rrects (flutter/engine#53704)
2024-07-03 skia-flutter-autoroll@skia.org Roll Skia from 9fd1dc779589 to d5f8dde714e4 (2 revisions) (flutter/engine#53721)
2024-07-03 yjbanov@google.com [web] ignore pointer events on plain text spans (flutter/engine#53694)
2024-07-03 mdebbar@google.com Add Semantics Property `linkUrl` (flutter/engine#53507)
2024-07-03 chinmaygarde@google.com [Embedder] Document incorrectly named field in FlutterOpenGLFramebuffer. (flutter/engine#53720)
2024-07-03 chinmaygarde@google.com [Impeller] Make storage sizes typed. (flutter/engine#53700)
2024-07-03 matanlurey@users.noreply.github.com Convert `run_ios_tests.sh` to `run_ios_tests.dart`. (flutter/engine#53645)
2024-07-03 matanlurey@users.noreply.github.com Move `//third_party/android_embedding_dependencies` to `//flutter/third_party`. (flutter/engine#53587)
2024-07-03 chinmaygarde@google.com [Impeller] Document how to debug/profile OpenGL ES on macOS. (flutter/engine#53671)
2024-07-03 kingtous@qq.com [Flutter Web(HTML)] fix: shader mask is painted incorrectly on shared offscreen canvas (flutter/engine#44998)
2024-07-03 kingtous@qq.com fix: mask disappeared when having nested mask filter on Flutter web HTML (flutter/engine#45166)
2024-07-03 skia-flutter-autoroll@skia.org Roll Skia from 86ee8cc61508 to 9fd1dc779589 (3 revisions) (flutter/engine#53715)
2024-07-03 skia-flutter-autoroll@skia.org Roll Skia from c14cce59222b to 86ee8cc61508 (1 revision) (flutter/engine#53713)

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 aaclarke@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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@chinmaygarde chinmaygarde deleted the measurements branch August 22, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants