Commit Graph

25 Commits

Author SHA1 Message Date
Mike Klein
1feceb3b97 split SkBBoxHierarchy into public/base types
This just does the boring work, keeping the
old type as an empty, now public, base type,
with all the existing methods on _Base.

Bug: skia:9796
Change-Id: I96d93d25955430c0586deca4bb562913d1d7815a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265447
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
2020-01-22 15:33:55 +00:00
Mike Klein
196e9fbbf0 let picture record take a bbh directly
I'm not sure I see a compelling reason for BBH factory objects
to exist, as opposed to say, factory functions.  I don't see any
major harm in letting people hold onto BBH objects themselves.

This allows the caller to create and hang onto the BBH themselves,
allowing use cases like the slight extension I've made to the unit
test PictureNegativeSpace as a demo.  Not super useful yet, as
SkBBoxHierarchy is not a public type...

And add test that mini pictures fill the bbh, which had never been
an interesting question until now we let the user hold onto the BBH.

Bug: skia:9796
Change-Id: I14737f25b31a457a8716669183af0c3bbf01859b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265445
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
2020-01-21 18:59:11 +00:00
Mike Klein
945d1e538c simplify rtree loops in Skia too
Sorry for yanking you around on whether on not this is worth doing.
Seems like SkRTrees are going to stick around for now for Flutter,
so I think we might as well keep both implementations up to date.

This ripples out a little further than in Chromium, as the math we're
deleting here was the only use of the aspect ratio of the passed-in
bounds, that itself the only use of those bounds themselves.  So we can
un-plumb all that too.  I'd still like to see how much we can minimize
the need for those user-provided bounds at all, especially when we're
calculating them all here.

Change-Id: Iea07e8e3d23a4dd31da8bcde512b24caabc96a10
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/213840
Reviewed-by: James Bankoski <jimbankoski@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
2019-05-15 15:42:10 +00:00
Mike Klein
c0bd9f9fe5 rewrite includes to not need so much -Ifoo
Current strategy: everything from the top

Things to look at first are the manual changes:

   - added tools/rewrite_includes.py
   - removed -Idirectives from BUILD.gn
   - various compile.sh simplifications
   - tweak tools/embed_resources.py
   - update gn/find_headers.py to write paths from the top
   - update gn/gn_to_bp.py SkUserConfig.h layout
     so that #include "include/config/SkUserConfig.h" always
     gets the header we want.

No-Presubmit: true
Change-Id: I73a4b181654e0e38d229bc456c0d0854bae3363e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/209706
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Hal Canary <halcanary@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2019-04-24 16:27:11 +00:00
Mike Klein
ad67c66a58 track bounds of top-level control ops
We don't need an explicit save-restore block to determine the bounds of
top-level control operations... the implicit save-restore that all
picutres have should logically work the same way.

The commented test failed before this and passes now.

Bug: skia:7735
Change-Id: Ibd31a3a9b0b48042ab3869a6bb57bc8d8bb78c09
Reviewed-on: https://skia-review.googlesource.com/126460
Commit-Queue: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@chromium.org>
Reviewed-by: Brian Osman <brianosman@google.com>
2018-05-07 16:14:09 +00:00
Mike Klein
738b80d8a5 Don't use getDeviceClipBounds() to bound pic ops.
The values returned by SkCanvas::getDeviceClipBounds() are in the right
space, but have extra constraints on them that are not desirable for
bounding the logical bounds of draw operations:

  - they are integral
  - they are non-negative

We've been intersecting the bounds of each operation with these bounds,
which means we're mixing these bogus constraints into the bounds of each
recorded operation.  This percolates up to the SkPicutre cull rect too.

The most egregious way to see the problem is to record a draw op
entirely in negative space... it'll come back with empty logical bounds
rather than its correct (negative-space) bounds.  I've added a test
for this, and another test I also think should be passing but left
making it so as a follow up.

I've had to disable a couple tests asserting clips affect the bounds. :/

A possible follow-up might go back to using the clips to tighten the
bounds of the ops, just so long as we take the original user bounds and
map them with the CTM through to device space ourselves, rather than
relying on the recording canvas' clip stack.  I think this means we'd
need to maintain our own stack of device-space float SkRect clip bounds
while calculating these op bounds.

Bug: skia:7735
Change-Id: I6bf15f6b2a9ba4329a4eeae7f9d57aa8729ec1bb
Reviewed-on: https://skia-review.googlesource.com/126002
Commit-Queue: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@chromium.org>
Reviewed-by: Brian Osman <brianosman@google.com>
2018-05-04 18:56:13 +00:00
Mike Reed
274218ef01 move largest apis into private
Related to https://skia-review.googlesource.com/c/skia/+/91860

Bug: skia:
Change-Id: Ia8fd981b422bbab75541b078277d2e09e1fc9d41
Reviewed-on: https://skia-review.googlesource.com/91940
Reviewed-by: Brian Salomon <bsalomon@google.com>
2018-01-08 20:42:27 +00:00
Florin Malita
ab244f045a Revert "Revert "Reland: Remove SkLights include from SkCanvas.h""
This reverts commit 9d5f66d9c2.

Reason for revert: Leon landed Android fixes.

Original change's description:
> Revert "Reland: Remove SkLights include from SkCanvas.h"
> 
> This reverts commit fed00319c9.
> 
> Reason for revert: breaking the Android roll.
> 
> Original change's description:
> > Reland: Remove SkLights include from SkCanvas.h
> > 
> > SkLights.h pulls in a bunch of other headers and is not needed (fwdecl
> > works fine).
> > 
> > Change-Id: I3ed97cd7861e51dcb7cfa7950a97b420dbc6fbfb
> > TBR=reed@google.com
> > Reviewed-on: https://skia-review.googlesource.com/15143
> > Commit-Queue: Florin Malita <fmalita@chromium.org>
> > Reviewed-by: Florin Malita <fmalita@chromium.org>
> > 
> 
> TBR=fmalita@chromium.org,reed@google.com
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> 
> Change-Id: I3b0e69f1d04d160f16a5567b09982d35cc9ca84e
> Reviewed-on: https://skia-review.googlesource.com/15195
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> Commit-Queue: Florin Malita <fmalita@chromium.org>
> 

TBR=msarett@google.com,reviews@skia.org,fmalita@chromium.org,reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I0a1c2f9df61f16987ab72dfb4f3a205fbcc37667
Reviewed-on: https://skia-review.googlesource.com/15229
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
2017-05-03 19:17:11 +00:00
Florin Malita
9d5f66d9c2 Revert "Reland: Remove SkLights include from SkCanvas.h"
This reverts commit fed00319c9.

Reason for revert: breaking the Android roll.

Original change's description:
> Reland: Remove SkLights include from SkCanvas.h
> 
> SkLights.h pulls in a bunch of other headers and is not needed (fwdecl
> works fine).
> 
> Change-Id: I3ed97cd7861e51dcb7cfa7950a97b420dbc6fbfb
> TBR=reed@google.com
> Reviewed-on: https://skia-review.googlesource.com/15143
> Commit-Queue: Florin Malita <fmalita@chromium.org>
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> 

TBR=fmalita@chromium.org,reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I3b0e69f1d04d160f16a5567b09982d35cc9ca84e
Reviewed-on: https://skia-review.googlesource.com/15195
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
2017-05-03 15:41:07 +00:00
Florin Malita
fed00319c9 Reland: Remove SkLights include from SkCanvas.h
SkLights.h pulls in a bunch of other headers and is not needed (fwdecl
works fine).

Change-Id: I3ed97cd7861e51dcb7cfa7950a97b420dbc6fbfb
TBR=reed@google.com
Reviewed-on: https://skia-review.googlesource.com/15143
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2017-05-03 13:22:28 +00:00
Florin Malita
38aad81a54 Revert "Remove SkLights include from SkCanvas.h"
This reverts commit 9ff301bf91.

Reason for revert: need to update G3, Flutter.

Original change's description:
> Remove SkLights include from SkCanvas.h
> 
> SkLights.h pulls in a bunch of other headers and is not needed (fwdecl
> works fine).
> 
> Change-Id: Id2d7176eb3bf4609f72f46d513eebf59318f542f
> Reviewed-on: https://skia-review.googlesource.com/14904
> Reviewed-by: Mike Reed <reed@google.com>
> Commit-Queue: Florin Malita <fmalita@chromium.org>
> 

TBR=mtklein@google.com,fmalita@chromium.org,reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I4799ad5b31aaeaf529c8b912bbe09aa8869a5e6c
Reviewed-on: https://skia-review.googlesource.com/15107
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@chromium.org>
2017-05-02 15:42:07 +00:00
Florin Malita
9ff301bf91 Remove SkLights include from SkCanvas.h
SkLights.h pulls in a bunch of other headers and is not needed (fwdecl
works fine).

