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

Conversation

@betrevisan
Copy link
Contributor

This draft PR is a prototype of how one would add support for partial repaint to the Embedder API when using OpenGL.

@flutter-dashboard flutter-dashboard bot added the embedder Related to the embedder API label Jul 12, 2022
@betrevisan betrevisan requested a review from bdero July 12, 2022 20:48
/// damage in a self-contained way.
typedef struct {
/// The size of this struct. Must be sizeof(FlutterDamage).
size_t struct_size;
Copy link
Member

Choose a reason for hiding this comment

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

Any reason this should have a checked size? Given the way it's used in FlutterPresentInfo, the size of this struct can't ever be updated in a backwards compatible way because buffer_damage.damage will change its position.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I didn't think about that but it makes sense. I updated it now.

SkIRect rect = {static_cast<int32_t>(flutter_rect.left),
static_cast<int32_t>(flutter_rect.right),
static_cast<int32_t>(flutter_rect.top),
static_cast<int32_t>(flutter_rect.bottom)};
Copy link
Member

Choose a reason for hiding this comment

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

Should the right/bottom values be rounded up here?


std::array<EGLint, 4> static RectToInts(EGLDisplay display,
EGLSurface surface,
const SkIRect& rect) {
Copy link
Member

Choose a reason for hiding this comment

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

Rather than taking an SkIRect, maybe just take the FlutterRect and do the conversion inline?

option(SKIA_INSTALL "" OFF)
add_subdirectory(${CMAKE_SOURCE_DIR}/../../../third_party/skia/out/config skia)
target_link_libraries(flutter_glfw skia)
include_directories(${CMAKE_SOURCE_DIR}/../../../third_party/skia)
Copy link
Member

Choose a reason for hiding this comment

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

To keep the build simple, I'd avoid building Skia here and just implement the rectangle behavior you need in the example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds like a good approach! Thanks!

@Hixie
Copy link
Contributor

Hixie commented Oct 18, 2022

@betrevisan Is this PR still on your radar?

@betrevisan
Copy link
Contributor Author

@betrevisan Is this PR still on your radar?

Hi @Hixie! My internship has already ended but if there is anything you would like me to take a look at, I'd be happy to help.

@Hixie
Copy link
Contributor

Hixie commented Oct 27, 2022

@betrevisan That's entirely up to you, if you are interested in still contributing you are welcome to do so. :-) If you do not have the bandwidth that's totally understandable too; in that case should we close this PR? Is there an issue I should attach the PR to so that we don't forget the work you've done here?

@betrevisan
Copy link
Contributor Author

@betrevisan That's entirely up to you, if you are interested in still contributing you are welcome to do so. :-) If you do not have the bandwidth that's totally understandable too; in that case should we close this PR? Is there an issue I should attach the PR to so that we don't forget the work you've done here?

@Hixie this can be closed because it has already been implemented and merged in this PR here #35022. So this can be closed.

@goderbauer goderbauer closed this Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

embedder Related to the embedder API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants