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

Conversation

@matanlurey
Copy link
Contributor

Partial work towards flutter/flutter#131450.


Run the Playground locally:

$ENGINE/out/host_debug_unopt/impeller_unittests \
  --enable_playground \
  --gtest_filter="*CanRenderLinearGradientWithDithering*"

Summary of changes:

  • Added a playground/test for dithering disabled and enabled.
  • Added bool dither to Impeller's Paint, but SetDither (use-facing) is ignored.
  • Converted Skia's dithering to GLSL and invoked it when dither is set.

Before

Screenshot 2023-07-31 at 1 29 16 PM

After

Screenshot 2023-07-31 at 1 29 25 PM

Deleted Scenes

Here are some of my earlier attempts that are fun to share :)

Screenshot 2023-07-31 at 11 35 07 AM

Screenshot 2023-07-31 at 12 12 38 PM

Screenshot 2023-07-31 at 12 19 56 PM

Screenshot 2023-07-31 at 12 25 59 PM

Screenshot 2023-07-31 at 12 44 08 PM

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM with nits

@chinmaygarde chinmaygarde changed the title Implement, non user-facing, dithering for LinearGradients in Impeller [Impeller] Implement, non user-facing, dithering for LinearGradients. Jul 31, 2023
@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 31, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 31, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Jul 31, 2023

auto label is removed for flutter/engine, pr: 44181, due to - The status or check suite Linux Web Framework tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

  • The status or check suite Linux Framework Smoke Tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

Still LGTM

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 1, 2023
@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 #44181 at sha a44306a

@auto-submit auto-submit bot merged commit 1433e23 into flutter:main Aug 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 1, 2023
@matanlurey matanlurey deleted the impeller-gradient-dither branch August 3, 2023 17:38
matanlurey added a commit that referenced this pull request Aug 8, 2023
…44331)

Partial work towards flutter/flutter#131450
similar to #44181.

---

Run the Playground locally:

```bash
$ENGINE/out/host_debug_unopt/impeller_unittests \
  --enable_playground \
  --gtest_filter="*CanRender*GradientWithDithering*"
```

## Radial

### Before

<img width="1014" alt="Screenshot 2023-08-03 at 10 08 53 AM"
src="https://github.com/flutter/engine/assets/168174/d53f2c0e-5c48-4ecb-8e67-d4ab28bfe488">

### After

<img width="1018" alt="Screenshot 2023-08-03 at 10 13 57 AM"
src="https://github.com/flutter/engine/assets/168174/3b6e6e65-3dd3-4cb3-9950-36e2ba5c1da2">

## Sweep

### Before

<img width="1019" alt="Screenshot 2023-08-03 at 10 27 35 AM"
src="https://github.com/flutter/engine/assets/168174/4e3bc82d-c0d5-43dd-952a-c11cb586fb65">

### After

<img width="1018" alt="Screenshot 2023-08-03 at 10 33 11 AM"
src="https://github.com/flutter/engine/assets/168174/7e526391-cfd7-4920-89ff-fe26793b24fc"

## Conical

### Before

<img width="1019" alt="Screenshot 2023-08-08 at 11 55 43 AM"
src="https://github.com/flutter/engine/assets/168174/944709f4-8163-4de3-bfc5-eaf30b978529">

### After

<img width="1016" alt="Screenshot 2023-08-08 at 1 11 40 PM"
src="https://github.com/flutter/engine/assets/168174/60ad67a4-b409-4136-a753-b8608f46fbf2">
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…flutter#44181)

Partial work towards flutter/flutter#131450.

---

Run the Playground locally:

```bash
$ENGINE/out/host_debug_unopt/impeller_unittests \
  --enable_playground \
  --gtest_filter="*CanRenderLinearGradientWithDithering*"
```

Summary of changes:

- Added a playground/test for dithering disabled and enabled.
- Added `bool dither` to Impeller's `Paint`, but `SetDither` (use-facing) is ignored.
- Converted [Skia's dithering](https://github.com/google/skia/blob/f9de059517a6f58951510fc7af0cba21e13dd1a8/src/opts/SkRasterPipeline_opts.h#L1717) to GLSL and invoked it when `dither` is set.

## Before

![Screenshot 2023-07-31 at 1 29 16 PM](https://github.com/flutter/engine/assets/168174/51d2f7a0-dc22-44fe-b7f9-990b826c5fd9)

## After

![Screenshot 2023-07-31 at 1 29 25 PM](https://github.com/flutter/engine/assets/168174/78da2efe-2c5d-438f-b7f7-d8eb092c6a2c)

## Deleted Scenes

<details>
<summary>Here are some of my earlier attempts that are fun to share :)</summary>

![Screenshot 2023-07-31 at 11 35 07 AM](https://github.com/flutter/engine/assets/168174/719f97fc-1a3d-4920-8687-486c4de28b79)

![Screenshot 2023-07-31 at 12 12 38 PM](https://github.com/flutter/engine/assets/168174/ed262f83-442f-484b-8288-30e8e0d5e768)

![Screenshot 2023-07-31 at 12 19 56 PM](https://github.com/flutter/engine/assets/168174/057b1f95-fbd2-4ae2-bb25-8dd967cf8466)

![Screenshot 2023-07-31 at 12 25 59 PM](https://github.com/flutter/engine/assets/168174/34857c6e-49cd-40c1-9e91-646b7bfbf97c)

![Screenshot 2023-07-31 at 12 44 08 PM](https://github.com/flutter/engine/assets/168174/3b272428-b5be-4ca5-8cfe-1b12062a64f4)

</details>
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…lutter#44331)

Partial work towards flutter/flutter#131450
similar to flutter#44181.

---

Run the Playground locally:

```bash
$ENGINE/out/host_debug_unopt/impeller_unittests \
  --enable_playground \
  --gtest_filter="*CanRender*GradientWithDithering*"
```

## Radial

### Before

<img width="1014" alt="Screenshot 2023-08-03 at 10 08 53 AM"
src="https://github.com/flutter/engine/assets/168174/d53f2c0e-5c48-4ecb-8e67-d4ab28bfe488">

### After

<img width="1018" alt="Screenshot 2023-08-03 at 10 13 57 AM"
src="https://github.com/flutter/engine/assets/168174/3b6e6e65-3dd3-4cb3-9950-36e2ba5c1da2">

## Sweep

### Before

<img width="1019" alt="Screenshot 2023-08-03 at 10 27 35 AM"
src="https://github.com/flutter/engine/assets/168174/4e3bc82d-c0d5-43dd-952a-c11cb586fb65">

### After

<img width="1018" alt="Screenshot 2023-08-03 at 10 33 11 AM"
src="https://github.com/flutter/engine/assets/168174/7e526391-cfd7-4920-89ff-fe26793b24fc"

## Conical

### Before

<img width="1019" alt="Screenshot 2023-08-08 at 11 55 43 AM"
src="https://github.com/flutter/engine/assets/168174/944709f4-8163-4de3-bfc5-eaf30b978529">

### After

<img width="1016" alt="Screenshot 2023-08-08 at 1 11 40 PM"
src="https://github.com/flutter/engine/assets/168174/60ad67a4-b409-4136-a753-b8608f46fbf2">
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

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants