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

[Impeller] Cleanups to geometry interfaces. #48180

Merged
auto-submit merged 1 commit into from
Nov 17, 2023
Merged

[Impeller] Cleanups to geometry interfaces. #48180

auto-submit merged 1 commit into from
Nov 17, 2023

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Nov 17, 2023 •
edited
Loading

  • Removed virtual destructor. We only use Geometry types with std::shared_ptr which remembers the correct dtor, we'd only need the virtual destructor if we were using free ourselves.
  • Remove std:unique_ptr. We had a mix on unique and shared ptrs, and were even doing some copies/conversions between the two. I made it consistently shared_ptr, I don't see the advantage of unique given that geometries are essentially immutable.
  • made geometry classes have more const methods and final.
  • Added some asserts on trivial destruction.

@jonahwilliams jonahwilliams marked this pull request as ready for review November 17, 2023 16:28
matanlurey
matanlurey approved these changes Nov 17, 2023
View reviewed changes
Copy link
Contributor

@matanlurey matanlurey left a comment

Choose a reason for hiding this comment

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

Makes sense, consistency is good.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 17, 2023
@auto-submit auto-submit bot merged commit 062eaad into flutter:main Nov 17, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 17, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 17, 2023
@engine-flutter-autoroll
062eaad0d [Impeller] Cleanups to geometry interfaces. (flutter/engine…
3283e50 
…#48180)
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Nov 17, 2023
@engine-flutter-autoroll
Roll Flutter Engine from 90c3ada3682c to 141a01c5c70b (16 revisions) (#…
30b6586 
…138637)

flutter/engine@90c3ada...141a01c

2023-11-17 [email protected] Reland "[Impeller] Fail if software backend is chosen and Impeller is enabled on iOS." (flutter/engine#46275)
2023-11-17 [email protected] Make `impeller/geometry/...` compatible with `.clang-tidy`. (flutter/engine#48154)
2023-11-17 [email protected] Make `impeller/{archivist|compiler|core|entity}/...` compatible with â�¦ (flutter/engine#48153)
2023-11-17 [email protected] Assign mojom `kSwitch` role to switches (flutter/engine#48146)
2023-11-17 [email protected] [web] Move scene DOM management to DomManager (flutter/engine#47460)
2023-11-17 [email protected] [Impeller] Unify around "transform" (flutter/engine#48184)
2023-11-17 [email protected] Roll Dart SDK from a9c212f2f54b to 03cddb5d740d (1 revision) (flutter/engine#48182)
2023-11-17 [email protected] Actually make `status_or.h` compatible with `.clang-tidy`. (flutter/engine#48151)
2023-11-17 [email protected] [Impeller] Cleanups to geometry interfaces. (flutter/engine#48180)
2023-11-17 [email protected] [web] Move all DOM creation to DomManager (flutter/engine#48123)
2023-11-17 [email protected] Reenable UnobstructedPlatformViewTests testMultiplePlatformViewsWithOverlays (flutter/engine#48139)
2023-11-17 [email protected] Roll Dart SDK from 46e8b18047eb to a9c212f2f54b (1 revision) (flutter/engine#48176)
2023-11-17 [email protected] Roll Skia from bcd22e8f95bc to 8e9e168418a0 (1 revision) (flutter/engine#48173)
2023-11-17 [email protected] Roll Fuchsia Linux SDK from M0zM3CJLIrd5lb0u0... to Bcq9TZdt-vtTSL5YH... (flutter/engine#48172)
2023-11-17 [email protected] Roll Skia from c8ee25282849 to bcd22e8f95bc (1 revision) (flutter/engine#48170)
2023-11-17 [email protected] [Flutter GPU] Add Textures. (flutter/engine#48118)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from M0zM3CJLIrd5 to Bcq9TZdt-vtT

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 [email protected],[email protected],[email protected] 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@matanlurey matanlurey matanlurey approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App e: impeller

Projects

No open projects
Impeller
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jonahwilliams @matanlurey