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

Extract TestGLContext to separate translation unit #56647

Merged
merged 1 commit into from
Nov 18, 2024
Merged

Extract TestGLContext to separate translation unit #56647

merged 1 commit into from
Nov 18, 2024

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Nov 16, 2024 •
edited
Loading

For consistency with the Test.*Context classes for other backends, which live in their own implementation file with their own header, extract TestEGLContext to its own header and TU so that in cases where only a TestEGLContext is required (e.g. EmbedderTestBackingStoreProducerGL), we don't need to include all the various test GL surface classes as well.

GetEGLError is used by both TestEGLContext and the TestGLSurface classes, so moves to its own utils file.

No tests because this is a refactoring with no semantic changes, and the code itself is test code.

Issue: flutter/flutter#158998

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@cbracken
Extract TestGLContext to separate translation unit
78c7df9 
For consistency with the Test.*Context classes for other backends, which
live in their own implementation file with their own header, extract
TestEGLContext to its own header and TU so that in cases where only a
TestEGLContext is required (e.g. EmbedderTestBackingStoreProducerGL), we
don't need to include all the various test GL surface classes as well.

Issue: flutter/flutter#158998
@cbracken
Copy link
Member Author

cbracken commented Nov 16, 2024 •
edited
Loading

Sent flutter/cocoon#4033 to exempt the testing dir from tests in Cocoon, which has now landed.

@flutter flutter deleted a comment from flutter-dashboard bot Nov 16, 2024
@stuartmorgan-g
Copy link
Contributor

test-exempt: code refactor with no semantic change

