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

Conversation

@chinmaygarde
Copy link
Member

@chinmaygarde chinmaygarde commented Apr 30, 2024

@chinmaygarde
Copy link
Member Author

Patched up the EGL toolkit headerdocs in #52469

@chinmaygarde chinmaygarde requested a review from jonahwilliams May 1, 2024 02:34
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM with nits

@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label May 1, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 1, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented May 1, 2024

auto label is removed for flutter/engine/52473, due to - The status or check suite Linux mac_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label May 1, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 1, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented May 1, 2024

auto label is removed for flutter/engine/52473, due to - The status or check suite Linux mac_android_aot_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@chinmaygarde chinmaygarde force-pushed the impeller_gl_porting_guide branch from 2ea87f3 to 6d708a3 Compare May 1, 2024 17:53
@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label May 1, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented May 1, 2024

auto label is removed for flutter/engine/52473, due to - The status or check suite Linux linux_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 1, 2024

## Render to the Surface

Give the surface to the rendere along with a callback that details how you will populate the render target the renderer sets up that is directed at that surface.

Choose a reason for hiding this comment

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

Suggested change
Give the surface to the rendere along with a callback that details how you will populate the render target the renderer sets up that is directed at that surface.
Give the surface to the renderer along with a callback that details how you will populate the render target the renderer sets up that is directed at that surface.

});
```

And that's it. Now you have functional WSI and render loop. Higher level frameworks like Aiks and DisplayList that use the render target to render their rendering into to the surface.

Choose a reason for hiding this comment

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

Suggested change
And that's it. Now you have functional WSI and render loop. Higher level frameworks like Aiks and DisplayList that use the render target to render their rendering into to the surface.
And that's it. Now you have a functional WSI and render loop. Higher level frameworks like Aiks and DisplayList use the render target to render their rendering into the surface.

@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label May 1, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 1, 2024
@auto-submit
Copy link
Contributor

auto-submit bot commented May 1, 2024

auto label is removed for flutter/engine/52473, due to - The status or check suite Mac mac_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

  • The status or check suite Mac mac_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux linux_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

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.

This guide looks great!

First create an EGL display connection:

```c++
egl::Display display;
Copy link
Member

Choose a reason for hiding this comment

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

nit: would be nice to include headers for the various symbols referenced, although that would make this doc a bit more fragile.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I figured linking the patch would be sufficient and leaving the includes for brevity.

Copy link
Contributor

Choose a reason for hiding this comment

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

fwiw (not saying we need to do this here), what we do in the flutter/flutter repo is literally extract out the ``` bits of code and statically analyze them, and what we do in the flutter/packages repo is have the README files explicitly link to actual code that is tested and then enforce that the README file snippets match the actual code. In both cases the result is that code in documentation is checked on every commit.

Copy link
Member Author

@chinmaygarde chinmaygarde May 6, 2024

Choose a reason for hiding this comment

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

I spoke to @yjbanov and I may just check in the code (in a separate patch). Previously, I didn't want to setup the toolchain and testing for a target just for documentation.


This guide describes how to use Impeller as a standalone rendering library using OpenGL ES. Additionally, some form of WSI (Window System Integration) is essential. Since EGL is the most the most popular form of performing WSI on platforms with OpenGL ES, Impeller has a toolkit that assists in working with EGL. This guide will use that toolkit.

While this guide focuses on OpenGL ES with EGL, the steps to setup rendering with another client rendering API (Metal and Vulkan) are fairly similar and you should be able to follow the same pattern for other backends.
Copy link
Contributor

Choose a reason for hiding this comment

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

setup rendering -> set up rendering or maybe even set rendering up

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


While this guide focuses on OpenGL ES with EGL, the steps to setup rendering with another client rendering API (Metal and Vulkan) are fairly similar and you should be able to follow the same pattern for other backends.

This guide details extremely low-level setup and the `//impeller/renderer` API directly above the HAL. Most users of Impeller will likely use the API using convenience wrappers already written for the platform. Interacting directly with the HAL is extremely powerful but also verbose. Applications are likely to also use higher level frameworks like Aiks or Display Lists.
Copy link
Contributor

Choose a reason for hiding this comment

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

can Aiks and Display Lists link to documentation for them?

Building Impeller for the target platform is outside the scope of this guide.

> [!CAUTION]
> The code provided inline is pseudo-code and doesn't include error handling. See the headerdocs for more on error handling and failure modes. All classes are assumed to be in the `impeller` namespace. For a more complete example of setting up standalone Impeller, see [this patch](https://github.com/flutter/engine/pull/52472/files) that adds support for Impeller rendering via Wasm [in the browser and WebGL 2](https://public.chinmaygarde.com/impeller/wasm/wasm.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

the patch will bit rot over time, is there some way we can link to tested code that will remain up to date?

Copy link
Member Author

Choose a reason for hiding this comment

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

In a separate patch, I'm going to check it in. I didn't want to setup the toolchain and testing for a target that was a toy.


## Setup the Swap Callback

The swap callback will get invoked when the renderer presents the surface. Remember in our list of things to do, we need to first tell the reactor worker to flush all pending OpenGL operations and then present the surface. Setup the swap callback appropriately.
Copy link
Contributor

Choose a reason for hiding this comment

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

Set up

Copy link
Contributor

Choose a reason for hiding this comment

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

actually Set the swap callback up

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@Hixie
Copy link
Contributor

Hixie commented May 1, 2024

This is great, so happy to see these kinds of docs. I hope you don't mind the nitpicky nature of my grammar comments.

@chinmaygarde
Copy link
Member Author

This is great, so happy to see these kinds of docs. I hope you don't mind the nitpicky nature of my grammar comments.

Not at all. Want to add to these docs at a regular cadence.

@chinmaygarde chinmaygarde force-pushed the impeller_gl_porting_guide branch from 6d708a3 to 59ad8d7 Compare May 6, 2024 23:03
@chinmaygarde
Copy link
Member Author

@Hixie PTAL.

Copy link
Contributor

@Hixie Hixie left a comment

Choose a reason for hiding this comment

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

LGTM

@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label May 6, 2024
@auto-submit auto-submit bot merged commit b64e230 into flutter:main May 6, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 7, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 7, 2024
…147904)

flutter/engine@4cb9e02...b64e230

2024-05-06 [email protected] [Impeller] Document how to use Impeller as a standalone library with OpenGL ES and EGL. (flutter/engine#52473)

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
@chinmaygarde chinmaygarde deleted the impeller_gl_porting_guide branch May 7, 2024 16:42
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

None yet

Development

Successfully merging this pull request may close these issues.

5 participants