Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions lib/ui/compositing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,15 @@ class PhysicalShapeEngineLayer extends _EngineLayerWrapper {
PhysicalShapeEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer);
}

/// An opaque handle to a picture engine layer.
///
/// Instances of this class are created by [SceneBuilder.addPicture].
///
/// {@macro dart.ui.sceneBuilder.oldLayerCompatibility}
class PictureEngineLayer extends _EngineLayerWrapper {
PictureEngineLayer._(EngineLayer nativeLayer) : super._(nativeLayer);
}

/// Builds a [Scene] containing the given visuals.
///
/// A [Scene] can then be rendered using [FlutterView.render].
Expand Down Expand Up @@ -699,18 +708,29 @@ class SceneBuilder extends NativeFieldWrapperClass2 {
/// Adds a [Picture] to the scene.
///
/// The picture is rasterized at the given offset.
void addPicture(
///
/// {@macro dart.ui.sceneBuilder.oldLayer}
PictureEngineLayer addPicture(
Offset offset,
Picture picture, {
bool isComplexHint = false,
bool willChangeHint = false,
PictureEngineLayer? oldLayer,
Copy link
Contributor

Choose a reason for hiding this comment

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

Docs for the new argument? I think the dart.ui.sceneBuilder.oldLayer macro should be reusable here. There's also the dart.ui.sceneBuilder.oldLayerVsRetained macro but I'm not sure it applies to pictures. Picture layer is a leaf layer and the same picture can be reused multiple time in the same scene.

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

}) {
assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'addPicture'));
final EngineLayer engineLayer = EngineLayer._();
final int hints = (isComplexHint ? 1 : 0) | (willChangeHint ? 2 : 0);
_addPicture(offset.dx, offset.dy, picture, hints);
_addPicture(engineLayer, offset.dx, offset.dy, picture, hints, oldLayer?._nativeLayer);
return PictureEngineLayer._(engineLayer);
}

void _addPicture(double dx, double dy, Picture picture, int hints)
native 'SceneBuilder_addPicture';
void _addPicture(
EngineLayer outEngineLayer,
double dx,
double dy,
Picture picture,
int hints,
EngineLayer? oldLayer) native 'SceneBuilder_addPicture';

/// Adds a backend texture to the scene.
///
Expand Down
14 changes: 10 additions & 4 deletions lib/ui/compositing/scene_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,14 +272,20 @@ void SceneBuilder::pop() {
PopLayer();
}

