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

Conversation

@bdero
Copy link
Member

@bdero bdero commented Aug 20, 2023

Resolves flutter/flutter#130947.

  • Allow for setting the text renderer (TextRenderContext) when the AiksContext is built. Previously, the text renderer was selected at build time through linking a TextRenderContext::Create symbol.
  • Support using Impeller without a text backend. Throw a clear error when trying to render text if no text backend is present.
  • Don't track the Impeller context in TextRenderContext. Just let the renderer supply its context when generating atlases.
  • Allow for overriding the TextRenderContext for individual Aiks playground tests/goldens.

@bdero bdero requested review from chinmaygarde and zanderso August 20, 2023 21:56
@bdero bdero self-assigned this Aug 20, 2023
@bdero bdero force-pushed the bdero/runtime-text-context branch from 2c711eb to e2b4863 Compare August 20, 2023 22:26
@bdero bdero changed the title [Impeller] Supply the text backend to the AiksContext at runtime [Impeller] Supply a text backend to the AiksContext at runtime Aug 20, 2023
@bdero bdero changed the title [Impeller] Supply a text backend to the AiksContext at runtime [Impeller] Supply a text backend to the AiksContext at runtime. Aug 20, 2023
bool AiksPlayground::OpenPlaygroundHere(const Picture& picture) {
bool AiksPlayground::OpenPlaygroundHere(
const Picture& picture,
std::unique_ptr<TextRenderContext> text_render_context_override) {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of the context being provided as as override to each playground open call, I'd prefer the fixture itself take and store the context. Does it need to be unique ptr? If not, it could just store it as a shared ptr.

Avoid having to patch each call-site to open the playground and looks more extensible.

Copy link
Member Author

Choose a reason for hiding this comment

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

By the "fixture" storing the context, do you mean having something like AiksPlayground::SetTextRenderContext()?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

Copy link
Member Author

Choose a reason for hiding this comment

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

SGTM, done.

@bdero bdero force-pushed the bdero/runtime-text-context branch from 63e82f1 to c2685a2 Compare August 22, 2023 00:18
@bdero bdero merged commit fd52779 into flutter:main Aug 22, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 22, 2023
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Aug 22, 2023
…133026)

flutter/engine@454e0e3...e183e8a

2023-08-22 [email protected] Roll Skia from 9ecdc9265aaa to 14e7b3b2d660 (1 revision) (flutter/engine#44942)
2023-08-22 [email protected] [Impeller] Supply a text backend to the AiksContext at runtime. (flutter/engine#44884)

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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…ter#44884)

Resolves flutter/flutter#130947.

- Allow for setting the text renderer (`TextRenderContext`) when the
`AiksContext` is built. Previously, the text renderer was selected at
build time through linking a `TextRenderContext::Create` symbol.
- Support using Impeller without a text backend. Throw a clear error
when trying to render text if no text backend is present.
- Don't track the Impeller context in `TextRenderContext`. Just let the
renderer supply its context when generating atlases.
- Allow for overriding the `TextRenderContext` for individual Aiks
playground tests/goldens.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

[Impeller] Add support for swapping backends dynamically in impeller/typographer.

2 participants