We'll switch to the correct context when necessary (e.g. before
calls that talk to the GPU). This is achieved by adding in
calls at the JS layer to switch the context before making a call
that is known to talk to the GPU (e.g. draw calls on SkCanvas).
Another implementation that was considered was to add a C++
shim in GrGLInterface that would switch the context before
every call in the GPU - however, that seemed too difficult
and would add extra overhead if a single draw* call talks
to the GPU multiple times.
Bug: skia:12255
Change-Id: I96e4c6b41a5bfcc9913aeaca7ccb125358048ad3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/432136
Reviewed-by: Brian Salomon <bsalomon@google.com>
The make_lotties_with_assets is a handy tool to take a
directory of lottie files and move them into their own
folders. This will make it easier to add assets (e.g. fonts,
images) if we want to get more accurate benchmarks.
This also makes the test not stop on the first failure, but complete
all of them, so we can see more easily if multiple things have broken.
Change-Id: Idb2ece71376a11e7524cd513a2798884a44bb59f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/421927
Reviewed-by: Florin Malita <fmalita@google.com>
We also catch any errors that happen during running the
test or flushing. This allows them to show up in the CI logs.
For skottie-frames in particular, we stop clearing the canvas
before each render, as that was masking an error (and not how
we do things on skottie.skia.org).
Change-Id: I83936a35b6c314da76a0a64e15deaabd156e71f2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/421923
Reviewed-by: Kevin Lubick <kjlubick@google.com>
The previous version accidentally retrieved all values from
the in memory glyph cache because it was always measuring
the range 1-300. This changes the glyphID to always be an
increasing number, which avoids the cache hits.
There is now some differences between the font types:
- ttf: ~4ms
- woff: ~160ms
- woff2: ~230ms
The high numbers in the woff categories are expected to be
fixed by https://skia-review.googlesource.com/c/skia/+/420578
Bug: skia:12112
Change-Id: I52f6f7baf27fc28b99ac65a067142281918d7298
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/421324
Reviewed-by: Kevin Lubick <kjlubick@google.com>
As is, all 3 formats have a median time of about 0.12 milliseconds.
Perhaps the performance hit is more strongly felt on larger font
files.
Bug: skia:12112
Change-Id: Ifb4b0aa5a8240d93e94f4b674d0a50e6009887ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/420097
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Bumps [ws](https://github.com/websockets/ws) from 7.3.0 to 7.4.6.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/websockets/ws/releases">ws's releases</a>.</em></p>
<blockquote>
<h2>7.4.6</h2>
<h1>Bug fixes</h1>
<ul>
<li>Fixed a ReDoS vulnerability (00c425ec).</li>
</ul>
<p>A specially crafted value of the <code>Sec-Websocket-Protocol</code> header could be used
to significantly slow down a ws server.</p>
<pre lang="js"><code>for (const length of [1000, 2000, 4000, 8000, 16000, 32000]) {
const value = 'b' + ' '.repeat(length) + 'x';
const start = process.hrtime.bigint();
<p>value.trim().split(/ *, */);</p>
<p>const end = process.hrtime.bigint();</p>
<p>console.log('length = %d, time = %f ns', length, end - start);
}
</code></pre></p>
<p>The vulnerability was responsibly disclosed along with a fix in private by
<a href="https://github.com/robmcl4">Robert McLaughlin</a> from University of California, Santa Barbara.</p>
<p>In vulnerable versions of ws, the issue can be mitigated by reducing the maximum
allowed length of the request headers using the <a href="https://nodejs.org/api/cli.html#cli_max_http_header_size_size"><code>--max-http-header-size=size</code></a>
and/or the <a href="https://nodejs.org/api/http.html#http_http_createserver_options_requestlistener"><code>maxHeaderSize</code></a> options.</p>
<h2>7.4.5</h2>
<h1>Bug fixes</h1>
<ul>
<li>UTF-8 validation is now done even if <code>utf-8-validate</code> is not installed
(23ba6b29).</li>
<li>Fixed an edge case where <code>websocket.close()</code> and <code>websocket.terminate()</code> did
not close the connection (67e25ff5).</li>
</ul>
<h2>7.4.4</h2>
<h1>Bug fixes</h1>
<ul>
<li>Fixed a bug that could cause the process to crash when using the
permessage-deflate extension (92774377).</li>
</ul>
<h2>7.4.3</h2>
<h1>Bug fixes</h1>
<ul>
<li>The deflate/inflate stream is now reset instead of reinitialized when context
takeover is disabled (<a href="https://github-redirect.dependabot.com/websockets/ws/issues/1840">#1840</a>).</li>
</ul>
<h2>7.4.2</h2>
<h1>Bug fixes</h1>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="f5297f7090"><code>f5297f7</code></a> [dist] 7.4.6</li>
<li><a href="00c425ec77"><code>00c425e</code></a> [security] Fix ReDoS vulnerability</li>
<li><a href="990306d144"><code>990306d</code></a> [lint] Fix prettier error</li>
<li><a href="32e3a8439b"><code>32e3a84</code></a> [security] Remove reference to Node Security Project</li>
<li><a href="8c914d18b8"><code>8c914d1</code></a> [minor] Fix nits</li>
<li><a href="fc7e27d12a"><code>fc7e27d</code></a> [ci] Test on node 16</li>
<li><a href="587c201bfc"><code>587c201</code></a> [ci] Do not test on node 15</li>
<li><a href="f672710797"><code>f672710</code></a> [dist] 7.4.5</li>
<li><a href="67e25ff502"><code>67e25ff</code></a> [fix] Fix case where <code>abortHandshake()</code> does not close the connection</li>
<li><a href="23ba6b2922"><code>23ba6b2</code></a> [fix] Make UTF-8 validation work even if utf-8-validate is not installed</li>
<li>Additional commits viewable in <a href="https://github.com/websockets/ws/compare/7.3.0...7.4.6">compare view</a></li>
</ul>
</details>
<br />
[](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.
[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)
---
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/google/skia/network/alerts).
</details>
This is an imported pull request from
https://github.com/google/skia/pull/80
GitOrigin-RevId: a9a98b522212ab86f81c15979a3550ea7e276edb
Change-Id: Ia61edf467fb25d96bed7066b9355975005ca6393
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/415377
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
This is a security fix.
This is an imported pull request from
https://github.com/google/skia/pull/65
GitOrigin-RevId: 38f6e1b1fd360a28135ce0579dbc5521edbcf1a3
Change-Id: I5f38b6f76fcd2943decd5dbe7df7546d4ce2084f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338052
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
Drawing to a Premul Raster Surface and then copying out Unpremul pixels
was 15x slower. Drawing to an Unpremul Raster surface was a little
slower than the previous RasterDirect surface.
Out of an abundance of caution, I'm going back to the way things were.
We can revisit drawing to a Raster surface in the future.
This also adds better error handling to the SkottieFrames test which
thankfully caught this regression.
Bug: skia:10717
Change-Id: Ib8b9d490c92e013c3311adb1d08de5fa0fc0351c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334356
Reviewed-by: Kevin Lubick <kjlubick@google.com>
This is a massive breaking change for all existing users of CanvasKit.
It will be (one of the only) changes in 0.19.0 to make the transition
easier.
Suggested reviewing order:
- index.d.ts (to see type changes). Notice SkPicture still has Sk
prefix, but no other types do (this felt "right" since Sk is
part of the name of the type, but I can be swayed on this).
- canvaskit-wasm-tests.ts
- tests/*.spec.js
- interface.js and helper.js
- html examples
- markdown files
Change-Id: I3b3d3815df2078f986893df3c70101d6248c117d
Docs-Preview: https://skia.org/?cl=322617
Bug: skia:10717
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/322617
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Nathaniel Nifong <nifong@google.com>
I added 3 tests, one using the drawRect API naively,
one using drawRect with a Malloc'd array, and one using
the drawRect4f.
rough local measurements:
- [baseline with tip of tree code]: 50ms
- naive drawRect: 40ms
- drawRect with malloc: 28ms
- drawRect4f: 27ms
I also tried the benchmarks locally with taking in paint
as a const reference. I did not see any changes, but that
could just be small sample size. I plan to land the code
as is for now, collect a bit of data in Perf and then try
landing the const reference stuff and see if we get
something measurable.
To aid this, I added in a helper list of tests to only run
some benchmarks easily.
Change-Id: I2d8c5296e93f05be45fc12059955fb9d9e339d83
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315143
Reviewed-by: Kevin Lubick <kjlubick@google.com>
This is a reland of bdc214a50e
The CPU version of SkottieWasm is timing out for reasons unknown,
but the GPU version is happy. I think we can get rid of the CPU
version of the job since it has been more or less superseded by
the SkottieFrames one (and the latter is more stable).
Original change's description:
> [canvaskit] Change SkRects to be arrays, not objects.
>
> This changes several APIs, so there are lots of breaking
> notes in the Changelog.
>
> This made the "draw 100 colored regions" benchmark about
> 20% faster (1ms -> .8ms).
>
> In theory, rendering should stay the same.
>
> Change-Id: Ib80b15e2d980ad5d568fff4460d2b529766c1b36
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312491
> Reviewed-by: Nathaniel Nifong <nifong@google.com>
Change-Id: I674aba85ecfb30b72e94cbaf89b2d97bfae3b7a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315142
Reviewed-by: Nathaniel Nifong <nifong@google.com>
Revert submission 314622
Reason for revert: breaking wasm bots
Reverted Changes:
Ia1ba13814:[canvaskit] Replace RRect objects with TypedArrays...
Ib80b15e2d:[canvaskit] Change SkRects to be arrays, not objec...
I790b2d6fc:[canvaskit] Add drawRect4f as example 'fast path' ...
Change-Id: Ie6e4c57ba412ca9ff8e4446b06681b51029da2d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314893
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Revert submission 314622
Reason for revert: breaking wasm bots
Reverted Changes:
Ia1ba13814:[canvaskit] Replace RRect objects with TypedArrays...
Ib80b15e2d:[canvaskit] Change SkRects to be arrays, not objec...
I790b2d6fc:[canvaskit] Add drawRect4f as example 'fast path' ...
Change-Id: I295791532a2d27bff41f706fa8ed37e0499c27d7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314892
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
I added 3 tests, one using the drawRect API naively,
one using drawRect with a Malloc'd array, and one using
the drawRect4f.
rough local measurements:
- [baseline with tip of tree code]: 50ms
- naive drawRect: 40ms
- drawRect with malloc: 28ms
- drawRect4f: 27ms
I also tried the benchmarks locally with taking in paint
as a const reference. I did not see any changes, but that
could just be small sample size. I plan to land the code
as is for now, collect a bit of data in Perf and then try
landing the const reference stuff and see if we get
something measurable.
To aid this, I added in a helper list of tests to only run
some benchmarks easily.
Change-Id: I790b2d6fc32c125a4d371768fffb19ab22633d4e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314622
Reviewed-by: Kevin Lubick <kjlubick@google.com>
This changes several APIs, so there are lots of breaking
notes in the Changelog.
This made the "draw 100 colored regions" benchmark about
20% faster (1ms -> .8ms).
In theory, rendering should stay the same.
Change-Id: Ib80b15e2d980ad5d568fff4460d2b529766c1b36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312491
Reviewed-by: Nathaniel Nifong <nifong@google.com>
Adds the command:
make skps_release_and_SIMD
for perfing builds against a set of SKPs in ~/skps for release and
simd builds of CanvasKit. Also outputs a summary of the perf results
in a table format.
See the document "SIMD CanvasKit Build Performance Testing"
for more details:
https://docs.google.com/document/d/114kdSGPMnOSQCZ7pFgd3MGMn5mIW562RMoXVmD13e0M/edit#
Bug: skia:10453
Change-Id: I311629a1420301dda41f7ec57ce1403b05fd949b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/301982
Reviewed-by: Elliot Evans <elliotevans@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
For each skp in the corpus, we start a fresh instance of
Chromium (via puppeteer), draw the skp and measure that time.
This process is repeated a fixed amount of repetitions
and the median, the average, and the std deviation is reported
to perf (as well as the individual datapoints as an FYI).
Importantly (and something we'll need to change about
SkottieFrames), we measure the average time between frames
after unlocking the framerate. This ensures we account for
the time needed by the GPU to actually draw (flush() returns
after the GPU has all the instructions, but not necessarily
has been able to draw).
This implementation is very similar to the SkottieFrames
code; a notable deviation is the repetitions are handled
outside of the html, i.e. a new chrome window per run.
I explored using content_shell, but noticed that requires
building Chromium, which our infrastructure is not set up
to do well.
Change-Id: I14fdbdc951604d3fdf06e81a4be7e614d0e53c03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/295079
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Nathaniel Nifong <nifong@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
and rotations both have an effect on cache usage. Snapping translations to integer coordinations
reduces cache usage. Opacity of path painting does not have an effect on cache usage.
Bug: skia:10272
Change-Id: Id5d5f08cb43645c9ec44b9d8e5e96643041727c3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292280
Reviewed-by: Kevin Lubick <kjlubick@google.com>
IMPORTANT LESSON: when bringing in node (and possibly other
executables) via CIPD, add them to the path in gen_tasks_logic
so the parent executable (the task driver itself) has the right
PATH set. Otherwise, the subprocesses it spawns might grab the
wrong version because of how golang handles environments of
subprocesses.
This is starting as a fork of Skottie WASM. I hope to have a more unified
system for creating and running benchmarks.
Overall overview:
gen_tasks_logic.go creates a task in task.json that compiles
CanvasKit and the task drivers and then executes our task
(i.e. perf_puppeteer.go)
perf_puppeteer runs a node program (perf-with-puppeteer.js)
that uses puppeteer to execute benchmarking code on an
html page (canvaskit-skottie-frames-load.html).
I needed to update the node package so npm could be updated from
3.x to 6.14.4 so it knew about `npm ci`. This may not have been
entirely necessary, given the problems of executing the correct
npm (see important lesson above), but it hasn't broken things
further, so more up-to-date is probably a good thing.
Suggested Review Order:
- canvaskit-skottie-frames-load.html (note it is similar to
skottie-wasm-perf.html, but it waits for a button click
to start animating and records times from the main JS thread
itself)
- perf-with-puppeteer.js (similar to skottie-wasm-perf.js, but
has some things made optional [e.g. tracing])
- perf_puppeteer_test.go (shows the inputs/outputs of various steps)
- perf_puppeteer.go
- Everything else.
Change-Id: I380e81b825f36682c257664d488267edaf36369e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/285783
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
