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
062eaad [Impeller] Cleanups to geometry interfaces. (flutter/engine#4…
3283e50 
…8180)
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Nov 17, 2023
@engine-flutter-autoroll
Roll Flutter Engine from 90c3ada to 141a01c (16 revisions) (#138637)
30b6586 
flutter/engine@90c3ada...141a01c

2023-11-17 dnfield@google.com Reland "[Impeller] Fail if software backend is chosen and Impeller is enabled on iOS." (flutter/engine#46275)
2023-11-17 matanlurey@users.noreply.github.com Make `impeller/geometry/...` compatible with `.clang-tidy`. (flutter/engine#48154)
2023-11-17 matanlurey@users.noreply.github.com Make `impeller/{archivist|compiler|core|entity}/...` compatible with � (flutter/engine#48153)
2023-11-17 109111084+yaakovschectman@users.noreply.github.com Assign mojom `kSwitch` role to switches (flutter/engine#48146)
2023-11-17 mdebbar@google.com [web] Move scene DOM management to DomManager (flutter/engine#47460)
2023-11-17 30870216+gaaclarke@users.noreply.github.com [Impeller] Unify around "transform" (flutter/engine#48184)
2023-11-17 skia-flutter-autoroll@skia.org Roll Dart SDK from a9c212f2f54b to 03cddb5d740d (1 revision) (flutter/engine#48182)
2023-11-17 matanlurey@users.noreply.github.com Actually make `status_or.h` compatible with `.clang-tidy`. (flutter/engine#48151)
2023-11-17 jonahwilliams@google.com [Impeller] Cleanups to geometry interfaces. (flutter/engine#48180)
2023-11-17 mdebbar@google.com [web] Move all DOM creation to DomManager (flutter/engine#48123)
2023-11-17 15619084+vashworth@users.noreply.github.com Reenable UnobstructedPlatformViewTests testMultiplePlatformViewsWithOverlays (flutter/engine#48139)
2023-11-17 skia-flutter-autoroll@skia.org Roll Dart SDK from 46e8b18047eb to a9c212f2f54b (1 revision) (flutter/engine#48176)
2023-11-17 skia-flutter-autoroll@skia.org Roll Skia from bcd22e8f95bc to 8e9e168418a0 (1 revision) (flutter/engine#48173)
2023-11-17 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from M0zM3CJLIrd5lb0u0... to Bcq9TZdt-vtTSL5YH... (flutter/engine#48172)
2023-11-17 skia-flutter-autoroll@skia.org Roll Skia from c8ee25282849 to bcd22e8f95bc (1 revision) (flutter/engine#48170)
2023-11-17 bdero@google.com [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 jonahwilliams@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
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