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

Conversation

@johnmccutchan
Copy link
Contributor

  • Introduce PlatformViewRenderTarget interface.
  • Refactor VirtualDisplayController and PlatformViewWrapper to extract SurfaceTexturePlatformViewRenderTarget into a separate class.

In a future CL I will add an ImageReaderPlatformViewRenderTarget which will enable Platform Views on Impeller/Vulkan.

Tracking issue: flutter/flutter#130892

void unlockCanvasAndPost(Canvas canvas);

// The id of this render target.
public long id();
Copy link
Contributor

Choose a reason for hiding this comment

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

getId?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a style guide rule here? I see some getters prefixed with "get" and others without. I don't have a strong opinion here but it would be nice if there was a recommended approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Google Java style guide doesn't explicitly say, but many of the examples there start with get, and my impression (maybe incorrect, I don't have a lot of Java background) is that Java convention in general was to use get for getters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

// The id of this render target.
public long id();

// Release backing resources.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Releases

(For consistency with all the other comments.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

public SurfaceTexture getTexture() {
return tx;
}
public void setRenderTarget(@Nullable PlatformViewRenderTarget renderTarget) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a no-op? Shouldn't it change this.renderTarget?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was dead code. Removed.

- Introduce PlatformViewRenderTarget interface.
- Refactor VirtualDisplayController and PlatformViewWrapper to extract
  SurfaceTexturePlatformViewRenderTarget into a separate class.

In a future CL I will add an ImageReaderPlatformViewRenderTarget which will
enable Platform Views on Impeller/Vulkan.
@johnmccutchan johnmccutchan merged commit 2046254 into flutter:main Jul 20, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 20, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 20, 2023
flutter/engine@c902fec...2046254

2023-07-20 john@johnmccutchan.com Add a PlatformViewRenderTarget abstraction (flutter/engine#43813)

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 bdero@google.com,rmistry@google.com,zra@google.com 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
harryterkelsen pushed a commit to harryterkelsen/engine that referenced this pull request Jul 20, 2023
- Introduce PlatformViewRenderTarget interface.
- Refactor VirtualDisplayController and PlatformViewWrapper to extract
SurfaceTexturePlatformViewRenderTarget into a separate class.

In a future CL I will add an ImageReaderPlatformViewRenderTarget which
will enable Platform Views on Impeller/Vulkan.

Tracking issue: flutter/flutter#130892
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
)

flutter/engine@c902fec...2046254

2023-07-20 john@johnmccutchan.com Add a PlatformViewRenderTarget abstraction (flutter/engine#43813)

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 bdero@google.com,rmistry@google.com,zra@google.com 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants