-
Notifications
You must be signed in to change notification settings - Fork 6k
Backend-specific EmbedderTestBackingStoreProducers #56638
Backend-specific EmbedderTestBackingStoreProducers #56638
Conversation
|
|
||
| GrMtlTextureInfo skia_texture_info; | ||
| skia_texture_info.fTexture.reset(SkCFSafeRetain(texture_info.texture)); | ||
| GrBackendTexture backend_texture = GrBackendTextures::MakeMtl( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing to do here, but all these tests/test harness that use Skia types directly like this will need to either be rewritten or deleted next year
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. I suspect there's a non-zero amount of Skia-specific code in the embedder_unittests source too for that matter.
Refactoring all this stuff out into backend-specific classes hopefully makes either of those options easier.
jonahwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| // Ideally this would be set in the constructor, however we can't do that | ||
| // without introducing different constructors depending on different graphics | ||
| // APIs, which is a bit ugly. | ||
| test_egl_context_ = std::move(context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out introducing different constructors is less ugly.
| #ifdef SHELL_ENABLE_GL | ||
| std::shared_ptr<TestEGLContext> egl_context_ = nullptr; | ||
| #endif | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last ifdef of the file, gone.
Extracts backend-specific subclasses of EmbedderTestBackingStoreProducer for OpenGL, Metal, Software, and Vulkan. Issue: flutter/flutter#158998
…159017) flutter/engine@f238321...98c3b7f 2024-11-16 [email protected] Backend-specific EmbedderTestBackingStoreProducers (flutter/engine#56638) 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] 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
Eliminates an unused ifdef'ed include in embedder_test_backingstore_producer.h and adds it to embedder_test_context_gl.h, where it's needed. The need for this include was eliminated in #56638. No test changes since the patch introduces no semantic changes. Issue: flutter/flutter#158998 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
) Extracts backend-specific subclasses of EmbedderTestBackingStoreProducer for OpenGL, Metal, Software, and Vulkan. Issue: flutter#158998 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Eliminates an unused ifdef'ed include in embedder_test_backingstore_producer.h and adds it to embedder_test_context_gl.h, where it's needed. The need for this include was eliminated in flutter/engine#56638. No test changes since the patch introduces no semantic changes. Issue: flutter#158998 [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Extracts backend-specific subclasses of EmbedderTestBackingStoreProducer for OpenGL, Metal, Software, and Vulkan.
Issue: flutter/flutter#158998
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.