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

skip painting clipped out pictures #18204

Merged
yjbanov merged 1 commit into from
May 8, 2020
Merged

skip painting clipped out pictures #18204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 10 additions & 3 deletions lib/web_ui/lib/src/engine/surface/picture.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,11 @@ class PersistedStandardPicture extends PersistedPicture {
return 1.0;
} else {
final BitmapCanvas oldCanvas = existingSurface._canvas;
if (!oldCanvas.doesFitBounds(_exactLocalCullRect)) {
if (oldCanvas == null) {
// We did not allocate a canvas last time. This can happen when the
// picture is completely clipped out of the view.
return 1.0;
} else if (!oldCanvas.doesFitBounds(_exactLocalCullRect)) {
// The canvas needs to be resized before painting.
return 1.0;
} else {
Expand Down Expand Up @@ -547,7 +551,10 @@ abstract class PersistedPicture extends PersistedLeafSurface {

void _applyPaint(PersistedPicture oldSurface) {
final EngineCanvas oldCanvas = oldSurface?._canvas;
if (!picture.recordingCanvas.didDraw) {
if (!picture.recordingCanvas.didDraw || _optimalLocalCullRect.isEmpty) {
// The picture is empty, or it has been completely clipped out. Skip
// painting. This removes all the setup work and scaffolding objects
// that won't be useful for anything anyway.
_recycleCanvas(oldCanvas);
domRenderer.clearDom(rootElement);
return;
Expand Down Expand Up @@ -642,7 +649,7 @@ abstract class PersistedPicture extends PersistedLeafSurface {
super.debugValidate(validationErrors);

if (picture.recordingCanvas.didDraw) {
if (debugCanvas == null) {
if (!_optimalLocalCullRect.isEmpty && debugCanvas == null) {
validationErrors
.add('$runtimeType has non-trivial picture but it has null canvas');
}
Expand Down
27 changes: 26 additions & 1 deletion lib/web_ui/test/compositing_test.dart → ...st/engine/surface/scene_builder_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import 'package:ui/ui.dart';

import 'package:test/test.dart';

import 'matchers.dart';
import '../../matchers.dart';
Copy link
Contributor

@ferhatb ferhatb May 7, 2020

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

@yjbanov yjbanov May 7, 2020

Choose a reason for hiding this comment

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

I moved this test so it matches what it's testing, so had to update relative imports.


void main() {
group('SceneBuilder', () {
Expand Down Expand Up @@ -239,6 +239,31 @@ void main() {
});
});

test('skips painting picture when picture fully clipped out', () async {
final Picture picture = _drawPicture();

// Picture not clipped out, so we should see a `<flt-canvas>`
{
final SurfaceSceneBuilder builder = SurfaceSceneBuilder();
builder.pushOffset(0, 0);
builder.addPicture(Offset.zero, picture);
builder.pop();
html.HtmlElement content = builder.build().webOnlyRootElement;
expect(content.querySelectorAll('flt-picture').single.children, isNotEmpty);
}

// Picture fully clipped out, so we should not see a `<flt-canvas>`
{
final SurfaceSceneBuilder builder = SurfaceSceneBuilder();
builder.pushOffset(0, 0);
builder.pushClipRect(const Rect.fromLTRB(1000, 1000, 2000, 2000));
builder.addPicture(Offset.zero, picture);
builder.pop();
builder.pop();
html.HtmlElement content = builder.build().webOnlyRootElement;
expect(content.querySelectorAll('flt-picture').single.children, isEmpty);
}
});
}

typedef TestLayerBuilder = EngineLayer Function(
Expand Down