There are two ways negative sigma values may occur: in
the original filter parameters, or after multiplication
by a negative scaling CTM. The former case is
invalid according to the spec, so we continue to check
for it at validation time. In the latter case, we should
interpret it as a horizontal flip in the kernel pixel
access, and simply take the absolute value (since the
filter kernel is symmetric).
Also refactor all this logic into a single place for the
CPU, GPU and onFilterBounds() paths.
BUG=https://code.google.com/p/chromium/issues/detail?id=409602R=sugoi@google.com, reed@google.com, sugoi@chromium.org
Author: senorblanco@chromium.org
Review URL: https://codereview.chromium.org/555603002
morphology, blur.
[Reland with fixed tests.]
Because we're computing "backwards" from a clip rect of destination
pixels to be filled to the required source pixels, we should use tail
recursion rather than head recursion in onFilterBounds().
This actually only makes a difference for drop-shadow, where
the computation is non-commutative. Blur and morphology commute, but I
moved them to tail recursion anyway for clarity (so all onFilterBounds
use tail recursion).
BUG=skia:
R=bsalomon@google.com
Author: senorblanco@chromium.org
Review URL: https://codereview.chromium.org/481273005
Reason for revert:
This CL is currently breaking the Win7 and Win8 bots on some of the new tests (in DM).
Original issue's description:
> Fix recursive computation of filter bounds for drop shadow,
> morphology, blur.
>
> Because we're computing "backwards" from a clip rect of destination
> pixels to be filled to the required source pixels, we should use tail
> recursion rather than head recursion in onFilterBounds().
>
> This actually only makes a difference for drop-shadow, where
> the computation is non-commutative. Blur and morphology commute, but I
> moved them to tail recursion anyway for clarity (so all onFilterBounds
> use tail recursion).
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/28648fe4a69b0cee8df42b5966e4e645c3aabefbR=bsalomon@google.com, senorblanco@chromium.orgTBR=bsalomon@google.com, senorblanco@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=skia:
Author: djsollen@google.com
Review URL: https://codereview.chromium.org/504773003
morphology, blur.
Because we're computing "backwards" from a clip rect of destination
pixels to be filled to the required source pixels, we should use tail
recursion rather than head recursion in onFilterBounds().
This actually only makes a difference for drop-shadow, where
the computation is non-commutative. Blur and morphology commute, but I
moved them to tail recursion anyway for clarity (so all onFilterBounds
use tail recursion).
BUG=skia:
R=bsalomon@google.com
Author: senorblanco@chromium.org
Review URL: https://codereview.chromium.org/481273005
In SkBBoxHierarchyRecord:
Since the bounds we pass to saveLayer are in the pre-filtering
coordinate space, they aren't correct for determining the actual
device pixels touched by the saveLayer in this case.
The easiest fix for now is to pass the clip bounds, since the final
draw done in restore() will never draw outside the clip.
In SkRecordDraw:
We do adjust the bounds passed to saveLayer, so we just need to make
sure that when we're using a paint that may affect transparent black,
we ignore the calculated bounds of draw ops and use the clip intersected
with those adjusted bounds.
See originally crrev.com/497773002
BUG=skia:
R=reed@google.com, senorblanco@chromium.org, junov@chromium.org, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/496963003
Add a unique ID to SkImageFilter, and use it as part
of a persistent cache of image-filtered results. This is used for
caching frame-to-frame coherent filters.
We also keep track of which filter subtrees do not reference the
src input, and use a GenID of zero for the src input in that case.
That way, subtrees which are not dependent on the filter input can be
cached independently of it.
This gives approximately a 4X speedup on
letmespellitoutforyou.com/samples/svg/filter_terrain.svg on Z620
and Nexus10. The cache key consists of the uniqueID of the filter, the
clip bounds, the CTM and the genID of the input bitmap.
Since this does not yet handle the case where the input primitives
(and part of the resulting filter tree) are unchanged, we have
to keep around the external cache for that painting case.
When the work to cache unchanging input primitives is done, the
old cache can be removed, and the new UniqueIDCache will be renamed
to Cache.
R=bsalomon@google.com, mtklein@google.com
Author: senorblanco@chromium.org
Review URL: https://codereview.chromium.org/414483003
When building acceleration structures for SkPicture, we must transform
the primitive's bounds not only by its own SkPaint, but by the paints of
any saveLayer()s currently active above it.
We do this by pushing the SkPaint onto a stack on
saveLayer(), and popping them on restore(). We also push
a NULL paint in save(), so that the pushes and pops are
balanced.
BUG=skia:2734
R=mtklein@google.com, reed@google.com
Author: senorblanco@chromium.org
Review URL: https://codereview.chromium.org/380373003
Now that all creation of SkImageFilters goes through
factory Create() methods, there's no real reason for the
convenience constructors. Some SkImageFilter subclasses
which actually have zero DAG-able inputs were passing NULL
to the superclass constructor. This actually means 1 input,
with a NULL value, not zero inputs. This becomes more
relevant for the upcoming cache infrastructure, where this
indicates that the filter will use its src input, where in
fact some of these filters do not (they are image generators
only).
Limiting SkImageFilter to a single constructor resolves this
ambiguity.
Along the way, I removed all of the default parameters to
the constructors, since the Create methods always call them
with the full argument list.
BUG=skia:
R=reed@google.com
Author: senorblanco@chromium.org
Review URL: https://codereview.chromium.org/376953003
That way when declaring a test with DEF_TEST() macro, you don't have to
uniquify the test name because it might colide with the class it is
being testing.
For example, if you are testing SkBase64 and do:
DEF_TEST(SkBase64, reporter) {
}
That will generate an error because the macro will declare a function
named SkBase64 which colides with the type SkBase64.
By adding Test to the function name we avoid this problem.
Fixed the entries found with the following command line:
$ git grep "Test, r" | grep DEF
BUG=None
TEST=make tests && out/Debug/tests
R=mtklein@google.com
Author: tfarina@chromium.org
Review URL: https://codereview.chromium.org/345753007
Picture serialization is not yet hardened, but it turns out we do need
serialization of SkPictureImageFilter for deferred SVG-on-SVG filters,
since the SkPaints (and thus the SkImageFilters) are serialized by
SkPictureRecord. However, deferred filters are always drawn in the
same process, so we can safely serialize them in this case. We do this
by turning the compile-time check for
SK_ALLOW_PICTUREIMAGEFILTER_SERIALIZATION to a runtime check for
isCrossProcess().
The image filter fuzzer sample was also modified to enable fuzzing
of basic picture image filters (the code had rotted a bit, being behind
an #ifdef that no one sets).
BUG=375162
R=sugoi@google.com
Review URL: https://codereview.chromium.org/311443003
git-svn-id: http://skia.googlecode.com/svn/trunk@15008 2bbb7eff-a529-9590-31e7-b0007b416f81
If an saveLayer()/restore() is recorded, tilegrid/rtree will cull them
out and not draw anything. This is correct for most cases, but if the
paint in the saveLayer() is one that affects transparent black (e.g.,
it contains a color filter or image filter which affects transparent
black), this is incorrect: the filter should be applied.
Fixed by adding a no-op between the saveLayer() and restore(), and
adding a bbox node pointing at that node with the saveLayer()'s bounds.
This exposed a bug in SkPictureRecord.cpp's match(), where it would
assert if the NOOP was the last op seen. Fixed with an early-out before
calling peek_op_and_size().
BUG=skia:2254
Review URL: https://codereview.chromium.org/262363007
git-svn-id: http://skia.googlecode.com/svn/trunk@14604 2bbb7eff-a529-9590-31e7-b0007b416f81
When matrix convolution processes border pixels with zero width, it
asserts in getAddr32() with an invalid x coordinate. The assert is
harmless, since the returned pointer is never accessed (the next line
is a loop from left to right, which does nothing, since left == right).
However, the fix is simple: early out on an empty rect before entering
the outer loop.
R=sugoi@chromium.org
Review URL: https://codereview.chromium.org/265693005
git-svn-id: http://skia.googlecode.com/svn/trunk@14497 2bbb7eff-a529-9590-31e7-b0007b416f81
Add a new GM that exercises tiled drawing all pixel-moving filters
(and some non-pixel-moving ones) and compares it against non-tiled
drawing of the same filters. Fixing this test revealed that tile and
matrix convolution filters had no onFilterBounds() traversals
(test-driven development FTW). Tile requires (conservatively) the
bounds to include the whole source rect, since it may end up in the
result. Matrix convolution requires the bounds to be offset by the
kernel size and target.
R=reed@google.com
BUG=skia:
Review URL: https://codereview.chromium.org/258243005
git-svn-id: http://skia.googlecode.com/svn/trunk@14432 2bbb7eff-a529-9590-31e7-b0007b416f81
This CL sets the stage for retracting the SkPicture::kOptimizeForClippedPlayback_RecordingFlag flag
from the public API (more work needs to be done in Blink & Chrome). In the new world the only way
to set this flag (and thus instantiate an SkPicture-derived
class) is by passing a factory to the SkPictureRecorder class. This is to get all clients always using
factories so that we can then change the factory call used (i.e., so the factory just creates a BBH) and
do away with the SkPicture-derived classes.
BUG=skia:2315
R=reed@google.com
Author: robertphillips@google.com
Review URL: https://codereview.chromium.org/239703006
git-svn-id: http://skia.googlecode.com/svn/trunk@14221 2bbb7eff-a529-9590-31e7-b0007b416f81
When image filters are processed within Skia, they simply do
a blind recursion. This has the side-effect of turning the
DAG into a tree. I.e., nodes visited more than once during
the traversal will be processed more than once.
This change implements a very simple cacheing scheme: a
cache is created before traversing the DAG, and handed
into the processing traversal. Before recursing into a child
in SkImageFilter::filterImage(), the cache is checked for a
hit, and early-out is performed. Otherwise, the node is
processed, and its result bitmap and location (offset) are
cached, but only if it contains two or more children and
thus will be visited again during the traversal.
Currently, the child count is approximated with the
refcount. This is good enough in most cases (and exactly
correct for the Chrome use case). We could add an exact
child count to the image filter, but this will require
violating the immutability of image filters slightly in
order to bump the child count as nodes are connected. I
leave it up to the reviewer to decide which is better.
R=reed@google.com
Author: senorblanco@chromium.org
Review URL: https://codereview.chromium.org/230653005
git-svn-id: http://skia.googlecode.com/svn/trunk@14160 2bbb7eff-a529-9590-31e7-b0007b416f81
When adjusting the CTM for filter use, we were subtracting off the
destination coordinates of the drawDevice() or drawSprite(). This is
not quite correct: we should subtract off the coordinates relative to
the device origin instead. This occurs when one filtered saveLayer() is
drawn inside another saveLayer(), both with non-zero origin.
This fixes layout test svg/batik/text/smallFonts.svg in Blink, and is
exercised by the provided unit test.
BUG=skia:
R=bsalomon@google.com
Review URL: https://codereview.chromium.org/222723002
git-svn-id: http://skia.googlecode.com/svn/trunk@14029 2bbb7eff-a529-9590-31e7-b0007b416f81
If one of inputs to SkXfermodeImageFilter draws nothing, either due to
it being cropped out upstream, or within the filter itself, the filter
should still draw the other input, since otherwise the result will be incorrect.
For the GPU path, since we can't detect this case in
canFilterImageGPU() without recursing, we'll just drop to
the generic path if either input is empty, since we can't use the effect in that case anyway.
While we're at it, let's drop to the generic path if the
xfermode can't be expressed as an effect, since the code
here was doing a 2-pass render in that case anyway, which
is equivalent to what the (xfermode == NULL) case was doing
anyway.
R=bsalomon@google.com, sugoi@chromium.org
Review URL: https://codereview.chromium.org/220723007
git-svn-id: http://skia.googlecode.com/svn/trunk@14016 2bbb7eff-a529-9590-31e7-b0007b416f81
This was crashing on the GPU path, due to a failed texture allocation.
The belt-and-suspenders fix is to:
1) Limit the GPU path to only allocate up to maxTextureSize.
2) Limit both the raster and GPU paths to reasonable blur sizes (box blur
kernel size of 1000, resulting in a sigma limit of 532).
R=bsalomon@google.com
BUG=skia:
Review URL: https://codereview.chromium.org/209353014
git-svn-id: http://skia.googlecode.com/svn/trunk@13923 2bbb7eff-a529-9590-31e7-b0007b416f81
NOTE: this patch set is based on https://codereview.chromium.org/189913021/,
and needs that patch to land first.
Until now, crop rects in Skia have only been able to reduce
the size of the destination bounds, but not expand them.
SVG semantics require the latter as well. The heart of
the change is in applyCropRect(), which now assigns each
edge, instead of doing an intersection with the crop rect.
In order to support this (and still work well with tiled
drawing) we need to clip the resulting crop rect to the
clipping region of the filters. This uses the Context struct
previously landed from https://codereview.chromium.org/189913021/.
Many of the pixel loops are not yet ready to handle a
destination rect larger than the source rect. So we provide
a convenience version of applyCropRect() which creates an
offscreen and pads it out with transparent black. Once the
pixel loops and shaders have been fixed to support larger
destination bounds, they should be switched back to the
non-drawing version of applyCropRect().
BUG=skia:
R=bsalomon@google.com, reed@google.com
Committed: https://code.google.com/p/skia/source/detail?r=13805
Review URL: https://codereview.chromium.org/198003008
git-svn-id: http://skia.googlecode.com/svn/trunk@13809 2bbb7eff-a529-9590-31e7-b0007b416f81
Some upcoming work (support for expanding crop rects) requires
the clip bounds to be available during filter traversal. This change
replaces the SkMatrix parameter in the onFilterImage() traversals
with a Context parameter. It contains the CTM, as well as the clip
bounds.
BUG=skia:
R=reed@google.com
Review URL: https://codereview.chromium.org/189913021
git-svn-id: http://skia.googlecode.com/svn/trunk@13803 2bbb7eff-a529-9590-31e7-b0007b416f81
