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

Conversation

@jonahwilliams
Copy link
Contributor

This reduces the shader bootstrap count by 15.

Instead of using the regular advanced blend filters for BlendFilters, start a render pass and use the framebuffer advanced blend.

part of flutter/flutter#143540

@jonahwilliams jonahwilliams marked this pull request as ready for review April 22, 2024 17:24
#include <memory>
#include <optional>

#include "fml/logging.h"
Copy link
Member

Choose a reason for hiding this comment

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

Nit: flutter/fml/logging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

<< "The ColorBurned texture wasn't allocated (100x100 scales up 2x)";
}

// glyph_atlas_pipelines_.CreateDefault(
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Did you mean to leave in this checked in comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

/// blend filters.
///
/// This allows a substantial reduction in the number of bootstrapped filters.
std::optional<Entity> CreateFramebufferAdvancedBlend(
Copy link
Member

Choose a reason for hiding this comment

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

Also document the SupportsFramebufferFetch requirement?

Copy link
Member

Choose a reason for hiding this comment

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

Framebuffer advanced blends vs the other kind (pipeline blend?) I think are our own inventions of terminology (which totally make sense). But it would be good to point it out either here or in a glossary.

framebuffer_blend_softlight_pipelines_.CreateDefault(
*context_, options_trianglestrip,
{static_cast<Scalar>(BlendSelectValues::kSoftLight), supports_decal});
} else {
Copy link
Member

@chinmaygarde chinmaygarde Apr 22, 2024

Choose a reason for hiding this comment

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

Oh, I thought we were always doing this. So this is is only ever relevant on OpenGL ES and the desktop Metal (which I keep forgetting exists) stuff right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And iOS simulator!

@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #52284 at sha 180c3fa

@jonahwilliams
Copy link
Contributor Author

Hey golden diffs! Cool I have something to look at now.

@flutter-dashboard
Copy link

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #52284 at sha 6fc0314

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 23, 2024
@auto-submit auto-submit bot merged commit c393b9f into flutter:main Apr 23, 2024
@jonahwilliams jonahwilliams deleted the combine_blends branch April 23, 2024 18:26
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 23, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Apr 23, 2024
…147258)

flutter/engine@f23cc4d...b686508

2024-04-23 [email protected] Roll Fuchsia Linux SDK from L533ubzhjWwW7jxbc... to le_-uFgRD5DjvvqgL... (flutter/engine#52329)
2024-04-23 [email protected] [Impeller] only use framebuffer advanced blends if available. (flutter/engine#52284)
2024-04-23 [email protected] [Impeller] drawVertices uber shader. (flutter/engine#52315)
2024-04-23 [email protected] Roll Dart SDK from acd54f86bdbb to 6a670b60eb06 (1 revision) (flutter/engine#52327)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from L533ubzhjWwW to le_-uFgRD5Dj

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],[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
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 will affect goldens

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants