Issue 12516 in skia: Apply dithering for drawImage() if it's 565 instead of using texture op

44 views
Skip to first unread message

micha… via monorail

unread,
Oct 8, 2021, 3:41:23 PM10/8/21
to bu...@skia.org
Status: Accepted
Owner: michae...@google.com
Area: GPU
Priority: Medium
Type: Defect

New issue 12516 by michae...@google.com: Apply dithering for drawImage() if it's 565 instead of using texture op
https://bugs.chromium.org/p/skia/issues/detail?id=12516

If we go straight to the TextureOp, dithering is not applied as a fragment processor. For most higher-bit color types this isn't noticeable but if the texture is 565 we should dither if the paint requested dithering.

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Git Watcher via monorail

unread,
Feb 15, 2022, 2:00:10 PM2/15/22
to bu...@skia.org

Comment #1 on issue 12516 by Git Watcher: Apply dithering for drawImage() if it's 565 instead of using texture op
https://bugs.chromium.org/p/skia/issues/detail?id=12516#c1

The following revision refers to this bug:
https://skia.googlesource.com/skia/+/10241484aa076ee0db8994bd16bd9a870abf083e

commit 10241484aa076ee0db8994bd16bd9a870abf083e
Author: Michael Ludwig <michae...@google.com>
Date: Tue Feb 15 17:44:01 2022

Fix dithering on drawImage[Rect] calls

Bug: skia:12516
Change-Id: Ibb68558e0f474143df3c6a5fb0fb3d4881529226
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/508677
Reviewed-by: Brian Osman <brian...@google.com>
Commit-Queue: Michael Ludwig <michae...@google.com>

[modify] https://crrev.com/10241484aa076ee0db8994bd16bd9a870abf083e/src/gpu/v1/Device_drawTexture.cpp
[modify] https://crrev.com/10241484aa076ee0db8994bd16bd9a870abf083e/src/core/SkPaintPriv.cpp
[modify] https://crrev.com/10241484aa076ee0db8994bd16bd9a870abf083e/src/core/SkMatrix.cpp
[modify] https://crrev.com/10241484aa076ee0db8994bd16bd9a870abf083e/gm/image.cpp
[modify] https://crrev.com/10241484aa076ee0db8994bd16bd9a870abf083e/src/core/SkPaintPriv.h
[modify] https://crrev.com/10241484aa076ee0db8994bd16bd9a870abf083e/src/gpu/SkGr.cpp

micha… via monorail

unread,
Feb 16, 2022, 7:51:27 PM2/16/22
to bu...@skia.org
Updates:
Status: Fixed

Comment #2 on issue 12516 by michae...@google.com: Apply dithering for drawImage() if it's 565 instead of using texture op
https://bugs.chromium.org/p/skia/issues/detail?id=12516#c2

(No comment was entered for this change.)

Git Watcher via monorail

unread,
Feb 17, 2022, 12:45:08 PM2/17/22
to bu...@skia.org

Comment #3 on issue 12516 by Git Watcher: Apply dithering for drawImage() if it's 565 instead of using texture op
https://bugs.chromium.org/p/skia/issues/detail?id=12516#c3


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/05a4b12dbdf279280c691be0ccf31b718bde10c7

commit 05a4b12dbdf279280c691be0ccf31b718bde10c7
Author: Michael Ludwig <michae...@google.com>
Date: Thu Feb 17 16:45:22 2022

Don't dither const shaders that don't have a GP override

https://gold.skia.org/detail?test=gradients_no_texture&digest=008c796df4da7fbb6718e96db2c36c90

If paintFP was only non-null because it came from the main SkShader, we
should defer to SkPaintPriv's check that the shader is not a constant.
We don't have a isConstant() for FPs so this assumes that if the GP
overrides it with a new FP it probably is doing something spatially
interesting and does need the dithering.

Bug: skia:12516
Change-Id: I7935ba8f0baf91e7b7ba07280452871c60ffd3e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/509996
Commit-Queue: Michael Ludwig <michae...@google.com>
Auto-Submit: Michael Ludwig <michae...@google.com>
Reviewed-by: Brian Osman <brian...@google.com>
Commit-Queue: Brian Osman <brian...@google.com>

[modify] https://crrev.com/05a4b12dbdf279280c691be0ccf31b718bde10c7/src/gpu/SkGr.cpp

Git Watcher via monorail

unread,
Feb 22, 2022, 6:12:09 PM2/22/22
to bu...@skia.org

Comment #4 on issue 12516 by Git Watcher: Apply dithering for drawImage() if it's 565 instead of using texture op
https://bugs.chromium.org/p/skia/issues/detail?id=12516#c4


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/fd1f1b3503419fee853b9fd4e25eaa9958d74e71

commit fd1f1b3503419fee853b9fd4e25eaa9958d74e71
Author: Brian Osman <brian...@google.com>
Date: Tue Feb 22 22:28:51 2022

Revert "Don't dither const shaders that don't have a GP override"

This reverts commit 05a4b12dbdf279280c691be0ccf31b718bde10c7.

Reason for revert: Speculative fix for b/220676062

Original change's description:

> Don't dither const shaders that don't have a GP override
>
> https://gold.skia.org/detail?test=gradients_no_texture&digest=008c796df4da7fbb6718e96db2c36c90
>
> If paintFP was only non-null because it came from the main SkShader, we
> should defer to SkPaintPriv's check that the shader is not a constant.
> We don't have a isConstant() for FPs so this assumes that if the GP
> overrides it with a new FP it probably is doing something spatially
> interesting and does need the dithering.
>
> Bug: skia:12516
> Change-Id: I7935ba8f0baf91e7b7ba07280452871c60ffd3e8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/509996
> Commit-Queue: Michael Ludwig <michae...@google.com>
> Auto-Submit: Michael Ludwig <michae...@google.com>
> Reviewed-by: Brian Osman <brian...@google.com>
> Commit-Queue: Brian Osman <brian...@google.com>

Bug: skia:12516
Change-Id: I9c45f457d42cd5890cc110617e1636ffa6669503
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/511807
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>
Commit-Queue: Brian Osman <brian...@google.com>

[modify] https://crrev.com/fd1f1b3503419fee853b9fd4e25eaa9958d74e71/src/gpu/SkGr.cpp

Git Watcher via monorail

unread,
Feb 22, 2022, 6:18:10 PM2/22/22
to bu...@skia.org

Comment #5 on issue 12516 by Git Watcher: Apply dithering for drawImage() if it's 565 instead of using texture op
https://bugs.chromium.org/p/skia/issues/detail?id=12516#c5


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/1e420a3652562bcc59a634adcda5ebc4d49054c9

commit 1e420a3652562bcc59a634adcda5ebc4d49054c9
Author: Brian Osman <brian...@google.com>
Date: Tue Feb 22 22:24:41 2022

Revert "Fix dithering on drawImage[Rect] calls"

This reverts commit 10241484aa076ee0db8994bd16bd9a870abf083e.

Reason for revert: Speculative revert for b/220676062

Original change's description:

> Fix dithering on drawImage[Rect] calls
>
> Bug: skia:12516
> Change-Id: Ibb68558e0f474143df3c6a5fb0fb3d4881529226
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/508677
> Reviewed-by: Brian Osman <brian...@google.com>
> Commit-Queue: Michael Ludwig <michae...@google.com>

Bug: skia:12516
Change-Id: If496c6be355bbb026ffe478512f8a6334116bb66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/511806
Commit-Queue: Brian Osman <brian...@google.com>
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>

[modify] https://crrev.com/1e420a3652562bcc59a634adcda5ebc4d49054c9/src/gpu/v1/Device_drawTexture.cpp
[modify] https://crrev.com/1e420a3652562bcc59a634adcda5ebc4d49054c9/src/core/SkPaintPriv.cpp
[modify] https://crrev.com/1e420a3652562bcc59a634adcda5ebc4d49054c9/src/core/SkMatrix.cpp
[modify] https://crrev.com/1e420a3652562bcc59a634adcda5ebc4d49054c9/gm/image.cpp
[modify] https://crrev.com/1e420a3652562bcc59a634adcda5ebc4d49054c9/src/core/SkPaintPriv.h
[modify] https://crrev.com/1e420a3652562bcc59a634adcda5ebc4d49054c9/src/gpu/SkGr.cpp

Git Watcher via monorail

unread,
Feb 22, 2022, 6:19:10 PM2/22/22
to bu...@skia.org
Updates:
Labels: merge-merged-next-release

Comment #6 on issue 12516 by Git Watcher: Apply dithering for drawImage() if it's 565 instead of using texture op
https://bugs.chromium.org/p/skia/issues/detail?id=12516#c6


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/046e3f4b42bff2d2d8f5888847aa0741edb9d249

commit 046e3f4b42bff2d2d8f5888847aa0741edb9d249
Author: Brian Osman <brian...@google.com>

Date: Tue Feb 22 22:28:51 2022

Revert "Don't dither const shaders that don't have a GP override"

This reverts commit 05a4b12dbdf279280c691be0ccf31b718bde10c7.

Reason for revert: Speculative fix for b/220676062

Original change's description:

> Don't dither const shaders that don't have a GP override
>
> https://gold.skia.org/detail?test=gradients_no_texture&digest=008c796df4da7fbb6718e96db2c36c90
>
> If paintFP was only non-null because it came from the main SkShader, we
> should defer to SkPaintPriv's check that the shader is not a constant.
> We don't have a isConstant() for FPs so this assumes that if the GP
> overrides it with a new FP it probably is doing something spatially
> interesting and does need the dithering.
>
> Bug: skia:12516
> Change-Id: I7935ba8f0baf91e7b7ba07280452871c60ffd3e8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/509996
> Commit-Queue: Michael Ludwig <michae...@google.com>
> Auto-Submit: Michael Ludwig <michae...@google.com>
> Reviewed-by: Brian Osman <brian...@google.com>

> Commit-Queue: Brian Osman <brian...@google.com>

Bug: skia:12516
Change-Id: I9c45f457d42cd5890cc110617e1636ffa6669503

Git Watcher via monorail

unread,
Feb 22, 2022, 6:40:16 PM2/22/22
to bu...@skia.org

Comment #7 on issue 12516 by Git Watcher: Apply dithering for drawImage() if it's 565 instead of using texture op
https://bugs.chromium.org/p/skia/issues/detail?id=12516#c7


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/9f83d57ba97dd23e19a0743f216bf17a97b4a5b2

commit 9f83d57ba97dd23e19a0743f216bf17a97b4a5b2
Author: Brian Osman <brian...@google.com>

Date: Tue Feb 22 22:24:41 2022

Revert "Fix dithering on drawImage[Rect] calls"

This reverts commit 10241484aa076ee0db8994bd16bd9a870abf083e.

Reason for revert: Speculative revert for b/220676062

Original change's description:

> Fix dithering on drawImage[Rect] calls
>
> Bug: skia:12516
> Change-Id: Ibb68558e0f474143df3c6a5fb0fb3d4881529226
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/508677
> Reviewed-by: Brian Osman <brian...@google.com>

> Commit-Queue: Michael Ludwig <michae...@google.com>

Bug: skia:12516
Change-Id: If496c6be355bbb026ffe478512f8a6334116bb66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/511806
Commit-Queue: Brian Osman <brian...@google.com>
Bot-Commit: Rubber Stamper <rubber-...@appspot.gserviceaccount.com>

micha… via monorail

unread,
Mar 1, 2022, 11:10:17 AM3/1/22
to bu...@skia.org
Updates:
Status: Accepted

Comment #8 on issue 12516 by michae...@google.com: Apply dithering for drawImage() if it's 565 instead of using texture op
https://bugs.chromium.org/p/skia/issues/detail?id=12516#c8

Re-opening since we have to figure out what's going on in Android with this change.
Reply all
Reply to author
Forward
0 new messages