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

feat: Implement Snapshot mixin on PositionComponent #2695

Merged

Conversation

projectitis
Copy link
Contributor

Description

This implements a Snapshot mixin on PositionComponent that allows:

  1. A component to be rendered once then cached (for performance)
  2. and/or a Picture or Image snapshot to be taken at any time

Internally the snapshot is cached and redrawn as a Picture. After testing with both Image and Picture I found that Image had no performance improvement, and the rendering quality was lower.

A helper method snapshotToImage was added to allow the user better control of generating an Image from the internal Picture snapshot if that is desired.

See the documentation in this PR for more details.

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

Closes #2693

@projectitis projectitis changed the title feature: Implement Snapshot mixin on PositionComponent feat: Implement Snapshot mixin on PositionComponent Aug 31, 2023
@projectitis
Copy link
Contributor Author

projectitis commented Aug 31, 2023

I think there are differences between the rendering of the github runner and my own computer.

The 3 golden tests I wrote are failing with a 2-3% margin (i.e. quite small). I also have other golden tests failing on my local machine for other tests. Most of them with very small margins (e.g. 0.5%).

@spydon
Copy link
Member

spydon commented Aug 31, 2023

I think there are differences between the rendering of the github runner and my own computer.

The 3 golden tests I wrote are failing with a 2-3% margin (i.e. quite small). I also have other golden tests failing on my local machine for other tests. Most of them with very small margins (e.g. 0.5%).

Yup, they have to be generated on a Linux machine. I can generate them for you if you want.

@spydon
Copy link
Member

spydon commented Aug 31, 2023

Updated the goldens, won't have time to review until Wednesday next week though (Flutter & Friends starts on Sunday, so it's a loooot to organize 😅).

@projectitis
Copy link
Contributor Author

Wow, what an amazing event! Great speaker line up. Good luck with the workshop!

@spydon
Copy link
Member

spydon commented Aug 31, 2023

Great speaker line up.

I know right, I picked them myself. 😂

@projectitis
Copy link
Contributor Author

I wish I wasn't so far away. Would love to attend :)

@projectitis
Copy link
Contributor Author

Updated the goldens

They look good! Thank you.

@spydon
Copy link
Member

spydon commented Sep 2, 2023

Apparently our pipeline was a bit broken, there are some very minor analyze issues in this PR (missing trailing commas)

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Thanks for a very high quality PR!
Just some small nitpicks, overall it looks very good.

doc/flame/rendering/layers.md Show resolved Hide resolved
doc/flame/rendering/layers.md Outdated Show resolved Hide resolved
doc/flame/rendering/layers.md Outdated Show resolved Hide resolved
doc/flame/rendering/layers.md Outdated Show resolved Hide resolved
doc/flame/rendering/layers.md Outdated Show resolved Hide resolved
doc/flame/rendering/layers.md Outdated Show resolved Hide resolved
packages/flame/lib/src/components/mixins/snapshot.dart Outdated Show resolved Hide resolved
packages/flame/lib/src/components/mixins/snapshot.dart Outdated Show resolved Hide resolved
packages/flame/test/components/mixins/snapshot_test.dart Outdated Show resolved Hide resolved
packages/flame/test/components/mixins/snapshot_test.dart Outdated Show resolved Hide resolved
@projectitis
Copy link
Contributor Author

ok, ready for another look when you have time :)

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Lgtm! Thanks for your contribution

@projectitis
Copy link
Contributor Author

Sorry, I found some documentation errors I introduced :(


Layers and snapshots share some common features, including the ability to pre-render and cache
objects for improved performance. However, they also have unique features which make them better
suited for different use-cases.
Copy link
Member

Choose a reason for hiding this comment

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

while I think both sections below do a good job of describing each feature in a vacuum, I think this paragraph could give more details about what are the differences and what rationale I should use to choose one or the other

Copy link
Contributor Author

@projectitis projectitis Sep 11, 2023

Choose a reason for hiding this comment

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

I've added a description. Please amend it if you don't agree!

To be honest, I'm not sure of when you might use a Layer instead of a Snapshot, unless you don't want the overhead of a position component. Layers support processors (of which there is only 1 so far, but more could be made). However processors are sort of doubling-up on Effects?

This is what I have (below) -

Copy link
Contributor Author

@projectitis projectitis Sep 11, 2023

Choose a reason for hiding this comment

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

Layers and Snapshots

Layers and snapshots share some common features, including the ability to pre-render and cache
objects for improved performance. However, they also have unique features which make them better
suited for different use-cases.

Snapshot is a mixin that can be added to any PositionComponent. Use this for:

  • Mixing in to existing game objects (that are PositionComponents).
  • Caching game objects, such as sprites, that are complex to render.
  • Drawing the same object many times without rendering it each time.
  • Capturing an image snapshot to save as a screenshot (for example).

Layer is a class. Use or extend this class for:

  • Structuring your game with logical layers (e.g. UI, foreground, main, background).
  • Grouping objects to form a complex scene, and then caching it (e.g. a background layer).
  • Processor support. Layers allow user-defined processors to run pre- and post- render.

Copy link
Member

Choose a reason for hiding this comment

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

Let's keep it like that for now and then we can extend on that in a follow-up. :)

Copy link
Member

@luanpotter luanpotter left a comment

Choose a reason for hiding this comment

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

LGTM, really cool idea! Can we followup with changing TextBoxComponent to use this internally? I think it vastly simplify its code!

@spydon spydon enabled auto-merge (squash) September 12, 2023 20:56
@spydon spydon merged commit c1ee24a into flame-engine:main Sep 12, 2023
7 checks passed
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.

feature: Snapshot a component (like a layer, but not)
3 participants