Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement save and restore functions on mock canvas so their effects can be tested #3

Merged
merged 1 commit into from Nov 27, 2021

Conversation

luanpotter
Copy link
Member

@luanpotter luanpotter commented Nov 27, 2021

I'm writing a PR on flame to fix an issue I found where components are not properly rendered in priority order if they are HUD.
Since that involves checking if the camera is applied or not, I am modifying this so that not only the save/restore count as number is saved, but the transformations are actually applied.

This has small implications to all our other existing tests, basically because of two things:

  1. we always clear up the camera at the end of the render phase, which requires a new assertion for tests using matchExactly:
    before:
 .. camera is set
 .. your render checks

after:

 .. camera is set
 .. your render checks
 .. camera is cleared
  1. since we collapse adjacent transform operations, in some camera tests we were not actually rendering any component, and just checking that the camera transform is applied; however due to the collapse and my optimization on the render flow, the camera was no longer applied needlessly, so all camera tests had to be modified to actually have a component (most already had)

For the context of how this is used, check flame-engine/flame#1148

@@ -78,7 +82,7 @@ class MockCanvas extends Fake implements Canvas, Matcher {

@override
Description describe(Description description) {
description.add('Canvas$_commands');
description.add('Canvas {\n${_commands.join('\n')}\n}');
Copy link
Member Author

Choose a reason for hiding this comment

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

I just found that its much easier to read like this when there are a handful of commands

@@ -6,26 +6,31 @@ import 'command.dart';
/// `canvas.translate()`, `canvas.rotate()`, `canvas.scale()`, or
/// `canvas.transform()`.
class TransformCommand extends CanvasCommand {
TransformCommand() : _transform = Matrix4.identity();
final Matrix4 matrix;
Copy link
Member Author

Choose a reason for hiding this comment

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

a public transform conflicts with the method name so I changed to matrix

@luanpotter luanpotter merged commit 5eaf2ac into main Nov 27, 2021
@luanpotter luanpotter deleted the luan.save-restore branch November 27, 2021 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants