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

Conversation

@jonahwilliams
Copy link
Contributor

For non-aa axis aligned difference clip rects or aa axis aligned difference clips that are very nearly integral, just use the scissor update and don't perform the depth write at all.

if (!clip_coverage.is_difference_or_non_square &&
clip_coverage.coverage.has_value()) {
const Rect& coverage = clip_coverage.coverage.value();
constexpr Scalar threshold = 0.2;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

The default 4x MSAA sample positions are 2/16 (0.125) pixels away from the edges. So maybe go with something a bit smaller, like 0.124?

image

if (!clip_coverage.is_difference_or_non_square &&
clip_coverage.coverage.has_value()) {
const Rect& coverage = clip_coverage.coverage.value();
constexpr Scalar threshold = 0.2;
Copy link
Member

Choose a reason for hiding this comment

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

The default 4x MSAA sample positions are 2/16 (0.125) pixels away from the edges. So maybe go with something a bit smaller, like 0.124?

image

threshold)) {
subpass_state.clip_coverage.push_back(ClipCoverageLayer{
.coverage = Rect::Round(clip_coverage.coverage.value()), //
.clip_height = previous_clip_height + 1 //
Copy link
Member

Choose a reason for hiding this comment

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

Don't we still need to track the clip coverage layer so that the scissor will get set during clip replays?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah good point, let me refactor this so I can just make should_render conditional.

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

@jonahwilliams jonahwilliams requested a review from bdero October 23, 2024 18:16
Copy link
Member

@bdero bdero left a comment

Choose a reason for hiding this comment

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

LGTM!

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 23, 2024
@auto-submit auto-submit bot merged commit 925b0a6 into flutter:main Oct 23, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 23, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 23, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 24, 2024
…157478)

flutter/engine@73c54fa...5d7caf7

2024-10-23 [email protected] Delete v1 android engine embedding (flutter/engine#52022)
2024-10-23 [email protected] [ios][platform_view]force reset forwarding recognizer state when its stuck (flutter/engine#55958)
2024-10-23 [email protected] Allow running the YAPF formatter using Python version 3.11 (flutter/engine#56062)
2024-10-23 [email protected] [Impeller] optimize clip rects by checking if integral coverage is sufficient. (flutter/engine#56041)
2024-10-23 [email protected] Switch the web compilation step to use an AOT snapshot (flutter/engine#56040)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
M97Chahboun pushed a commit to M97Chahboun/flutter that referenced this pull request Oct 30, 2024
…lutter#157478)

flutter/engine@73c54fa...5d7caf7

2024-10-23 [email protected] Delete v1 android engine embedding (flutter/engine#52022)
2024-10-23 [email protected] [ios][platform_view]force reset forwarding recognizer state when its stuck (flutter/engine#55958)
2024-10-23 [email protected] Allow running the YAPF formatter using Python version 3.11 (flutter/engine#56062)
2024-10-23 [email protected] [Impeller] optimize clip rects by checking if integral coverage is sufficient. (flutter/engine#56041)
2024-10-23 [email protected] Switch the web compilation step to use an AOT snapshot (flutter/engine#56040)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
…fficient. (flutter/engine#56041)

For non-aa axis aligned difference clip rects or aa axis aligned difference clips that are very nearly integral, just use the scissor update and don't perform the depth write at all.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App e: impeller

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants