[Impeller] drawAtlas blend mode. #38335

jonahwilliams · 2022-12-15T22:40:37Z

Complete support for all blend modes in drawAtlas. Currently investigating if 1) we can share these blend mode shaders with drawVertices 2) if we can skip the normal blends by drawing colored trianges and then blending the image.

[Impeller] Implement blend param in DrawVertices and DrawAtlas flutter#108047
[impeller] support for ImageShader / texture coordinates on drawVertices flutter#109956

impeller/entity/contents/atlas_contents.cc

impeller/entity/contents/vertices_contents.cc

cmkweber · 2023-01-12T01:04:35Z

Here is an example of this not quite working:

// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'dart:ui' as ui;
import 'package:flutter/material.dart';

void main() =>
  runApp(MaterialApp(
    home: Scaffold(
      body:CustomPaint(painter: FooPainter())
    )
  ));


class FooPainter extends CustomPainter {
  final ui.Vertices vertices = ui.Vertices(
    VertexMode.triangles,
    <Offset>[
      Offset(0, 0),
      Offset(0, 100),
      Offset(100, 100),
      Offset(50, 50),
      Offset(50, 150),
      Offset(150, 150),
    ],
    colors: [
      Colors.red,
      Colors.green,
      Colors.blue,
      Colors.red,
      Colors.green,
      Colors.blue,
    ].map((x) => x.withAlpha(255)).toList(),
  );

  @override
  void paint(ui.Canvas canvas, ui.Size size) {
    canvas.drawVertices(vertices, BlendMode.srcATop, Paint()
    // ..color = Colors.black.withAlpha(100)
    ..shader = LinearGradient(colors: [
      Colors.yellow,
      Colors.green,
    ]).createShader(Rect.fromLTWH(0, 0, size.width, size.height)));
  }

  @override
  bool shouldRepaint(covariant CustomPainter oldDelegate) {
    return true;
  }
}

Could it be that you are creating the gradient shader at the painters size (size.width, size.height) instead of the triangles size (150, 150) ?

jonahwilliams · 2023-01-12T01:30:09Z

Yeah, the shader was a bit out of bounds, however the biggest issues was that I was accidentally dividing twice here: 0c23ff4 which made all the UVs tiny.

jonahwilliams · 2023-01-12T03:36:22Z

TODO:

texcoordinates
Always treat src color as fully opaque and pull out opacity according to https://api.skia.org/classSkCanvas.html#ae971eebc0c442a0310b830a7711b8798

jonahwilliams · 2023-01-12T04:14:33Z

I cheated a bit for src blend modes but that might have bitten me for supporting texture coordinates.

jonahwilliams · 2023-01-12T04:16:58Z

TODO figure out if texture coordinates apply if there are no per-vertex colors (I would think they do)

jonahwilliams · 2023-01-12T18:15:47Z

updates: texcoordinates should be treated as being in the coordinate space of whatever is being sampled and not already normalized 0-1. It should apply on src and it should apply if there are no colors, but not if there is only a solid color paint obviously

jonahwilliams · 2023-01-12T18:19:54Z

also as an optimization we should avoid calling makeSnapshot if the paint shader just contains an image

jonahwilliams · 2023-01-19T03:08:59Z

I've udpated this to make alpha work correctly for the pipeline blends. Seems inefficient for a lot of the src blend modes if we know the src image has no opacity. Do we have any tags/data on our images to be able to tell this?

jonahwilliams · 2023-01-19T03:10:27Z

I guess a fair question would be why even use the colors and srcOver if you know your image has no alpha.

bdero · 2023-01-20T00:10:37Z

Do we have any tags/data on our images to be able to tell this?

Huh, I feel like @chinmaygarde and I have briefly mentioned doing this several times, but I guess we never made a bug about it. Added one here: flutter/flutter#118854

jonahwilliams · 2023-01-20T00:43:37Z

It would probably useful for tracking in general. For example on the vertices blend modes, we could apply similar optimziations based on knowing if the color source has any transparency. Probably low priority compared to other optimizations though

bdero · 2023-01-20T00:41:18Z

impeller/entity/contents/filters/color_filter_contents.h

 private:
  bool absorb_opacity_ = false;
+  std::optional<Scalar> alpha_;


I think this is no longer being used. Remove?

This is used now

bdero · 2023-01-20T00:42:30Z

impeller/entity/contents/content_context.h

@@ -27,8 +27,7 @@
 #include "impeller/entity/advanced_blend_saturation.frag.h"
 #include "impeller/entity/advanced_blend_screen.frag.h"
 #include "impeller/entity/advanced_blend_softlight.frag.h"
-#include "impeller/entity/atlas_fill.frag.h"
-#include "impeller/entity/atlas_fill.vert.h"
+


Nit: Extra newline.

bdero · 2023-01-20T00:58:05Z

impeller/entity/contents/atlas_contents.cc

+    }
+    auto texture_contents = TextureContents::MakeRect(coverage);
+    texture_contents->SetTexture(subpass_texture);
+    texture_contents->SetOpacity(alpha_);


When the filter graph is rendered, it will already perform a separate blit if necessary to apply the entity properties when drawing to the render pass, so this extra subpass is almost redundant; just doing filter_contents->Render(renderer, entity, pass); would nearly work without the subpass.

The one thing that wouldn't work is making this alpha apply on the final blit as you're doing here. But, we already have a mechanism to propagate alpha to the final blit in the filter graph and can trivially add support to BlendFilterContents by repurposing the new/unused SetAlpha setter (added another comment about this).

bdero · 2023-01-20T01:17:24Z

impeller/entity/contents/filters/blend_filter_contents.cc

