Previously, REPORTER_ASSERT/ERRORF relied on a helper function named
`reporter_string` which papered over zero-argument and one-argument
messages (where one-argument messages are assumed to ignore printf
formatting rules entirely, and just forward the message as-is).
Replacing this helper with a direct call to `SkStringPrintf` allows
the compiler to check format arguments for correctness, but sacrifices
the one-argument special case. In practice the one-argument special
case was very rarely used, so it's not a significant sacrifice,
and this did uncover several real errors in assertion format strings
(including some cases where the wrong number of arguments was passed).
Change-Id: I4378c43b16fd8fdbf4c78d849a9f2f0a254f7abc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506617
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit b25f30348b.
Bug: skia:10154
Original message:
> Add client_utils for code that is specifically for a single client.
> Move SkFrontBufferedStream into its android/ subdir. Rename the class
> to android::skia::FrontBufferedStream. Temporarily leave in
> SkFrontBufferedStream until Android updates to the new API.
>
> Add a new optional target for client_utils/android. It is built in dev
> builds for testing, and when building for the Android framework.
Deliberately do not include client_utils in Google3, since the whole
point is to only include where necessary.
Change-Id: I48938c56aabb98e1ed820240d43ffcd0fdce7956
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/285104
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This reverts commit 513720f28e.
Reason for revert: Breaking the google3 roll:
https://sponge.corp.google.com/invocation?tab=Build+Log&id=5f96970b-8171-4c2f-abf3-006e11b8fff9
Original change's description:
> Move SkFrontBufferedStream into Android-only dir
>
> Bug: skia:10154
>
> Add client_utils for code that is specifically for a single client.
> Move SkFrontBufferedStream into its android/ subdir. Rename the class
> to android::skia::FrontBufferedStream. Temporarily leave in
> SkFrontBufferedStream until Android updates to the new API.
>
> Add a new optional target for client_utils/android. It is built in dev
> builds for testing, and when building for the Android framework.
>
> Change-Id: Ie0f425051ea370aab7861d61150a3d6007214a93
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/284721
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
> Reviewed-by: Derek Sollenberger <djsollen@google.com>
TBR=djsollen@google.com,scroggo@google.com,reed@google.com
Change-Id: Iaeedaed184cc35f507d5441631ae709e1c5cb1ac
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10154
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/285100
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Bug: skia:10154
Add client_utils for code that is specifically for a single client.
Move SkFrontBufferedStream into its android/ subdir. Rename the class
to android::skia::FrontBufferedStream. Temporarily leave in
SkFrontBufferedStream until Android updates to the new API.
Add a new optional target for client_utils/android. It is built in dev
builds for testing, and when building for the Android framework.
Change-Id: Ie0f425051ea370aab7861d61150a3d6007214a93
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/284721
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Use std::min and std::max everywhere.
SkTPin still exists. We can't use std::clamp yet, and even when
we can, it has undefined behavior with NaN. SkTPin is written
to ensure that we return a value in the [lo, hi] range.
Change-Id: I506852a36e024ae405358d5078a872e2c77fa71e
Docs-Preview: https://skia.org/?cl=269357
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269357
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
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>
This reverts commit 2a2f675926.
Reason for revert: this appears to be what is holding up the Chrome roll.
Original change's description:
> SkTypes: extract SkTo
>
> Change-Id: I8de790d5013db2105ad885fa2683303d7c250b09
> Reviewed-on: https://skia-review.googlesource.com/133620
> Reviewed-by: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,halcanary@google.com
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: Iafd738aedfb679a23c061a51afe4b98a8d4cdfae
Reviewed-on: https://skia-review.googlesource.com/134504
Reviewed-by: Hal Canary <halcanary@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>
This also fixes an issue noticed while making this change where
SkFontDescriptor improperly round trips negative axis values.
Change-Id: Iacc5929a185659dcacc18c802c4908e4f34c6899
Reviewed-on: https://skia-review.googlesource.com/128341
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Bug: b/78866720
::rewind() rewinds to fOriginalOffset
::seek(position) seeks to position + fOriginalOffset
::move(offset) will not move < fOriginalOffset
::getPosition() returns position relative to fOriginalOffset
::getLength() returns full size minus fOriginalOffset
::duplicate() and ::fork() pass on fOriginalOffset
Android may create an SkFILEStream using a file descriptor whose offset
is at the beginning of the data that Android cares about. Treat all
positions in SkFILEStream as relative to that original offset.
This allows AnimatedImageDrawable to read directly from the
SkFILEStream, rather than using an SkFrontBufferedStream and forcing
SkGifCodec to cache data for later use.
This fixes a TODO that was introduced in
https://skia-review.googlesource.com/c/skia/+/9498 and takes it a step
further. In that CL, bungeman@ and I discussed the change and decided to
"leave this alone for now to avoid changing behavior". Doing a code
search today, the only two callers want the new behavior.
Change-Id: I9211394d5b730adf528fac0df0af7a664b1295be
Reviewed-on: https://skia-review.googlesource.com/126511
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Change-Id: I9d84ce1ebbe417160a29ca2221b1df04901238e3
Reviewed-on: https://skia-review.googlesource.com/83541
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
To enable, set skia_embed_resources=true in args.gn.
Also add *-EmbededResouces bots.
Change-Id: Ia69b26e926a3ad4676a4fa021894432ea2104538
Reviewed-on: https://skia-review.googlesource.com/82626
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Should make it easier to ask just for images.
Change-Id: If821743dc924c4bfbc6b2b2d29b14affde7b3afd
Reviewed-on: https://skia-review.googlesource.com/82684
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
This reverts commit 49f1f34438.
Reason for revert: broke win-chrome
../../third_party/skia/src/ports/SkFontMgr_win_dw.cpp(89): error C2228: left of '.release' must have class/struct/union
../../third_party/skia/src/ports/SkFontMgr_win_dw.cpp(89): note: type is 'SkStreamAsset *'
../../third_party/skia/src/ports/SkFontMgr_win_dw.cpp(89): note: did you intend to use '->' instead?
Original change's description:
> use unique_ptr for stream api
>
> Bug: skia:6888
> Change-Id: I3459b4913982a7cae1c0061697c82cc65ad9a2d8
> Reviewed-on: https://skia-review.googlesource.com/26740
> Reviewed-by: Mike Klein <mtklein@chromium.org>
> Commit-Queue: Mike Reed <reed@google.com>
TBR=mtklein@chromium.org,mtklein@google.com,bungeman@google.com,reed@google.com
Change-Id: Ic1e4af557317abd06b7f6b7f5056645df7e469f0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:6888
Reviewed-on: https://skia-review.googlesource.com/47440
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Bug: skia:6888
Change-Id: I3459b4913982a7cae1c0061697c82cc65ad9a2d8
Reviewed-on: https://skia-review.googlesource.com/26740
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
I did some testing with very large datasets, and the difference in max
RSS is measurable and significant.
Change-Id: I6bb2f795d5b4f6ebdba42c3089dc85a278355d48
Reviewed-on: https://skia-review.googlesource.com/9686
Commit-Queue: Hal Canary <halcanary@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
* SkAutoFree moved to SkTemplates.h (now implmented with unique_ptr).
* SkAutoMalloc and SkAutoSMalloc moved to SkAutoMalloc.h
* "SkAutoFree X(sk_malloc_throw(N));" --> "SkAutoMalloc X(N);"
Revert "Revert 'SkTypes.h : move SkAutoMalloc into SkAutoMalloc.h'"
This reverts commit c456b73fef.
Change-Id: Ie2c1a17c20134b8ceab85a68b3ae3e61c24fbaab
Reviewed-on: https://skia-review.googlesource.com/6886
Reviewed-by: Hal Canary <halcanary@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>
* SkAutoFree moved to SkTemplates.h (now implmented with unique_ptr).
* SkAutoMalloc and SkAutoSMalloc moved to SkAutoMalloc.h
* "SkAutoFree X(sk_malloc_throw(N));" --> "SkAutoMalloc X(N);"
Change-Id: Idacd86ca09e22bf092422228599ae0d9bedded88
Reviewed-on: https://skia-review.googlesource.com/4543
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Hal Canary <halcanary@google.com>
This method does the same thing as ::bytesWritten but has a confusing
name. It appears there are no external users, so remove it.
Change-Id: I06aed269200c34c2dda36605092f8ea37fcec693
Reviewed-on: https://skia-review.googlesource.com/6188
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Checking to invalidate this on every write() call has a measurable cost, so removing it both simplifies the class and speeds it up.
BUG=skia:
Change-Id: Idf0baa265c9a0b5d26d82fce948c61ed9b0810b1
Reviewed-on: https://skia-review.googlesource.com/6096
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
It is moved to src/utils. It is almost a tool, but has two uses in
src/ports.
The existing SkOSFile.cpp is left empty for the time being since it is
mentioned in Chromium's BUILD.gn for Skia.
Change-Id: I3bb7f7c4214359eb6ab906bfe76737d20bf1d6c7
Reviewed-on: https://skia-review.googlesource.com/4536
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Ben Wagner <bungeman@google.com>
This doesn't create any apps or bundles or sign anything, but it all compiles and links.
Note the awkward transitional hack I used to make each tool's tool_main() serve as the real main() again when built with GN, while keeping the existing setup with GYP. Fun...
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4350
Change-Id: I632753d5d8e5848380854f413bf5905d676bfcf4
Reviewed-on: https://skia-review.googlesource.com/4350
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>
Replace with std::unique_ptr.
Change-Id: I5806cfbb30515fcb20e5e66ce13fb5f3b8728176
Reviewed-on: https://skia-review.googlesource.com/4381
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
With the move from SkData::NewXXX to SkData::MakeXXX most
SkAutoTUnref<SkData> were changed to sk_sp<SkData>. However,
there are still a few SkAutoTUnref<SkData> around, so clean
them up.
Review-Url: https://codereview.chromium.org/2212493002
When FrontBufferedStream was written, it implemented getPosition()
and returned true for hasPosition(). The CL that introduced it
(crrev.com/23717055) does not have any indication why, but I'm guessing
it was because it was easy to implement. None of our decoders rely on
this (only some tests do).
Now that we have a decoder (SkRawCodec) that expects to be able to seek
a stream if it has a position (which makes sense - SkStream.h
associates that with SkStreamSeekable, and there is no other way to
check to see if a stream is seekable), it is failing because
FrontBufferedStream reports it has a position and the decoder tries to
seek.
Remove FrontBufferedStream::hasPosition() (reverting to the default,
false) and ::getPosition() (so it will return 0).
Fix tests - do not call FrontBufferedStream::getPosition()
Update CodexTest to test using an FrontBufferedStream, like Android
does.
BUG=b/27218441
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1703293002
Review URL: https://codereview.chromium.org/1703293002
If the stream can peek less than requested, peek that amount. Return
the number of bytes peeked.
This simplifies crrev.com/1472123002. For a stream that is smaller than
14 bytes, it can successfully peek, meaning the client will not need to
fall back to read() + rewind(), which may fail if the stream can peek
but not rewind.
This CL revives code from patch set 3 of crrev.com/1044953002, where I
initially introduced peek() (including tests).
Add a test for SkFrontBufferedStream that verifies that peeking does
not make rewind() fail (i.e. by reading past the internal buffer).
BUG=skia:3257
Review URL: https://codereview.chromium.org/1490923005
The test "nullptr == fBlockMemory->fHead->fNext" tests that
all of the data is in one block, but it is also necessary to
test "fBlockMemory->fHead" to ensure that there is a first
block with data in it.
Review URL: https://codereview.chromium.org/1374493002
Add a virtual method on SkStream which will do a "peek" some bytes, so
that those bytes are read, but the next call to read will be
unaffected.
Implement peek for SkMemoryStream, where the implementation is simple
and obvious.
Implement peek on SkFrontBufferedStream.
Add tests.
Motivated by decoding streams which cannot be rewound.
TBR=reed@google.com
BUG=skia:3257
Review URL: https://codereview.chromium.org/1044953002
SkStream is a stateful object, so it does not make sense for it to have
multiple owners. Make SkStream inherit directly from SkNoncopyable.
Update methods which previously called SkStream::ref() (e.g.
SkImageDecoder::buildTileIndex() and SkFrontBufferedStream::Create(),
which required the existing owners to call SkStream::unref()) to take
ownership of their SkStream parameters and delete when done (including
on failure).
Switch all SkAutoTUnref<SkStream>s to SkAutoTDelete<SkStream>s. In some
cases this means heap allocating streams that were previously stack
allocated.
Respect ownership rules of SkTypeface::CreateFromStream() and
SkImageDecoder::buildTileIndex().
Update the comments for exceptional methods which do not affect the
ownership of their SkStream parameters (e.g.
SkPicture::CreateFromStream() and SkTypeface::Deserialize()) to be
explicit about ownership.
Remove test_stream_life, which tested that buildTileIndex() behaved
correctly when SkStream was a ref counted object. The test does not
make sense now that it is not.
In SkPDFStream, remove the SkMemoryStream member. Instead of using it,
create a new SkMemoryStream to pass to fDataStream (which is now an
SkAutoTDelete).
Make other pdf rasterizers behave like SkPDFDocumentToBitmap.
SkPDFDocumentToBitmap delete the SkStream, so do the same in the
following pdf rasterizers:
SkPopplerRasterizePDF
SkNativeRasterizePDF
SkNoRasterizePDF
Requires a change to Android, which currently treats SkStreams as ref
counted objects.
Review URL: https://codereview.chromium.org/849103004
skiatest::Test class is now a simple struct. Some
functionalty, such as counting errors or timing is now
handled elsewhere.
skiatest:Reporter is now a simpler abstract class. The two
implementations handle test errors.
DM and pathops_unittest updated.
Review URL: https://codereview.chromium.org/830513004