@cbracken cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 18, 2024
@auto-submit auto-submit bot merged commit 7f2d56b into flutter:main Nov 18, 2024
31 checks passed
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 18, 2024
@cbracken cbracken deleted the extract-gl-context branch November 18, 2024 23:22
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 18, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 18, 2024
@engine-flutter-autoroll
7f2d56b8b Extract TestGLContext to separate translation unit (flutter…
a37d382 
…/engine#56647)
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Nov 19, 2024
@engine-flutter-autoroll
Roll Flutter Engine from 878f593802e1 to 7643ef10c25f (10 revisions) (#…
f2d8069 
…159111)

flutter/engine@878f593...7643ef1

2024-11-18 chris@bracken.jp iOS: Eliminate unguarded-availability
opt-out (flutter/engine#56689)
2024-11-18 jacksongardner@google.com [skwasm] Use
`displayWidth`/`displayHeight` instead of `codedWidth`/`codedHeight`
(flutter/engine#56686)
2024-11-18 chris@bracken.jp Extract TestGLContext to separate
translation unit (flutter/engine#56647)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from b79e71223284 to
492e8347d7a4 (2 revisions) (flutter/engine#56687)
2024-11-18 jason-simmons@users.noreply.github.com Update the Skia build
scripts for a refactoring of the Fontconfig font manager
(flutter/engine#56684)
2024-11-18 chris@bracken.jp iOS,macOS: Enable ARC in
flutter_cflags_objc[c] (flutter/engine#56685)
2024-11-18 matanlurey@users.noreply.github.com Re-land "Remove
`android_jit_release_x86`." (flutter/engine#56681)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from 0d24bd3268ef to
b79e71223284 (1 revision) (flutter/engine#56683)
2024-11-18 tugorez@users.noreply.github.com Flutter views can gain focus
(flutter/engine#54985)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from 0b74d5c3eb4f to
0d24bd3268ef (1 revision) (flutter/engine#56680)

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 jimgraham@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
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 19, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 19, 2024
@engine-flutter-autoroll
7f2d56b8b Extract TestGLContext to separate translation unit (flutter…
4cc2877 
…/engine#56647)
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Nov 19, 2024
@engine-flutter-autoroll
Roll Flutter Engine from 878f593802e1 to c9a965c4e5e6 (11 revisions) (#…
91c7b81 
…159113)

flutter/engine@878f593...c9a965c

2024-11-18 skia-flutter-autoroll@skia.org Roll Dart SDK from
625e0a9cb67a to 05d58364e92f (1 revision) (flutter/engine#56688)
2024-11-18 chris@bracken.jp iOS: Eliminate unguarded-availability
opt-out (flutter/engine#56689)
2024-11-18 jacksongardner@google.com [skwasm] Use
`displayWidth`/`displayHeight` instead of `codedWidth`/`codedHeight`
(flutter/engine#56686)
2024-11-18 chris@bracken.jp Extract TestGLContext to separate
translation unit (flutter/engine#56647)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from b79e71223284 to
492e8347d7a4 (2 revisions) (flutter/engine#56687)
2024-11-18 jason-simmons@users.noreply.github.com Update the Skia build
scripts for a refactoring of the Fontconfig font manager
(flutter/engine#56684)
2024-11-18 chris@bracken.jp iOS,macOS: Enable ARC in
flutter_cflags_objc[c] (flutter/engine#56685)
2024-11-18 matanlurey@users.noreply.github.com Re-land "Remove
`android_jit_release_x86`." (flutter/engine#56681)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from 0d24bd3268ef to
b79e71223284 (1 revision) (flutter/engine#56683)
2024-11-18 tugorez@users.noreply.github.com Flutter views can gain focus
(flutter/engine#54985)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from 0b74d5c3eb4f to
0d24bd3268ef (1 revision) (flutter/engine#56680)

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 jimgraham@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
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 19, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 19, 2024
@engine-flutter-autoroll
7f2d56b8b Extract TestGLContext to separate translation unit (flutter…
f7d8710 
…/engine#56647)
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Nov 19, 2024
@engine-flutter-autoroll
Roll Flutter Engine from 878f593802e1 to 10dc07bf548f (12 revisions) (#…
6ed9327 
…159116)

flutter/engine@878f593...10dc07b

2024-11-18 reidbaker@google.com Update emulator definitions version to
latest available from chrome infra (flutter/engine#56313)
2024-11-18 skia-flutter-autoroll@skia.org Roll Dart SDK from
625e0a9cb67a to 05d58364e92f (1 revision) (flutter/engine#56688)
2024-11-18 chris@bracken.jp iOS: Eliminate unguarded-availability
opt-out (flutter/engine#56689)
2024-11-18 jacksongardner@google.com [skwasm] Use
`displayWidth`/`displayHeight` instead of `codedWidth`/`codedHeight`
(flutter/engine#56686)
2024-11-18 chris@bracken.jp Extract TestGLContext to separate
translation unit (flutter/engine#56647)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from b79e71223284 to
492e8347d7a4 (2 revisions) (flutter/engine#56687)
2024-11-18 jason-simmons@users.noreply.github.com Update the Skia build
scripts for a refactoring of the Fontconfig font manager
(flutter/engine#56684)
2024-11-18 chris@bracken.jp iOS,macOS: Enable ARC in
flutter_cflags_objc[c] (flutter/engine#56685)
2024-11-18 matanlurey@users.noreply.github.com Re-land "Remove
`android_jit_release_x86`." (flutter/engine#56681)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from 0d24bd3268ef to
b79e71223284 (1 revision) (flutter/engine#56683)
2024-11-18 tugorez@users.noreply.github.com Flutter views can gain focus
(flutter/engine#54985)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from 0b74d5c3eb4f to
0d24bd3268ef (1 revision) (flutter/engine#56680)

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 jimgraham@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
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 19, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 19, 2024
@engine-flutter-autoroll
7f2d56b8b Extract TestGLContext to separate translation unit (flutter…
7d58f21 
…/engine#56647)
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Nov 19, 2024
@engine-flutter-autoroll
Roll Flutter Engine from 878f593802e1 to c1b0e18a70b3 (13 revisions) (#…
4426413 
…159118)

flutter/engine@878f593...c1b0e18

2024-11-19 jonahwilliams@google.com [Impeller] use sync fence for image
uploads. (flutter/engine#56609)
2024-11-18 reidbaker@google.com Update emulator definitions version to
latest available from chrome infra (flutter/engine#56313)
2024-11-18 skia-flutter-autoroll@skia.org Roll Dart SDK from
625e0a9cb67a to 05d58364e92f (1 revision) (flutter/engine#56688)
2024-11-18 chris@bracken.jp iOS: Eliminate unguarded-availability
opt-out (flutter/engine#56689)
2024-11-18 jacksongardner@google.com [skwasm] Use
`displayWidth`/`displayHeight` instead of `codedWidth`/`codedHeight`
(flutter/engine#56686)
2024-11-18 chris@bracken.jp Extract TestGLContext to separate
translation unit (flutter/engine#56647)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from b79e71223284 to
492e8347d7a4 (2 revisions) (flutter/engine#56687)
2024-11-18 jason-simmons@users.noreply.github.com Update the Skia build
scripts for a refactoring of the Fontconfig font manager
(flutter/engine#56684)
2024-11-18 chris@bracken.jp iOS,macOS: Enable ARC in
flutter_cflags_objc[c] (flutter/engine#56685)
2024-11-18 matanlurey@users.noreply.github.com Re-land "Remove
`android_jit_release_x86`." (flutter/engine#56681)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from 0d24bd3268ef to
b79e71223284 (1 revision) (flutter/engine#56683)
2024-11-18 tugorez@users.noreply.github.com Flutter views can gain focus
(flutter/engine#54985)
2024-11-18 skia-flutter-autoroll@skia.org Roll Skia from 0b74d5c3eb4f to
0d24bd3268ef (1 revision) (flutter/engine#56680)

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 jimgraham@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
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
@cbracken
Extract TestGLContext to separate translation unit (flutter/engine#56647
6226520 
)

For consistency with the Test.*Context classes for other backends, which live in their own implementation file with their own header, extract TestEGLContext to its own header and TU so that in cases where only a TestEGLContext is required (e.g. EmbedderTestBackingStoreProducerGL), we don't need to include all the various test GL surface classes as well.

GetEGLError is used by both TestEGLContext and the TestGLSurface classes, so moves to its own utils file.

No tests because this is a refactoring with no semantic changes, and the code itself is test code.

Issue: flutter#158998

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@chinmaygarde chinmaygarde chinmaygarde approved these changes

@bdero bdero Awaiting requested review from bdero

@jonahwilliams jonahwilliams Awaiting requested review from jonahwilliams

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cbracken @stuartmorgan-g @chinmaygarde