@@ -109,7 +110,8 @@ static std::optional<Snapshot> AdvancedBlend(
    auto sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler({});
    FS::BindTextureSamplerDst(cmd, dst_snapshot->texture, sampler);
    blend_info.dst_y_coord_scale = dst_snapshot->texture->GetYCoordScale();
-    blend_info.dst_input_alpha = absorb_opacity ? dst_snapshot->opacity : 1.0f;
+    blend_info.dst_input_alpha =
+        (absorb_opacity ? dst_snapshot->opacity : 1.0) * alpha.value_or(1.0);


Since this alpha value is no longer being used by AtlasContents, we could repurpose it to just forward down the filter chain by multiplying it into the returned Snapshot::opacity, which would allow us to remove the need for the extra subpass as mentioned in my other comment (this snapshot alpha forwarding can be supported for both the pipeline and advanced blend case with no inconsistencies/ill effects).

ahh, Yeah that is what I missed. I had a similar technique for the pipeline blends which didn't work since it would just blend the alpha with one of the inputs. Will fix so both are applied to the snapshot.

jonahwilliams · 2023-01-20T20:02:53Z

Filled flutter/flutter#118903 for the blend issue, I verified by modifying the shader the the dst needs to be in the second slot for advanced blends, but the first for pipeline blends. Don't see the issue elsewhere though

jonahwilliams · 2023-01-20T22:29:25Z

This is updated now, PTAL @bdero

jonahwilliams · 2023-01-20T23:27:33Z

Filled flutter/flutter#118914 for the remaing alpha issue

bdero

LGTM!

jonahwilliams · 2023-01-24T19:53:09Z

yaaaay!

chinmaygarde · 2023-01-24T23:07:49Z

🎉 We can update the section about this in go/impeller-wiki#status

…119089) * 334c92e56 [Impeller] drawAtlas blend mode. (flutter/engine#38335) * 2499a5d9f Roll Fuchsia Mac SDK from HxpwvvbQdk54L6_8q... to MUvFS0baOnigVUIND... (flutter/engine#39105)

* fill out drawAtlas * ++ * add drawVertices uage * ++ * test fixes * dont double divide * CI cleanups * some texture coordinates work * ++ * basic texture coordiantes work * ++ * ++ * ++ * Delete color blending * back out vertices change * ++ * ++ * ++ * remove atlas_fill from licenses * blending cleanup * Use sub-contents * fix opacity for pipeline blends * use alpha in snapshot to avoid extra pass * ++ * ++ * fix order

fill out drawAtlas

ff6f4de

jonahwilliams commented Dec 15, 2022

View reviewed changes

impeller/entity/contents/atlas_contents.cc Outdated Show resolved Hide resolved

jonahwilliams self-assigned this Dec 15, 2022

jonahwilliams added the e: impeller label Dec 15, 2022

chinmaygarde changed the title ~~[WIP] Complete drawAtlas support~~ [WIP] [Impeller] Complete drawAtlas support. Dec 16, 2022

jonahwilliams mentioned this pull request Dec 16, 2022

[Impeller] move Atlas geometry calculation to geometry class #38360

Closed

jonahwilliams added 5 commits January 10, 2023 16:58

Merge branch 'master' of github.com:flutter/engine into drawAtlas

b2cc708

++

56bf47d

add drawVertices uage

683e306

++

8a1caa1

test fixes

1a00d10

jonahwilliams commented Jan 11, 2023

View reviewed changes

impeller/entity/contents/vertices_contents.cc Outdated Show resolved Hide resolved

jonahwilliams changed the title ~~[WIP] [Impeller] Complete drawAtlas support.~~ [Impeller] drawAtlas and drawVertices blend mode. Jan 11, 2023

jonahwilliams marked this pull request as ready for review January 11, 2023 20:41

This comment was marked as outdated.

Sign in to view

jonahwilliams mentioned this pull request Jan 11, 2023

[Impeller] support vertices per-vertex color blending with all color sources #38644

Closed

chinmaygarde self-requested a review January 11, 2023 21:55

chinmaygarde mentioned this pull request Jan 11, 2023

[Impeller] Allow non-FIFO order asynchronous construction of PSO. flutter/flutter#118353

Closed

dont double divide

0c23ff4

CI cleanups

2a27640

jonahwilliams added 2 commits January 11, 2023 20:05

some texture coordinates work

79c13e5

++

785e18b

basic texture coordiantes work

a3a9476

jonahwilliams requested a review from bdero January 19, 2023 18:16

bdero reviewed Jan 20, 2023

View reviewed changes

jonahwilliams added 4 commits January 19, 2023 18:52

use alpha in snapshot to avoid extra pass

b3b51b2

++

b422933

++

c3aa8ce

Merge branch 'master' of github.com:flutter/engine into drawAtlas

979d1ef

jonahwilliams mentioned this pull request Jan 20, 2023

[Impeller] src and dst are reversed for advanced blends in FilterContents::MakeBlend Constructor flutter/flutter#118903

Closed

jonahwilliams added 2 commits January 20, 2023 14:26

Merge branch 'master' of github.com:flutter/engine into drawAtlas

417ee4f

fix order

4837acc

bdero approved these changes Jan 24, 2023

View reviewed changes

Merge branch 'master' of github.com:flutter/engine into drawAtlas

53e9c23

jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 24, 2023

auto-submit bot merged commit 334c92e into flutter:main Jan 24, 2023

engine-flutter-autoroll mentioned this pull request Jan 24, 2023

Roll Flutter Engine from 59ea78bfabda to 2499a5d9fca7 (2 revisions) flutter/flutter#119089

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 24, 2023

334c92e56 [Impeller] drawAtlas blend mode. (flutter/engine#38335)

1a40bdb

jonahwilliams deleted the drawAtlas branch January 24, 2023 23:24

[Impeller] drawAtlas blend mode. #38335

[Impeller] drawAtlas blend mode. #38335

Uh oh!

Conversation

jonahwilliams commented Dec 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

cmkweber commented Jan 12, 2023

Uh oh!

jonahwilliams commented Jan 12, 2023

Uh oh!

jonahwilliams commented Jan 12, 2023

Uh oh!

jonahwilliams commented Jan 12, 2023

Uh oh!

jonahwilliams commented Jan 12, 2023

Uh oh!

jonahwilliams commented Jan 12, 2023

Uh oh!

jonahwilliams commented Jan 12, 2023

Uh oh!

jonahwilliams commented Jan 19, 2023

Uh oh!

jonahwilliams commented Jan 19, 2023

Uh oh!

bdero commented Jan 20, 2023

Uh oh!

jonahwilliams commented Jan 20, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Jan 20, 2023

Uh oh!

jonahwilliams commented Jan 20, 2023

Uh oh!

jonahwilliams commented Jan 20, 2023

Uh oh!

bdero left a comment

Choose a reason for hiding this comment

Uh oh!

jonahwilliams commented Jan 24, 2023

Uh oh!

chinmaygarde commented Jan 24, 2023

Uh oh!

Uh oh!

jonahwilliams commented Dec 15, 2022 •

edited

Loading