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

Conversation

@flar
Copy link
Contributor

@flar flar commented Oct 28, 2023

This is a pre-requisite step to deleting the un-named Constructors for Rect and moving to named factories. The intention is both to improve code readability and so that the underlying storage information is not implied or exposed.

This PR only modifies the unit tests which will not affect actual running apps, but migrates >95% of the concrete Rect construction to the named factory methods. Use of these constructors in the rest of the code is very rare since most methods get their Rect instances from data they compute or are delivered from elsewhere. Those other instances will be eliminated more on a file by file basis to reduce the surface area of errors arising due to typos or porting mistakes.

@flar flar requested a review from bdero October 28, 2023 21:07
@flar flar requested a review from chinmaygarde October 28, 2023 21:07
Copy link
Member

@bdero bdero left a comment

Choose a reason for hiding this comment

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

LGTM

@flar flar added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 31, 2023
@auto-submit auto-submit bot merged commit 29bc90a into flutter:main Oct 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 31, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 31, 2023
…137635)

flutter/engine@4da3e5b...39be0fc

2023-10-31 matanlurey@users.noreply.github.com [Impeller] OpenGLES MSAA Render Buffers (i.e. for stencils) (flutter/engine#47495)
2023-10-31 flar@google.com [Impeller] Migrate unit tests to named Rect factories (flutter/engine#47430)
2023-10-31 khalidcomilla58@gmail.com [Typo fixed] in DEPS (flutter/engine#47440)
2023-10-31 jonahwilliams@google.com [Impeller] Restore GLES GPU query times. (flutter/engine#47511)

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 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
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Oct 31, 2023
…lutter#137635)

flutter/engine@4da3e5b...39be0fc

2023-10-31 matanlurey@users.noreply.github.com [Impeller] OpenGLES MSAA Render Buffers (i.e. for stencils) (flutter/engine#47495)
2023-10-31 flar@google.com [Impeller] Migrate unit tests to named Rect factories (flutter/engine#47430)
2023-10-31 khalidcomilla58@gmail.com [Typo fixed] in DEPS (flutter/engine#47440)
2023-10-31 jonahwilliams@google.com [Impeller] Restore GLES GPU query times. (flutter/engine#47511)

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 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.

Labels

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

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants