Currently, the GPU-side image filter implementation creates
exact-match textures for the offscreen backing stores for
saveLayer(). This is because several filters have GPU
implementations which depend on the texture coordinates
being 0..1.
The fix is three-fold:
1) Store the actual requested size in the SkGpuDevice, so
that when wrapping it in an SkBitmap for passing to
filterImage(), we can give it the original size.
2) Fix the filters (SkMagnifierImageFilter,
SkLightingImageFilter, SkMatrixConvolutionImageFilter,
SkMatrixImageFilter) whose GPU implementation depends on
0..1 texture coordinates.
3) Remove the exception for GPU-side image filters in
SkCanvas::internalSaveLayer().
For the lighting filters, there were two bugs which were
cancelling each other out: the sobel filter matrix was
being computed upside down, but then we'd negate the
resulting normal. This worked fine in the exact-match case,
but in the approx-match case we'd sample garbage along
the edge pixels. Also, we never implemented the edge pixels
according to spec in the GPU case. It requires a
different fragment shader for each edge of the nine-patch,
which meant we couldn't use asFragmentProcessor(), and had
to implement the drawing via a filterImageGPU() override.
In order to avoid polluting the public API, I inserted a
new base class, SkLightingImageFilterInternal above
Sk[Diffuse|Specular]LightingImageFilter to handle the
implementation.
For the SkMatrixConvolutionImageFilter, it seems the
GLSL clamp() function occasionally returns values outside
the clamped range, resulting in access of garbage
texels even in GL_NEAREST. The fix here is to clamp to a
rect inset by half a texel. There was also a bug in
the unpremultiply step when fConvolveAlpha is false.
For SkMatrixImageFilter, the fix was to make the generic
draw path be more careful about when to use texture domain.
If the bitmap already has a texture, use texture domain
if the srcRect is smaller than the entire texture (not
the entire bitmap).
N.B.: this change will cause some minor pixel diffs in the
GPU results of the following GMs (and possibly more):
matriximagefilter, matrixconvolution, imagefiltersscaled,
lighting, imagemagnifier, filterfastbounds,
complexclip_aa_Layer_invert, complexclip_aa_layer,
complexclip_bw_layer_invert, complexclip_bw_layer.
BUG=skia:3532
Committed: https://skia.googlesource.com/skia/+/b97dafefe63ea0a1bbce8e8b209f4920983fb8b9
Committed: https://skia.googlesource.com/skia/+/f5f8518fe0bbd2703e4ffc1b11ad7b4312ff7641
Committed: https://skia.googlesource.com/skia/+/46112cf2a7c7307f1c9eebb5f881cbda15aa460c
Review URL: https://codereview.chromium.org/1034733002
Reason for revert:
Spoke to Stephen about this. Reverting because failing debug builds:
https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Mac10.9-Clang-MacMini6.2-GPU-HD4000-x86_64-Debug/builds/51https://uberchromegw.corp.google.com/i/client.skia/builders/Test-Ubuntu-GCC-ShuttleA-GPU-GTX660-x86_64-Debug/builds/54
Original issue's description:
> Implement approx-match support in image filter saveLayer() offscreen.
>
> Currently, the GPU-side image filter implementation creates
> exact-match textures for the offscreen backing stores for
> saveLayer(). This is because several filters have GPU
> implementations which depend on the texture coordinates
> being 0..1.
>
> The fix is three-fold:
>
> 1) Store the actual requested size in the SkGpuDevice, so
> that when wrapping it in an SkBitmap for passing to
> filterImage(), we can give it the original size.
> 2) Fix the filters (SkMagnifierImageFilter,
> SkLightingImageFilter, SkMatrixConvolutionImageFilter,
> SkMatrixImageFilter) whose GPU implementation depends on
> 0..1 texture coordinates.
> 3) Remove the exception for GPU-side image filters in
> SkCanvas::internalSaveLayer().
>
> For the lighting filters, there were two bugs which were
> cancelling each other out: the sobel filter matrix was
> being computed upside down, but then we'd negate the
> resulting normal. This worked fine in the exact-match case,
> but in the approx-match case we'd sample garbage along
> the edge pixels. Also, we never implemented the edge pixels
> according to spec in the GPU case. It requires a
> different fragment shader for each edge of the nine-patch,
> which meant we couldn't use asFragmentProcessor(), and had
> to implement the drawing via a filterImageGPU() override.
> In order to avoid polluting the public API, I inserted a
> new base class, SkLightingImageFilterInternal above
> Sk[Diffuse|Specular]LightingImageFilter to handle the
> implementation.
>
> For the SkMatrixConvolutionImageFilter, it seems the
> GLSL clamp() function occasionally returns values outside
> the clamped range, resulting in access of garbage
> texels even in GL_NEAREST. The fix here is to clamp to a
> rect inset by half a texel. There was also a bug in
> the unpremultiply step when fConvolveAlpha is false.
>
> For SkMatrixImageFilter, the fix was to make the generic
> draw path be more careful about when to use texture domain.
> If the bitmap already has a texture, use texture domain
> if the srcRect is smaller than the entire texture (not
> the entire bitmap).
>
> N.B.: this change will cause some minor pixel diffs in the
> GPU results of the following GMs (and possibly more):
> matriximagefilter, matrixconvolution, imagefiltersscaled,
> lighting, imagemagnifier, filterfastbounds,
> complexclip_aa_Layer_invert, complexclip_aa_layer,
> complexclip_bw_layer_invert, complexclip_bw_layer.
>
> BUG=skia:3532
>
> Committed: https://skia.googlesource.com/skia/+/b97dafefe63ea0a1bbce8e8b209f4920983fb8b9
>
> Committed: https://skia.googlesource.com/skia/+/f5f8518fe0bbd2703e4ffc1b11ad7b4312ff7641
>
> Committed: https://skia.googlesource.com/skia/+/46112cf2a7c7307f1c9eebb5f881cbda15aa460cTBR=bsalomon@google.com,reed@chromium.org,senorblanco@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:3532
Review URL: https://codereview.chromium.org/1057693002
Currently, the GPU-side image filter implementation creates
exact-match textures for the offscreen backing stores for
saveLayer(). This is because several filters have GPU
implementations which depend on the texture coordinates
being 0..1.
The fix is three-fold:
1) Store the actual requested size in the SkGpuDevice, so
that when wrapping it in an SkBitmap for passing to
filterImage(), we can give it the original size.
2) Fix the filters (SkMagnifierImageFilter,
SkLightingImageFilter, SkMatrixConvolutionImageFilter,
SkMatrixImageFilter) whose GPU implementation depends on
0..1 texture coordinates.
3) Remove the exception for GPU-side image filters in
SkCanvas::internalSaveLayer().
For the lighting filters, there were two bugs which were
cancelling each other out: the sobel filter matrix was
being computed upside down, but then we'd negate the
resulting normal. This worked fine in the exact-match case,
but in the approx-match case we'd sample garbage along
the edge pixels. Also, we never implemented the edge pixels
according to spec in the GPU case. It requires a
different fragment shader for each edge of the nine-patch,
which meant we couldn't use asFragmentProcessor(), and had
to implement the drawing via a filterImageGPU() override.
In order to avoid polluting the public API, I inserted a
new base class, SkLightingImageFilterInternal above
Sk[Diffuse|Specular]LightingImageFilter to handle the
implementation.
For the SkMatrixConvolutionImageFilter, it seems the
GLSL clamp() function occasionally returns values outside
the clamped range, resulting in access of garbage
texels even in GL_NEAREST. The fix here is to clamp to a
rect inset by half a texel. There was also a bug in
the unpremultiply step when fConvolveAlpha is false.
For SkMatrixImageFilter, the fix was to make the generic
draw path be more careful about when to use texture domain.
If the bitmap already has a texture, use texture domain
if the srcRect is smaller than the entire texture (not
the entire bitmap).
N.B.: this change will cause some minor pixel diffs in the
GPU results of the following GMs (and possibly more):
matriximagefilter, matrixconvolution, imagefiltersscaled,
lighting, imagemagnifier, filterfastbounds,
complexclip_aa_Layer_invert, complexclip_aa_layer,
complexclip_bw_layer_invert, complexclip_bw_layer.
BUG=skia:3532
Committed: https://skia.googlesource.com/skia/+/b97dafefe63ea0a1bbce8e8b209f4920983fb8b9
Committed: https://skia.googlesource.com/skia/+/f5f8518fe0bbd2703e4ffc1b11ad7b4312ff7641
Review URL: https://codereview.chromium.org/1034733002
Reason for revert:
Looks like this change is causing layout test failures which is blocking Skia's DEPS roll into Chromium:
https://codereview.chromium.org/1050563002/https://codereview.chromium.org/1043133005/https://codereview.chromium.org/1048273002/
Reverting to see if this fixes the DEPS roll.
Original issue's description:
> Implement approx-match support in image filter saveLayer() offscreen.
>
> Currently, the GPU-side image filter implementation creates
> exact-match textures for the offscreen backing stores for
> saveLayer(). This is because several filters have GPU
> implementations which depend on the texture coordinates
> being 0..1.
>
> The fix is three-fold:
>
> 1) Store the actual requested size in the SkGpuDevice, so
> that when wrapping it in an SkBitmap for passing to
> filterImage(), we can give it the original size.
> 2) Fix the filters (SkMagnifierImageFilter,
> SkLightingImageFilter) whose GPU implementation depends on
> 0..1 texture coordinates.
> 3) Remove the exception for GPU-side image filters in
> SkCanvas::internalSaveLayer().
>
> For the lighting filters, there were two bugs which were
> cancelling each other out: the sobel filter matrix was
> being computed upside down, but then we'd negate the
> resulting normal. This worked fine in the exact-match case,
> but in the approx-match case we'd sample garbage along
> the edge pixels. Also, we never implemented the edge pixels
> according to spec in the GPU case. It requires a
> different fragment shader for each edge of the nine-patch,
> which meant we couldn't use asFragmentProcessor(), and had
> to implement the drawing via a filterImageGPU() override.
> In order to avoid polluting the public API, I inserted a
> new base class, SkLightingImageFilterInternal above
> Sk[Diffuse|Specular]LightingImageFilter to handle the
> implementation.
>
> N.B.: this change will cause some minor pixel diffs in the
> GPU results of the following GMs (and possibly more):
> matriximagefilter, matrixconvolution, imagefiltersscaled,
> lighting, imagemagnifier, filterfastbounds,
> complexclip_aa_Layer_invert, complexclip_aa_layer,
> complexclip_bw_layer_invert, complexclip_bw_layer.
>
> BUG=skia:3532
>
> Committed: https://skia.googlesource.com/skia/+/b97dafefe63ea0a1bbce8e8b209f4920983fb8b9
>
> Committed: https://skia.googlesource.com/skia/+/f5f8518fe0bbd2703e4ffc1b11ad7b4312ff7641TBR=bsalomon@google.com,reed@chromium.org,senorblanco@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:3532
Review URL: https://codereview.chromium.org/1057443003
Currently, the GPU-side image filter implementation creates
exact-match textures for the offscreen backing stores for
saveLayer(). This is because several filters have GPU
implementations which depend on the texture coordinates
being 0..1.
The fix is three-fold:
1) Store the actual requested size in the SkGpuDevice, so
that when wrapping it in an SkBitmap for passing to
filterImage(), we can give it the original size.
2) Fix the filters (SkMagnifierImageFilter,
SkLightingImageFilter) whose GPU implementation depends on
0..1 texture coordinates.
3) Remove the exception for GPU-side image filters in
SkCanvas::internalSaveLayer().
For the lighting filters, there were two bugs which were
cancelling each other out: the sobel filter matrix was
being computed upside down, but then we'd negate the
resulting normal. This worked fine in the exact-match case,
but in the approx-match case we'd sample garbage along
the edge pixels. Also, we never implemented the edge pixels
according to spec in the GPU case. It requires a
different fragment shader for each edge of the nine-patch,
which meant we couldn't use asFragmentProcessor(), and had
to implement the drawing via a filterImageGPU() override.
In order to avoid polluting the public API, I inserted a
new base class, SkLightingImageFilterInternal above
Sk[Diffuse|Specular]LightingImageFilter to handle the
implementation.
N.B.: this change will cause some minor pixel diffs in the
GPU results of the following GMs (and possibly more):
matriximagefilter, matrixconvolution, imagefiltersscaled,
lighting, imagemagnifier, filterfastbounds,
complexclip_aa_Layer_invert, complexclip_aa_layer,
complexclip_bw_layer_invert, complexclip_bw_layer.
BUG=skia:3532
Committed: https://skia.googlesource.com/skia/+/b97dafefe63ea0a1bbce8e8b209f4920983fb8b9
Review URL: https://codereview.chromium.org/1034733002
This fixes every case where virtual and SK_OVERRIDE were on the same line,
which should be the bulk of cases. We'll have to manually clean up the rest
over time unless I level up in regexes.
for f in (find . -type f); perl -p -i -e 's/virtual (.*)SK_OVERRIDE/\1SK_OVERRIDE/g' $f; end
BUG=skia:
Review URL: https://codereview.chromium.org/806653007
Allow GM results to be compared across machines and platforms by
standardizing the fonts used by all tests.
This adds runtime flags to DM to use either the system font context (the
default), the fonts in the resources directory ( --resourceFonts ) or a set
of canonical paths generated from the fonts ( --portableFonts ).
This CL should leave the current DM results unchanged by default.
If the portable font data or resource font is missing when DM is run, it
falls back to using the system font context.
The create_test_font tool generates the paths and metrics read by DM
with the --portableFonts flag set, and generates the font substitution
tables read by DM with the --resourceFonts flag set.
If DM is run in SkDebug mode with the --reportUsedChars flag set, it
generates the corresponding data compiled into the create_test_font tool.
All GM tests set their typeface information by calling either
sk_tool_utils::set_portable_typeface or
sk_tool_utils::portable_typeface .
(The former takes the paint, the latter returns a SkTypeface.) These calls
can be removed in the future when the Font Manager can be superceded.
BUG=skia:2687
R=mtklein@google.com
Review URL: https://codereview.chromium.org/407183003
- Rename TileGrid -> Quilt to avoid the name overload.
- Tag all failing GMs with kSkipTiled_Flag.
You may be wondering, do any GMs pass? Yes, some do! And that trends towards all of them as we increase --quiltTile.
Two GMs only fail in --quilt mode in 565. Otherwise all GMs which fail are skipped, and those which don't fail aren't. (The 8888 variants of those two GMs are skipped even though they pass.)
BUG=skia:2477
R=reed@google.com, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/256373002
git-svn-id: http://skia.googlecode.com/svn/trunk@14457 2bbb7eff-a529-9590-31e7-b0007b416f81
I also had to offset the matrix passed to filter evaluation by drawSprite() and internalDrawBitmap() by the primitive position. This is the same offset that is applied when drawing the primitive, to compensate for the internal saveLayer().
Also apply the total matrix to the filter params in asNewEffect(), so that (for example) lighting params are offset by both the compositor clipping and upstream crop rects.
R=reed@google.com
Review URL: https://codereview.chromium.org/23295017
git-svn-id: http://skia.googlecode.com/svn/trunk@10961 2bbb7eff-a529-9590-31e7-b0007b416f81
For the GPU path, this required modifying the signature of SkImageFilter::asNewEffect() to receive the bounds offset, so that the lighting filters could offset the light position by the offset. It also required modifying the base-class implementation of SkImageFilter::filterImageGPU() (which implements single-pass filters) to intersect against the bounds rect, to pass its offset to asNewEffect(), and to modify the caller's offset (so it's drawn in the correct place).
Note: this will require rebaselining the lighting GM. Six new test cases were added, to accommodate a cropped version of each lighting filter.
R=bsalomon@google.com, reed@google.com
Review URL: https://codereview.chromium.org/20426002
git-svn-id: http://skia.googlecode.com/svn/trunk@10379 2bbb7eff-a529-9590-31e7-b0007b416f81
the caller instantiates a light (distant, point or spot), and an
SkDiffuseLightingFilter or SkSpecularLightingImageFilter with that light. A
Sobel edge detection filter is applied to the alpha of the incoming bitmap, and
the result is used as a height map for lighting calculations.
Review URL: http://codereview.appspot.com/6302101/
git-svn-id: http://skia.googlecode.com/svn/trunk@4314 2bbb7eff-a529-9590-31e7-b0007b416f81
