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>
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-string-compare.html
Find suspicious usage of runtime string comparison functions.
This check is valid in C and C++.
Checks for calls with implicit comparator and proposed to
explicitly add it:
if (strcmp(...)) // Implicitly compare to zero
if (!strcmp(...)) // Won't warn
if (strcmp(...) != 0) // Won't warn
Checks that compare function results (i,e, strcmp) are compared to valid
constant. The resulting value is
< 0 when lower than,
> 0 when greater than,
== 0 when equals.
A common mistake is to compare the result to 1 or -1:
if (strcmp(...) == -1) // Incorrect usage of the returned value.
Additionally, the check warns if the results value is implicitly cast
to a suspicious non-integer type. It’s happening when the returned
value is used in a wrong context:
if (strcmp(...) < 0.) // Incorrect usage of the returned value.
Change-Id: I001b88d06cc4f3eb5846103885be675f9b78e126
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310761
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@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>
Remove REPORTER_ASSERT_MESSAGE.
Change-Id: I6d00715901159c93e22d182fe24aac92b5fdbcf4
Reviewed-on: https://skia-review.googlesource.com/100361
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
This reverts commit 88757dacd4.
Reason for revert: Still seems to be failing Chromium "telemetry_perf_unittests (with patch) on Android" on android_n5x_swarming_rel.
Original change's description:
> guard old apis for querying byte-size of a bitmap/imageinfo/pixmap
>
> Now with legacy behavior for allocpixels
>
> This was reverted, so the current CL is a "fix" on top of ...
> https://skia-review.googlesource.com/c/skia/+/50980
>
> Related update to Chrome (in preparation for this change)
> https://chromium-review.googlesource.com/c/chromium/src/+/685719
>
> Bug: skia:
> Change-Id: I4b370ee7e95083ab27421f008132219c9c7b86e9
> Reviewed-on: https://skia-review.googlesource.com/51341
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> Commit-Queue: Mike Reed <reed@google.com>
TBR=fmalita@chromium.org,reed@google.com
Change-Id: I827a0ca1d1e3909e648fde3342cdb8601d34da8d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/52381
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This reverts commit 98a6216b18.
Reason for revert: breaking the chrome roll. Looks like they may be writing data to create an image across all the row bytes and thus writing to unalloced data on the last row. Link to example failing bot:
https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/539960
Original change's description:
> guard old apis for querying byte-size of a bitmap/imageinfo/pixmap
>
> Previously we had size_t and uint64_t variations.
>
> The new (simpler) API always..
> - returns size_t, or 0 if the calculation overflowed
> - returns the trimmed size (does not include rowBytes padding for the last row)
>
> Bug: skia:
> Change-Id: I05173e877918327c7b207d2f7f1ab0db36892e2e
> Reviewed-on: https://skia-review.googlesource.com/50980
> Commit-Queue: Mike Reed <reed@google.com>
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> Reviewed-by: Leon Scroggins <scroggo@google.com>
TBR=mtklein@google.com,herb@google.com,scroggo@google.com,fmalita@chromium.org,reed@google.com
Change-Id: I726f6ab1b36b14979ba6f37105e0a469b3f0dbc0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:
Reviewed-on: https://skia-review.googlesource.com/51262
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Previously we had size_t and uint64_t variations.
The new (simpler) API always..
- returns size_t, or 0 if the calculation overflowed
- returns the trimmed size (does not include rowBytes padding for the last row)
Bug: skia:
Change-Id: I05173e877918327c7b207d2f7f1ab0db36892e2e
Reviewed-on: https://skia-review.googlesource.com/50980
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Realized that a pending CL needed to add (yet another) private type to SkRecords.h, but w/o this CL I'd be forced to move that header also into private. This change frees us up to not have transitive exposure for types that need to be recorded.
Bug: skia:
Change-Id: Id79f1c2e44ba85e063c1360cf96c92de6397ca2b
Reviewed-on: https://skia-review.googlesource.com/17031
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
This is step one:
- make SkXfermode useless to public clients
- everything they should need is in SkBlendMode.h
Step two:
- remove SkXfermode.h entirely (since skia core will already be using SkXfermodePriv.h)
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4534
Change-Id: If2cea9f71df92430ed6644edb98dd306c5572cbc
Reviewed-on: https://skia-review.googlesource.com/4534
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
- perform version check in CreateProc for XfermodeImageFilter and ArithmeticImageFilter
This reverts commit 3ed485f424.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2992
Change-Id: Ib4a154cdd5f5d1dcac921ef50d53b79a2d6a1be8
Reviewed-on: https://skia-review.googlesource.com/2992
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Feature-wise, this removes:
1) BBH support;
2) peephole optimizations;
3) record-time text op specializations;
4) the guarantee that SkPaints are flattened.
This deletes the optimizations GM, which only exists to test the peepholes of
the old backend. SkRecord optimizations are unit tested, and if that ever fails we
can think about adding another GM like this, but they're different enough we'd
want to start from scratch anyway.
We need to keep the code that plays back the specialized text ops around for
a while for compatibility with existing .SKPs that have those ops recorded.
BUG=skia:
CQ_EXTRA_TRYBOTS=tryserver.skia:Canary-Chrome-Ubuntu13.10-Ninja-x86_64-ToT-Trybot
R=robertphillips@google.com, reed@google.com, mtklein@google.com
Author: mtklein@chromium.org
Review URL: https://codereview.chromium.org/617953002
