[Windows] Refactor EGL initialization #49895
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
loic-sharma
force-pushed
the
windows_refactor_egl_initialization
branch
from
f81646c to
b4442a8
Compare
|
test-exempt: code refactor with no semantic change |
yaakovschectman
left a comment
yaakovschectman
left a comment
There was a problem hiding this comment.
LGTM mod one minor question
| result = ::eglChooseConfig(display_, impeller_config_attributes, &config_, | ||
| 1, &num_config); | ||
|
|
||
| if (result == EGL_TRUE && num_config > 0) { |
There was a problem hiding this comment.
Is == EGL_TRUE preferable to testing result outright?
There was a problem hiding this comment.
As in something like if (result) { ... }? Yes something like == EGL_TRUE is preferrable as EGL uses non-zero values to represent error codes. For example, EGL would return 0x300B to present a context lost error.
loic-sharma
added
the
autosubmit
…141909) flutter/engine@517fc2b...d00e55f 2024-01-19 [email protected] [Windows] Refactor EGL initialization (flutter/engine#49895) 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
This is a refactoring with no semantic changes. The following types were renamed: * `flutter::AngleSurfaceManager` to `flutter::egl::Manager`. In subsequent pull requests, this type will _create_ but not _own_ surfaces. Furthermore, this type will be split up to introduce `flutter::egl::Surface` and `flutter::egl::Context`. The manager will own the contexts and each surface will be owned by its view. * `flutter::GlProcTable` to `flutter::egl::ProcTable` This also introduces an `egl` directory to the Windows embedder. Previous pull request: #49895 Part of flutter/flutter#141996 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
3 participants
This is a refactoring with no semantic changes:
AngleSurfaceManagerAngleSurfaceManager's initialization logicNext pull request: #49900
Part of flutter/flutter#141996
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.