Skip to content
Open
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
36 changes: 21 additions & 15 deletions packages/flame/lib/src/camera/camera_component.dart
Original file line number Diff line number Diff line change
Expand Up @@ -203,16 +203,6 @@ class CameraComponent extends Component {
canvas.save();
try {
currentCameras.add(this);
void renderWorld(Canvas canvas) {
canvas.transform2D(viewfinder.transform);
world!.renderFromCamera(canvas);

// Render the viewfinder elements, which will be in front of
// the world,
// but with the same transforms applied to them.
viewfinder.renderTree(canvas);
}

final postProcessors = children.query<PostProcessComponent>();
if (postProcessors.isNotEmpty) {
assert(
Expand All @@ -223,13 +213,11 @@ class CameraComponent extends Component {
postProcessor.render(
canvas,
viewport.virtualSize,
renderWorld,
(context) {
renderContext.currentPostProcess = context;
},
_renderWorld,
_updatePostProcessContext,
);
} else {
renderWorld(canvas);
_renderWorld(canvas);
}
} finally {
currentCameras.removeLast();
Expand All @@ -242,6 +230,24 @@ class CameraComponent extends Component {
canvas.restore();
}

/// Renders the world and the viewfinder elements through the camera
/// transform. An instance method rather than a local function, so that the
/// render pass does not allocate a closure per camera per frame.
void _renderWorld(Canvas canvas) {
canvas.transform2D(viewfinder.transform);
world!.renderFromCamera(canvas);
// Render the viewfinder elements, which will be in front of the world,
// but with the same transforms applied to them.
viewfinder.renderTree(canvas);
}

// Not a setter: this is passed as a `ValueSetter` tear-off to
// `PostProcess.render`.
// ignore: use_setters_to_change_properties
void _updatePostProcessContext(PostProcess? context) {
renderContext.currentPostProcess = context;
}

/// Converts from the global (canvas) coordinate space to
/// local (camera = viewport + viewfinder).
///
Expand Down
6 changes: 5 additions & 1 deletion packages/flame/lib/src/components/mixins/has_decorator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,16 @@ import 'package:flame/src/rendering/decorator.dart';
mixin HasDecorator on Component {
Decorator? decorator;

/// Cached `super.renderTree` tear-off, so that the render pass does not
/// allocate a fresh closure for [Decorator.applyChain] on every frame.
void Function(Canvas)? _superRenderTree;

@override
void renderTree(Canvas canvas) {
if (decorator == null) {
super.renderTree(canvas);
} else {
decorator!.applyChain(super.renderTree, canvas);
decorator!.applyChain(_superRenderTree ??= super.renderTree, canvas);
}
}
}
6 changes: 5 additions & 1 deletion packages/flame/lib/src/components/position_component.dart
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,13 @@ class PositionComponent extends Component
}
}

/// Cached `super.renderTree` tear-off, so that the render pass does not
/// allocate a fresh closure for [Decorator.applyChain] on every frame.
void Function(Canvas)? _superRenderTree;

@override
void renderTree(Canvas canvas) {
decorator.applyChain(super.renderTree, canvas);
decorator.applyChain(_superRenderTree ??= super.renderTree, canvas);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

curious why all the others can't use the has decorator mixin? is there a way to share this logic upstream?

}

@internal
Expand Down
6 changes: 5 additions & 1 deletion packages/flame/lib/src/components/router/route.dart
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,14 @@ class Route extends PositionComponent
}
}

/// Cached `super.renderTree` tear-off, so that the render pass does not
/// allocate a fresh closure for [Decorator.applyChain] on every frame.
void Function(Canvas)? _superRenderTree;

@override
void renderTree(Canvas canvas) {
if (isRendered) {
_renderEffect.applyChain(super.renderTree, canvas);
_renderEffect.applyChain(_superRenderTree ??= super.renderTree, canvas);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it is quite unfortunate that the compiler can't optimize this

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

an alternative to consider, make applyChain receive a generic component or superclass with renderTree and have it call the function directly, avoiding the lambda entirely

}
}

Expand Down
29 changes: 16 additions & 13 deletions packages/flame/lib/src/post_process/post_process_component.dart
Original file line number Diff line number Diff line change
Expand Up @@ -105,22 +105,25 @@ class PostProcessComponent<T extends PostProcess> extends PositionComponent {
return superSize;
}

/// Cached render chain, so that the render pass does not allocate fresh
/// closures for `Decorator.applyChain` on every frame.
void Function(Canvas)? _renderChain;

@override
@mustCallSuper
void renderTree(Canvas canvas) {
decorator.applyChain(
(canvas) {
postProcess.render(
canvas,
size,
super.renderTreeWithoutDecorator,
(context) {
_renderContext.postProcess = postProcess;
},
);
},
canvas,
);
decorator.applyChain(_renderChain ??= _buildRenderChain(), canvas);
}

void Function(Canvas) _buildRenderChain() {
final renderTree = super.renderTreeWithoutDecorator;
void updateContext(PostProcess? context) {
_renderContext.postProcess = postProcess;
}

return (canvas) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you don't want to tear-off-ify this lambda too?

postProcess.render(canvas, size, renderTree, updateContext);
};
}
}

Expand Down
22 changes: 16 additions & 6 deletions packages/flame/lib/src/rendering/decorator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,26 @@ class Decorator {
/// The next decorator in the chain, or null if there is none.
Decorator? _next;

/// Cached closure that forwards the draw call to the rest of the chain,
/// so that no closure needs to be allocated per frame. It is keyed by the
/// identity of the [_chainedDrawSource] it wraps: callers that pass the
/// same (cached) draw callback every frame reuse the same chain closure.
late void Function(Canvas) _chainedDraw;
void Function(Canvas)? _chainedDrawSource;

/// Applies this and all subsequent decorators if any.
///
/// This method is the main method through which the decorator is applied.
void applyChain(void Function(Canvas) draw, Canvas canvas) {
apply(
_next == null
? draw
: (nextCanvas) => _next!.applyChain(draw, nextCanvas),
canvas,
);
if (_next == null) {
apply(draw, canvas);
} else {
if (!identical(_chainedDrawSource, draw)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the need for this makes me question if the decorator pattern is the best one for the dart language.
the other tear-offs are ok but this is adding many more layers of complexity, state management and potential for mistakes.
but since this is a hot path for all render calls of all components (regardless of having decorators), this is ok. still trying to think more holistically in the meanwhile

_chainedDrawSource = draw;
_chainedDraw = (nextCanvas) => _next!.applyChain(draw, nextCanvas);
}
apply(_chainedDraw, canvas);
}
}

/// Applies visual effect while [draw]ing on the [canvas].
Expand Down
Loading