The multi-parameter version of gfx::Rect[F]::Inset() accepts
parameters in the order of left, top, right, bottom which is
different from the order of Insets::TLBR() which is top, left,
bottom, right. To avoid mistakes, we should avoid and remove
the multi-parameter version.
This CL is mostly automatically generated using the following
commands:
files=`grep -r -l --include="*.cc" --include="*.h" --include="*.mm" --exclude-dir=out '\.Inset('`
# Replace Inset(x, x) with Inset(x)
sed -i -r 's/(\.|->)Inset\((([^(),]*(\([^()]*\))?)*), \2\)/\1Inset(\2)/g' $files
# Replace Inset(x, x, x, x) with Inset(x)
sed -i -r 's/(\.|->)Inset\((([^(),]*(\([^()]*\))?)*), \2, \2, \2\)/\1Inset(\2)/g' $files
# Replace Inset(x, y) with Inset(gfx::Insets::VH(y, x))
sed -i -r 's/(\.|->)Inset\((([^(),]*(\([^()]*\))?)*), (([^(),]*(\([^()]*\))?)*)\)/\1Inset(gfx::Insets::VH(\5, \2))/g' $files
# Replace Inset(x, y, x, y) with Inset(gfx::Insets::VH(y, x))
sed -i -r 's/(\.|->)Inset\((([^(),]*(\([^()]*\))?)*), (([^(),]*(\([^()]*\))?)*), \2, \5\)/\1Inset(gfx::Insets::VH(\5, \2))/g' $files
# Replace Inset(l, t, r, b) with Inset(TEMPINSETS(t, l, r, b))
# This step is because sed doesn't support more than 9 capture groups
sed -i -r 's/(\.|->)Inset\((([^(),]*(\([^()]*\))?)*), (([^(),]*(\([^()]*\))?)*), (([^(),]*(\([^()]*\))?)*, ([^(),]*(\([^()]*\))?)*)\)/\1Inset(TEMPINSETS(\5, \2, \8))/g' $files
# Replace TEMPINSETS(t, l, r, b) with gfx::Insets::TLBR(t, l, b, r)
sed -i -r 's/\(TEMPINSETS\((([^(),]*(\([^()]*\))?)*, ([^(),]*(\([^()]*\))?)*), (([^(),]*(\([^()]*\))?)*), (([^(),]*(\([^()]*\))?)*)\)\)/(gfx::Insets::TLBR(\1, \9, \6))/g' $files
Then manually replace gfx::Insets:: with gfx::InsetsF:: where
gfx::InsetsF is required. There are about 20 such places.
Complex cases like multi-line statements and parameters with nested
parentheses will be modified manually in follow-ups.
Bug: 1302500
Change-Id: I1ed5d87b76968beb716e8bb988a70b421fc4d5da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3559674
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Owners-Override: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988541}
Previously the following files contain duplicated definition of
EXPECT_* macros for ui/gfx/geometry types:
gfx/ui/test/gfx_util.h
gfx/ui/geometry/test/size_test_util.h
gfx/ui/geometry/test/rect_test_util.h
gfx/ui/geometry/test/transform_test_util.h
cc/test/geometry_test-utils.h
Now consolidate them into gfx/ui/geometry/test/geometry_util.h.
Bug: 738465
Change-Id: Ie2b95f352891b1e034fcb3b466b3d9a3471111d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3208493
Auto-Submit: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Owners-Override: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#929848}
Currently SpiralDifferenceIterator implements logic based on directions
around the center rect. This logic does not take care of aspect ratios
of tile size. To consider the aspect ratios during iteration new logic
has to be implanted. Until the new logic gets tested thoroughly we
need the current logic. This patch detaches the current spiral iterator
to separate classes (SpiralIterator and ReverseSpiralIterator) and
moves them to their own files making SpiralDifferenceIterator thinner.
Now new logic can be added to SpiralDifferenceIterator based on some
runtime switch.
This patch also separates out unit tests related to SpiralIterator from
tiling_data_unittest.cc to its own file.
BUG=613695
TESTS=TilingDataSpiralIteratorTest.*
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
Review-Url: https://codereview.chromium.org/2352393002
Cr-Commit-Position: refs/heads/master@{#420823}
This patch adds an iterator similar to spiral difference iterator,
except that it iterates the indecies in reverse order. This is needed
for a fast eviction iterator.
R=danakj, enne
Review URL: https://codereview.chromium.org/645173004
Cr-Commit-Position: refs/heads/master@{#300221}
When removing tiles, we need to remove everything outside the live
tiles rect even if it has borders inside the live tiles rect. The
DifferenceIterator includes borders when deciding if a tile is in the
included rect, so tiles that are only in the included rect with a border
were not being removed from the tiling. This lead to us having tiles
outside the rect, and to creating a tile that already existed.
When adding tiles, the same problem existed. The DifferenceIterator
includes borders when deciding which tiles are to be excluded, so a
tile that was inside the old live tiles rect with borders only would
not have existed, but it would be excluded by the DifferenceIterator.
This prevented us from making tiles that were inside the live tiles
rect.
Depends on: https://codereview.chromium.org/513903002/R=enne@chromium.org, vmpstr@chromium.org, enne, vmpstr
BUG=405427
Review URL: https://codereview.chromium.org/505913003
Cr-Commit-Position: refs/heads/master@{#292417}
When expanding the live tiles rect, we want to include border pixels
for all tiles that intersect the live tiles rect. It used to do this
by also including any tiles whose border pixels intersect the live
tiles rect, but that's excessive.
This adds ExpandRectIgnoringBordersToTileBoundsWithBordersEmpty and
uses it to expand the live tiles rect, the unit test
PictureLayerTilingIteratorTest.ResizeOverBorderPixelsDeletesTiles
covers this usage of the method.
The ExpandRectToTileBoundsWithBorders is no longer used, so remove it.
R=enne
BUG=386998
Review URL: https://codereview.chromium.org/385123006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282896 0039d316-1c4b-4281-b951-d872f2087c98
Currently we do a walk over all tiles in the pending tree's pile, but
there can be 1 million x 1 million tiles on some pages, which makes this
method take multiple _seconds_ to complete.
These loops were added in r184525 which gave the tradeoffs that led to
them, which are that we want to only use a single pile for all the tiles
on this layer, but we don't want raster tiles on the active layer (when
this activates) that can't be rastered by the pile they are attached to.
Instead of walking every pile-tile, to find the tiles that are not
present in the current recording and invalidate them, we expand
invalidations outside the interest rect to cover the full recording tiles,
and we expand invalidation inside the interest rect to include any raster
tiles that don't have a recording (such as in the offscreen animating gif
case). We give this expanded invalidation to the pending layer, causing
it to drop rastered tiles from the active tree that intersect with an
unrecorded area.
R=enne
BUG=371839
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277964
Review URL: https://codereview.chromium.org/294163009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278196 0039d316-1c4b-4281-b951-d872f2087c98
This cl seems to cause heap use after free on the Asan bots.
LargePage
FindLongString
http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Tests%20%282%29/builds/3837/steps/browser_tests/logs/LargePagehttp://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Tests%20%283%29/builds/4211/steps/browser_tests/logs/FindLongString
> cc: In SyncFromActiveLayer, drop all tiles not in the recorded_viewport.
>
> Currently we do a walk over all tiles in the pending tree's pile, but
> there can be 1 million x 1 million tiles on some pages, which makes this
> method take multiple _seconds_ to complete.
>
> These loops were added in r184525 which gave the tradeoffs that led to
> them, which are that we want to only use a single pile for all the tiles
> on this layer, but we don't want raster tiles on the active layer (when
> this activates) that can't be rastered by the pile they are attached to.
>
> Instead of walking every pile-tile, to find the tiles that are not
> present in the current recording and invalidate them, we expand
> invalidations outside the interest rect to cover the full recording tiles,
> and we expand invalidation inside the interest rect to include any raster
> tiles that don't have a recording (such as in the offscreen animating gif
> case). We give this expanded invalidation to the pending layer, causing
> it to drop rastered tiles from the active tree that intersect with an
> unrecorded area.
>
> R=enne
> BUG=371839
>
> Review URL: https://codereview.chromium.org/294163009TBR=danakj@chromium.org
Review URL: https://codereview.chromium.org/347493002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277980 0039d316-1c4b-4281-b951-d872f2087c98
Currently we do a walk over all tiles in the pending tree's pile, but
there can be 1 million x 1 million tiles on some pages, which makes this
method take multiple _seconds_ to complete.
These loops were added in r184525 which gave the tradeoffs that led to
them, which are that we want to only use a single pile for all the tiles
on this layer, but we don't want raster tiles on the active layer (when
this activates) that can't be rastered by the pile they are attached to.
Instead of walking every pile-tile, to find the tiles that are not
present in the current recording and invalidate them, we expand
invalidations outside the interest rect to cover the full recording tiles,
and we expand invalidation inside the interest rect to include any raster
tiles that don't have a recording (such as in the offscreen animating gif
case). We give this expanded invalidation to the pending layer, causing
it to drop rastered tiles from the active tree that intersect with an
unrecorded area.
R=enne
BUG=371839
Review URL: https://codereview.chromium.org/294163009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277964 0039d316-1c4b-4281-b951-d872f2087c98
In practice, cc is responsible for implicit invalidations on layers,
such as the original layer being entirely invalid or new areas being
exposed during resize.
In this case, these invalidations were being clipped to the live tiles
rect, which itself is clipped to the bounds of the tiling. Therefore,
when the bounds of a layer grew, the new invalidation intersected with
the old bounds was empty.
The intersection of invalidations with the live tiles rect is an
optimization, to prevent needless work trying to look up invalidations
on parts of the layer that don't matter.
To fix the bug, rather than intersecting with the live tiles rect,
expand the live tiles rect to tile boundaries. This lets implicit
invalidation due to resize within a tile not skip rerasterizing
that tile.
BUG=357120
Review URL: https://codereview.chromium.org/240593004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@265313 0039d316-1c4b-4281-b951-d872f2087c98
This patch changes the assumption that the tiling covers the entire layer. It
can now cover an arbitray sub-rect, called the tiling_rect. This will be used
to implement viewport tiling in a separate patch.
R=enne@chromium.org
BUG=362668
Review URL: https://codereview.chromium.org/235753002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@264083 0039d316-1c4b-4281-b951-d872f2087c98
This iterator is needed to reland
https://codereview.chromium.org/196343005/ correctly, as whether or not
a set of picture tiles can raster a particular rect needs to ignore
borders.
BUG=353346
Review URL: https://codereview.chromium.org/202753002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@257688 0039d316-1c4b-4281-b951-d872f2087c98
When the center is off the tiling, then the unclamped calculations
are incorrect. That is, instead of values being rounded down in
integer divides, they are instead rounded toward zero.
This patch reworks the way the around coordinates are calculated.
Instead of relying on unclamped version of existing functions,
we instead explicitly check whether the src coord is within the
tiling total size.
R=enne
Review URL: https://codereview.chromium.org/188863002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@255670 0039d316-1c4b-4281-b951-d872f2087c98
This patch adds another iterator to tiling data. This iterator acts
similar to the difference iterator, but it also takes an extra center
rect parameter. The indices are returned in a counter-clockwise spiral
around the center rect. Note that the center rect itself is also
considered to be ignored.
Added tests to verify the behavior. Currently, the iterator is not used
anywhere outside of the unit tests.
R=enne@chromium.org
Review URL: https://codereview.chromium.org/183123004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@254622 0039d316-1c4b-4281-b951-d872f2087c98
Any struct of size > 4 bytes should be passed by const ref.
Passing by ref for these structs is faster than passing by value,
especially when invoking function has multiple parameters and some
other scenarios mentioned in the bug.
BUG=159273
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=247426
Review URL: https://codereview.chromium.org/145313006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247769 0039d316-1c4b-4281-b951-d872f2087c98
Any struct of size > 4 bytes should be passed by const ref.
Passing by ref for these structs is faster than passing by value,
especially when invoking function has multiple parameters and some
other scenarios mentioned in the bug.
BUG=159273
Review URL: https://codereview.chromium.org/145313006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247426 0039d316-1c4b-4281-b951-d872f2087c98
The flag flip will be a follow up once ChromeOS build is sorted out.
Review URL: https://codereview.chromium.org/13206004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@191417 0039d316-1c4b-4281-b951-d872f2087c98
And fix compile errors that it causes.
Review URL: https://codereview.chromium.org/13206004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@191364 0039d316-1c4b-4281-b951-d872f2087c98