void SceneBuilder::addPicture(double dx,
void SceneBuilder::addPicture(Dart_Handle layer_handle,
double dx,
double dy,
Picture* picture,
int hints) {
auto layer = std::make_unique<flutter::PictureLayer>(
int hints,
fml::RefPtr<EngineLayer> oldLayer) {
auto layer = std::make_shared<flutter::PictureLayer>(
SkPoint::Make(dx, dy), UIDartState::CreateGPUObject(picture->picture()),
!!(hints & 1), !!(hints & 2));
AddLayer(std::move(layer));
AddLayer(layer);
EngineLayer::MakeRetained(layer_handle, layer);
if (oldLayer && oldLayer->Layer()) {
layer->AssignOldLayer(oldLayer->Layer().get());
}
}

void SceneBuilder::addTexture(double dx,
Expand Down
7 changes: 6 additions & 1 deletion lib/ui/compositing/scene_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,12 @@ class SceneBuilder : public RefCountedDartWrappable<SceneBuilder> {
double top,
double bottom);

void addPicture(double dx, double dy, Picture* picture, int hints);
void addPicture(Dart_Handle layer_handle,
double dx,
double dy,
Picture* picture,
int hints,
fml::RefPtr<EngineLayer> oldLayer);

void addTexture(double dx,
double dy,
Expand Down
5 changes: 3 additions & 2 deletions lib/ui/painting/engine_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "flutter/lib/ui/painting/engine_layer.h"

#include "flutter/lib/ui/ui_dart_state.h"
#include "third_party/tonic/converter/dart_converter.h"
#include "third_party/tonic/dart_args.h"
#include "third_party/tonic/dart_binding_macros.h"
Expand All @@ -13,14 +14,14 @@ using tonic::ToDart;

namespace flutter {

EngineLayer::EngineLayer(std::shared_ptr<flutter::ContainerLayer> layer)
EngineLayer::EngineLayer(std::shared_ptr<flutter::Layer> layer)
: layer_(layer) {}

EngineLayer::~EngineLayer() = default;

size_t EngineLayer::GetAllocationSize() const {
// Provide an approximation of the total memory impact of this object to the
// Dart GC. The ContainerLayer may hold references to a tree of other layers,
// Dart GC. The Layer may hold references to a tree of other layers,
// which in turn may contain Skia objects.
return 3000;
};
Expand Down
14 changes: 8 additions & 6 deletions lib/ui/painting/engine_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,21 @@ class EngineLayer : public RefCountedDartWrappable<EngineLayer> {

size_t GetAllocationSize() const override;

static void MakeRetained(Dart_Handle dart_handle,
std::shared_ptr<flutter::ContainerLayer> layer) {
auto engine_layer = fml::MakeRefCounted<EngineLayer>(layer);
static fml::RefPtr<EngineLayer> MakeRetained(
Dart_Handle dart_handle,
std::shared_ptr<flutter::Layer> layer) {
auto engine_layer = fml::MakeRefCounted<EngineLayer>(std::move(layer));
engine_layer->AssociateWithDartWrapper(dart_handle);
return engine_layer;
}

static void RegisterNatives(tonic::DartLibraryNatives* natives);

std::shared_ptr<flutter::ContainerLayer> Layer() const { return layer_; }
std::shared_ptr<flutter::Layer> Layer() const { return layer_; }

private:
explicit EngineLayer(std::shared_ptr<flutter::ContainerLayer> layer);
std::shared_ptr<flutter::ContainerLayer> layer_;
explicit EngineLayer(std::shared_ptr<flutter::Layer> layer);
std::shared_ptr<flutter::Layer> layer_;

FML_FRIEND_MAKE_REF_COUNTED(EngineLayer);
};
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/canvaskit/layer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ class ShaderMaskEngineLayer extends ContainerLayer implements ui.ShaderMaskEngin
}

/// A layer containing a [Picture].
class PictureLayer extends Layer {
class PictureLayer extends Layer implements ui.PictureEngineLayer {
/// The picture to paint into the canvas.
final CkPicture picture;

Expand Down
13 changes: 10 additions & 3 deletions lib/web_ui/lib/src/engine/canvaskit/layer_scene_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,21 @@ class LayerSceneBuilder implements ui.SceneBuilder {
}

@override
void addPicture(
ui.PictureEngineLayer addPicture(
ui.Offset offset,
ui.Picture picture, {
bool isComplexHint = false,
bool willChangeHint = false,
ui.PictureEngineLayer? oldLayer,
}) {
currentLayer.add(PictureLayer(
picture as CkPicture, offset, isComplexHint, willChangeHint));
final PictureLayer layer = PictureLayer(
picture as CkPicture,
offset,
isComplexHint,
willChangeHint,
);
currentLayer.add(layer);
return layer;
}

@override
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/html/picture.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ void _recycleCanvas(EngineCanvas? canvas) {

/// A surface that uses a combination of `<canvas>`, `<div>` and `<p>` elements
/// to draw shapes and text.
class PersistedPicture extends PersistedLeafSurface {
class PersistedPicture extends PersistedLeafSurface implements ui.PictureEngineLayer {
PersistedPicture(this.dx, this.dy, this.picture, this.hints)
: localPaintBounds = picture.recordingCanvas!.pictureBounds;

Expand Down
13 changes: 10 additions & 3 deletions lib/web_ui/lib/src/engine/html/scene_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,12 @@ class SurfaceSceneBuilder implements ui.SceneBuilder {
///
/// The picture is rasterized at the given offset.
@override
void addPicture(
ui.PictureEngineLayer addPicture(
ui.Offset offset,
ui.Picture picture, {
bool isComplexHint = false,
bool willChangeHint = false,
ui.PictureEngineLayer? oldLayer,
}) {
int hints = 0;
if (isComplexHint) {
Expand All @@ -372,8 +373,14 @@ class SurfaceSceneBuilder implements ui.SceneBuilder {
if (willChangeHint) {
hints |= 2;
}
_addSurface(PersistedPicture(
offset.dx, offset.dy, picture as EnginePicture, hints));
final PersistedPicture layer = PersistedPicture(
offset.dx,
offset.dy,
picture as EnginePicture,
hints,
);
_addSurface(layer);
return layer;
}

/// Adds a backend texture to the scene.
Expand Down
7 changes: 6 additions & 1 deletion lib/web_ui/lib/src/ui/compositing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ abstract class ShaderMaskEngineLayer implements EngineLayer {}

abstract class PhysicalShapeEngineLayer implements EngineLayer {}

// TODO(yjbanov): Incorporate into DOM diffing algorithm.
// https://github.com/flutter/flutter/issues/82287
abstract class PictureEngineLayer implements EngineLayer {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please leave a TODO(yjbanov) here and a Github issue for me to incorporate this into our DOM diffing algorithm?

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


abstract class SceneBuilder {
factory SceneBuilder() {
if (engine.useCanvasKit) {
Expand Down Expand Up @@ -99,11 +103,12 @@ abstract class SceneBuilder {
void addRetained(EngineLayer retainedLayer);
void pop();
void addPerformanceOverlay(int enabledOptions, Rect bounds);
void addPicture(
PictureEngineLayer addPicture(
Offset offset,
Picture picture, {
bool isComplexHint = false,
bool willChangeHint = false,
PictureEngineLayer? oldLayer,
});
void addTexture(
int textureId, {
Expand Down
21 changes: 17 additions & 4 deletions testing/dart/compositing_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -279,16 +279,18 @@ void main() {
builder2.build();
}

void testNoSharing(_TestNoSharingFunction pushFunction) {
void testNoSharing(_TestNoSharingFunction pushFunction, {bool isLeafLayer = false}) {
testPushThenIllegalRetain(pushFunction);
testAddRetainedThenIllegalPush(pushFunction);
testDoubleAddRetained(pushFunction);
testPushOldLayerTwice(pushFunction);
testPushChildLayerOfRetainedLayer(pushFunction);
testRetainParentLayerOfPushedChild(pushFunction);
testRetainOldLayer(pushFunction);
testPushOldLayer(pushFunction);
testRetainsParentOfOldLayer(pushFunction);
if (!isLeafLayer) {
testPushChildLayerOfRetainedLayer(pushFunction);
testRetainParentLayerOfPushedChild(pushFunction);
testRetainsParentOfOldLayer(pushFunction);
}
}

test('SceneBuilder does not share a layer between addRetained and push*', () {
Expand Down Expand Up @@ -377,6 +379,17 @@ void main() {
oldLayer: oldLayer as ImageFilterEngineLayer,
);
});
testNoSharing((SceneBuilder builder, EngineLayer oldLayer) {
final PictureRecorder recorder = PictureRecorder();
final Canvas canvas = Canvas(recorder);
canvas.drawPaint(Paint());
final Picture picture = recorder.endRecording();
return builder.addPicture(
Offset.zero,
picture,
oldLayer: oldLayer as PictureEngineLayer,
);
}, isLeafLayer: true);
});
}

Expand Down