Change-Id: Id2d7176eb3bf4609f72f46d513eebf59318f542f
Reviewed-on: https://skia-review.googlesource.com/14904
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>
2017-05-02 14:50:38 +00:00
Mike Klein
1dd161c833 Fix infinite loop in SkRTree::insert()
When aspectRatio is not finite, insert() can fall into an infinite loop.
This happens if you pass SkRect::MakeLargest() to the factory as bounds.

BUG=skia:5974

Change-Id: Ibcc9e5c5943c718608d4c1448305f7b8f11413bc
Reviewed-on: https://skia-review.googlesource.com/11784
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Klein <mtklein@chromium.org>
2017-04-10 13:11:34 +00:00
Brian Salomon
d3b65972aa Mark overridden destructors with 'override' and remove 'virtual'
This silences a new warning in clang 5.0

Change-Id: Ieb5b75a6ffed60107c3fd16075d2ecfd515b55e8
Reviewed-on: https://skia-review.googlesource.com/10006
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
2017-03-22 16:06:18 +00:00
reed
73603f3c52 abstract name of clipping ops, to transtion to a more restricted set
SkRegion::Op --> SkCanvas::ClipOp (alias) --> SkClipOp

pre-CL needed in chrome : https://codereview.chromium.org/2355583002/

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2355483002

Review-Url: https://codereview.chromium.org/2355483002
2016-09-20 08:42:39 -07:00
reed
ca2622ba05 return pictures as sk_sp
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1811703002

Review URL: https://codereview.chromium.org/1811703002
2016-03-18 07:25:55 -07:00
halcanary
96fcdcc219 Style Change: NULL->nullptr
DOCS_PREVIEW= https://skia.org/?cl=1316233002

Review URL: https://codereview.chromium.org/1316233002
2015-08-27 07:41:16 -07:00
mtklein
36352bf5e3 C++11 override should now be supported by all of {bots,Chrome,Android,Mozilla}
NOPRESUBMIT=true

BUG=skia:
DOCS_PREVIEW= https://skia.org/?cl=1037793002

Review URL: https://codereview.chromium.org/1037793002
2015-03-25 18:17:32 -07:00
mtklein
72c9faab45 Fix up all the easy virtual ... SK_OVERRIDE cases.
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
2015-01-09 10:06:40 -08:00
mtklein
703dd2ed18 Remove SkTileGrid (except for TileGridInfo).
TBR=reed@google.com

BUG=skia:3085

Review URL: https://codereview.chromium.org/845623002
2015-01-09 06:41:48 -08:00
mtklein
49aabde6ef Clean up dead clear() code in SkRecord.
This removes the SkRecords::Clear struct and everything that refers to it.
Notice there is nothing actually creating a Clear, which means this is all
dead code.

Now that all ops obey the clip, I don't think we need the weird
inflate-empty-to-epsilon hack for BBH queries.

BUG=skia:

Review URL: https://codereview.chromium.org/835813002
2015-01-05 07:02:45 -08:00
reed
2ff1fcede1 Defer saves() until they're needed
patch from issue 759443006 at patchset 40001 (http://crrev.com/759443006#ps40001)

BUG=skia:

Review URL: https://codereview.chromium.org/767333002
2014-12-11 07:07:38 -08:00
reed
90d0ff013b add some debugging to SkNVRefCnt
BUG=skia:

Review URL: https://codereview.chromium.org/745383003
2014-11-24 12:02:31 -08:00
mtklein
7cc1a34fbf Make sure pictures draw Clears even when the clip is empty.
We fix this by rewriting empty queries to very tiny queries, which will certainly
hit ops that span the entire picture (like Clear) and hopefully not much more.

(This doesn't quite work in the full cull rect world if [0,0,ε,ε] doesn't
overlap the picture.  Let's cross that bridge when we get there.)

BUG=432991

Review URL: https://codereview.chromium.org/732723004
2014-11-20 08:01:09 -08:00
junov
a41d3c33bc Make RTree handle the case where the playback canvas has empty clip bounds
BUG=skia:3074

Review URL: https://codereview.chromium.org/678303004
2014-10-30 11:44:20 -07:00