aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/core
Commit message (Collapse)AuthorAge
* store info in basedevice, change getter to non-virtual const&Gravatar reed2016-08-20
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2261003003 Review-Url: https://codereview.chromium.org/2261003003
* Move GrFixedClip into src directoryGravatar csmartdalton2016-08-19
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2254343003 Review-Url: https://codereview.chromium.org/2254343003
* Plumb drawArc to SkDevice.Gravatar bsalomon2016-08-19
| | | | | | | | | Plumbs the drawArc canvas method down to SkDevice without converting to a path. Plumbs through the various recording canvas classes. BUG=skia:5227 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257023003 Review-Url: https://codereview.chromium.org/2257023003
* Added SkColor4f::FromColor3f(SkColor3f, float a)Gravatar dvonbeck2016-08-19
| | | | | | | | | Planning to use this for the ColorDisplay widget in https://codereview.chromium.org/2259183003/ BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2264643002 Review-Url: https://codereview.chromium.org/2264643002
* Fast path translate() in SkCanvas and SkLiteDL.Gravatar mtklein2016-08-19
| | | | | | | | | | | | | | | | | | | | | | | This adds didTranslate() so that SkLiteDL (and other canvas recorders) can record the translate rather than the full concat. It also adds a case to SkMatrix::preTranslate() to fast path translate x translate -> translate (i.e. +=). BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255283002 Committed: https://skia.googlesource.com/skia/+/5fa47f4fd13b3158de4599414c86d17649c2dd1c Misc bots failing in pictureimagefilter replay modes. https://luci-milo.appspot.com/swarming/task/30b8e53f3a1f4f10/steps/dm/0/stdout Problem is FMA vs. not. CQ_INCLUDE_TRYBOTS=master.client.skia: Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Fast-Trybot Review-Url: https://codereview.chromium.org/2255283002
* Simplify embeddability test.Gravatar bungeman2016-08-19
| | | | | | | | | There is no reason to read the whole OS/2 table, and no need to spell out the full names of all the types over and over. GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2256783002 Review-Url: https://codereview.chromium.org/2256783002
* Fix bug, always keep fIsScaleTranslate in correct stateGravatar msarett2016-08-19
| | | | | | | BUG:639179 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2263513003 Review-Url: https://codereview.chromium.org/2263513003
* Revert of Fast path translate() in SkCanvas and SkLiteDL. (patchset #5 ↵Gravatar mtklein2016-08-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | id:80001 of https://codereview.chromium.org/2255283002/ ) Reason for revert: speculative Original issue's description: > Fast path translate() in SkCanvas and SkLiteDL. > > This adds didTranslate() so that SkLiteDL (and other canvas recorders) > can record the translate rather than the full concat. > > It also adds a case to SkMatrix::preTranslate() to fast path > translate x translate -> translate (i.e. +=). > > BUG=skia: > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255283002 > > Committed: https://skia.googlesource.com/skia/+/5fa47f4fd13b3158de4599414c86d17649c2dd1c TBR=herb@google.com,reed@google.com,mtklein@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia: Review-Url: https://codereview.chromium.org/2264433002
* quick check to not use AutoDrawLooperGravatar reed2016-08-18
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2248373004 Review-Url: https://codereview.chromium.org/2248373004
* Detect CRC32 instructions on ARMv8.Gravatar mtklein2016-08-18
| | | | | | | | | | | | | I have successfully detected CRC32 instruction support on my Nexus 5x. Use of these instructions to follow... I am not yet sure which compilers if any will give me instrinsics or let me write them in asm. defined(__ARM_FEATURE_CRC32) should cover users like Android Framework who build with the best settings possible. cpu-features.h covers use cases like Clank and our bots. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2259133002 Review-Url: https://codereview.chromium.org/2259133002
* Batched implementation of drawLattice() for GPUGravatar msarett2016-08-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bechmarks (Nexus 6P): Src=100x100, Dst=250x250, NumRects=9 Android 77.7us Skia (without patch) 57.2us Skia (with patch) 30.9us Src=100x100, Dst=500x500, NumRects=9 Android 77.0us Skia (without patch) 56.9us Skia (with patch) 31.8us Src=100x100, Dst=1000x1000, NumRects=9 Android 180us Skia (without patch) 96.8us Skia (with patch) 70.5us Src=100x100, Dst=250x250, NumRects=15 Android 208us Skia (without patch) 155us Skia (with patch) 38.2us Src=100x100, Dst=500x500, NumRects=15 Android 207us Skia (without patch) 152us Skia (with patch) 38.4us Src=100x100, Dst=1000x1000, NumRects=15 Android 233us Skia (without patch) 156us Skia (with patch) 99.9us BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255963002 Committed: https://skia.googlesource.com/skia/+/93242c4ae50dfcc0d922cdb3ba80bbc7b4bbe93d Review-Url: https://codereview.chromium.org/2255963002
* Fast path translate() in SkCanvas and SkLiteDL.Gravatar mtklein2016-08-18
| | | | | | | | | | | | | This adds didTranslate() so that SkLiteDL (and other canvas recorders) can record the translate rather than the full concat. It also adds a case to SkMatrix::preTranslate() to fast path translate x translate -> translate (i.e. +=). BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255283002 Review-Url: https://codereview.chromium.org/2255283002
* Revert of Batched implementation of drawLattice() for GPU (patchset #7 ↵Gravatar msarett2016-08-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | id:180001 of https://codereview.chromium.org/2255963002/ ) Reason for revert: Things drawing weird. Original issue's description: > Batched implementation of drawLattice() for GPU > > Bechmarks (Nexus 6P): > > Src=100x100, Dst=250x250, NumRects=9 > Android 77.7us > Skia (without patch) 57.2us > Skia (with patch) 34.7us > > Src=100x100, Dst=500x500, NumRects=9 > Android 77.0us > Skia (without patch) 56.9us > Skia (with patch) 44.5us > > Src=100x100, Dst=1000x1000, NumRects=9 > Android 180us > Skia (without patch) 96.8us > Skia (with patch) 70.5us > > Src=100x100, Dst=250x250, NumRects=15 > Android 208us > Skia (without patch) 155us > Skia (with patch) 55.9us > > Src=100x100, Dst=500x500, NumRects=15 > Android 207us > Skia (without patch) 152us > Skia (with patch) 63.0us > > Src=100x100, Dst=1000x1000, NumRects=15 > Android 233us > Skia (without patch) 156us > Skia (with patch) 99.9us > > BUG=skia: > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255963002 > > Committed: https://skia.googlesource.com/skia/+/93242c4ae50dfcc0d922cdb3ba80bbc7b4bbe93d TBR=bsalomon@google.com,reed@google.com,djsollen@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia: Review-Url: https://codereview.chromium.org/2255683004
* Fix initialization bug for fConservativeIsScaleTranslateGravatar msarett2016-08-18
| | | | | | | | | | | Also don't mark it as conservative, we expect it to be correct all the time. TBR=reed@google.com BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2261573002 Review-Url: https://codereview.chromium.org/2261573002
* Batched implementation of drawLattice() for GPUGravatar msarett2016-08-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bechmarks (Nexus 6P): Src=100x100, Dst=250x250, NumRects=9 Android 77.7us Skia (without patch) 57.2us Skia (with patch) 34.7us Src=100x100, Dst=500x500, NumRects=9 Android 77.0us Skia (without patch) 56.9us Skia (with patch) 44.5us Src=100x100, Dst=1000x1000, NumRects=9 Android 180us Skia (without patch) 96.8us Skia (with patch) 70.5us Src=100x100, Dst=250x250, NumRects=15 Android 208us Skia (without patch) 155us Skia (with patch) 55.9us Src=100x100, Dst=500x500, NumRects=15 Android 207us Skia (without patch) 152us Skia (with patch) 63.0us Src=100x100, Dst=1000x1000, NumRects=15 Android 233us Skia (without patch) 156us Skia (with patch) 99.9us BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255963002 Review-Url: https://codereview.chromium.org/2255963002
* make LayerIter private, and remove skipClip optionGravatar reed2016-08-18
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257203002 Review-Url: https://codereview.chromium.org/2257203002
* remove disable aa/dither flags from Props -- never usedGravatar reed2016-08-18
| | | | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2257643003 TBR=bsalomon Review-Url: https://codereview.chromium.org/2257643003
* Eagerly update the unichar to glyphID mapping.Gravatar herb2016-08-17
| | | | | | | | | In addition, some small cleanups. BUG=chromium:635005 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2249063005 Review-Url: https://codereview.chromium.org/2249063005
* SkLiteDL: a little spring cleaningGravatar mtklein2016-08-17
| | | | | | | | | | | | | | | - kill off optimizeFor() now that we know it's broken and have a better way; - simplify makeThreadsafe() a bit: we're not calling it anyway. This sort of earns me back some crazy code currency, which I am going to try to spend making reset() faster. If anything, this appears to make makeThreadsafe() _faster_. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2254913002 Review-Url: https://codereview.chromium.org/2254913002
* pin as texture apiGravatar reed2016-08-17
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2241353002 Review-Url: https://codereview.chromium.org/2241353002
* SkLiteDL: inline empty()Gravatar mtklein2016-08-17
| | | | | | | | | | | 1.85% 1.85% RenderThread 6458 6475 /system/lib64/libskia.so SkLiteDL::empty() const :/ BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2254823002 Review-Url: https://codereview.chromium.org/2254823002
* move private test for sect_with_horizontal into unittestsGravatar reed2016-08-17
| | | | | | | BUG=638575 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2250353004 Review-Url: https://codereview.chromium.org/2250353004
* SkLiteDL: thread the original canvas matrix through for SetMatrix::Draw().Gravatar mtklein2016-08-16
| | | | | | | | | | | | | | | The SkLiteDL is recorded in some identity space (imagine, SkMatrix::I()), but played back in a different one (here named SkMatrix original). Any calls to setMatrix() need to be made relative to this new space. All other ops already operate in relative coordinates. This should let us not fiddle with setMatrix(). BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2247353003 Review-Url: https://codereview.chromium.org/2247353003
* SkPDF: SkPDFFont class changesGravatar halcanary2016-08-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | SkPDFFont: * inline some one-line methdods. - SkPDFFont::typeface() - SkPDFFont::fontInfo() - SkPDFFont::firstGlyphID() - SkPDFFont::lastGlyphID() - SkPDFFont::getFontDescriptor() * de-virtualize some methods: - SkPDFFont::getType() - SkPDFFont::multiByteGlyphs() * Constructor takes more arguments: fontType, multiByteGlyphs * re-order fields (pointers before shorts) * use sk_sp<T> more, T* less SkAdvancedTypefaceMetrics: * SkAdvancedTypefaceMetrics::fFont now a uint8_t * other enumes are sized. * SkAdvancedTypefaceMetrics::fStyle now big enough. * remove use of SkTBitOr, replaced with fancy templates No public API changes. TBR=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2246903002 Review-Url: https://codereview.chromium.org/2246903002
* SkLiteDL: remove freelisting, add reset() and SKLITEDL_PAGE knob.Gravatar mtklein2016-08-16
| | | | | | | | | | | | We think Android can cache these better than a global freelist allows. This removes the freelisting but adds reset() to allow reuse. I took the opportunity to abstract 4096 as a define SKLITEDL_PAGE. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2248693004 Review-Url: https://codereview.chromium.org/2248693004
* Add onDrawBitmapLattice(), avoid unnecessary bitmap->image copyGravatar msarett2016-08-16
| | | | | | | | | | | | | out/Release/nanobench --match Lattice --config gpu --ms 3000 3.42ms -> 17.2us For reference, a loop over drawBitmapRects (which is what Android currently does) is about 13us. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2205273003 Review-Url: https://codereview.chromium.org/2205273003
* Remove isOpaque from SkSpecialImage, just use alpha typeGravatar brianosman2016-08-16
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2246193002 Review-Url: https://codereview.chromium.org/2246193002
* Attempt to throw away rrect clips of rrects.Gravatar bsalomon2016-08-15
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2241273003 Review-Url: https://codereview.chromium.org/2241273003
* Revert of add parallel public API for recording SkLiteDL. (patchset #1 id:1 ↵Gravatar mtklein2016-08-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | of https://codereview.chromium.org/2246893002/ ) Reason for revert: looking like we won't need this Original issue's description: > add parallel public API for recording SkLiteDL. > > The API is restricted to pretty much just what Derek calls, > but it's enough that we can switch testing over to use it. > > BUG=skia: > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2246893002 > > Committed: https://skia.googlesource.com/skia/+/ced26a3d6b77d3a6744a8ccb8eff23eda45fc867 TBR=djsollen@google.com,reed@google.com,mtklein@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia: Review-Url: https://codereview.chromium.org/2243393002
* add gm that exercises compose shader allocationsGravatar caryclark2016-08-15
| | | | | | | | | | | | This gm triggers the assert in SkSmallAllocator.h commented out by this CL. PDFium constructs shaders that resemble the GM. R=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2249703002 Review-Url: https://codereview.chromium.org/2249703002
* just add SkLiteDL::empty()Gravatar mtklein2016-08-15
| | | | | | | | | TBR=djsollen@google.com BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2249713002 Review-Url: https://codereview.chromium.org/2249713002
* LightingFP now supports multiple directional lightsGravatar dvonbeck2016-08-15
| | | | | | | BUG=skia:5518 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2239933004 Review-Url: https://codereview.chromium.org/2239933004
* Revert of increase small allocator limit to encompass compose shader ↵Gravatar caryclark2016-08-15
| | | | | | | | | | | | | | | | | | | | | | | (patchset #1 id:1 of https://codereview.chromium.org/2234423005/ ) Reason for revert: Increase in stack broke Google3 Original issue's description: > increase small allocator limit to encompass compose shader > > R=reed@google.com > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2234423005 > > Committed: https://skia.googlesource.com/skia/+/8c200694f8eec5f623b3934b85c31dbb382be12c TBR=reed@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review-Url: https://codereview.chromium.org/2244293002
* Optimize color xforms when src and dst are matchingGravatar msarett2016-08-15
| | | | | | | | | | | | | | | Useful when: (1) Client does not realize src and dst match (calls color xform anyway). (2) Client wants half floats, src and dst have matching gamuts (3) Client wants premul (done correctly in linear space), src and dst have matching gamuts. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2206403003 Review-Url: https://codereview.chromium.org/2206403003
* add parallel public API for recording SkLiteDL.Gravatar mtklein2016-08-15
| | | | | | | | | | The API is restricted to pretty much just what Derek calls, but it's enough that we can switch testing over to use it. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2246893002 Review-Url: https://codereview.chromium.org/2246893002
* increase small allocator limit to encompass compose shaderGravatar caryclark2016-08-14
| | | | | | | R=reed@google.com GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2234423005 Review-Url: https://codereview.chromium.org/2234423005
* Assert fDeviceClipBounds is always empty when clip is emptyGravatar msarett2016-08-14
| | | | | | | | | | | | | | | | I've fixed a few issues where this assert triggers. The nastiest case is when the client sets the clip to a strange version of an empty rect (Ex: L,T,R,B=0,0,0,10). I think I've made sufficient fixes so that fDeviceClipBounds is always empty when clip is empty. Let's test it though. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2246503003 Review-Url: https://codereview.chromium.org/2246503003
* Prevent overflows when using gamma_alloc_sizeGravatar gogil2016-08-14
| | | | | | | BUG=636268 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2230163002 Review-Url: https://codereview.chromium.org/2230163002
* Added PointLights to SkLights::LightGravatar vjiaoblack2016-08-12
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2237493002 Review-Url: https://codereview.chromium.org/2237493002
* Optimized implementation of quickReject()Gravatar msarett2016-08-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Impl Overview (1) Keep the device clip bounds up to date. This requires minimal additional work in a few places throughout canvas. (2) Keep track of if the ctm isScaleTranslate. Yes, there's a function that does this, but it's slow to call. (3) Perform the src->device transform in quick reject, then check intersection/nan. Other Notes: (1) NaN and intersection checks are performed simultaneously. (2) We no longer quick reject infinity. (3) Affine and perspective are both handled in the slow case. (4) SkRasterClip::isEmpty() is handled by the intersection check. Performance on Nexus 6P: 93.2ms -> 59.8ms Overall Android Jank Tests Performance Impact: Should gain us a ms or two on some tests. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2225393002 Committed: https://skia.googlesource.com/skia/+/d22a817ff57986407facd16af36320fc86ce02da Review-Url: https://codereview.chromium.org/2225393002
* Revert of Optimized implementation of quickReject() (patchset #12 id:260001 ↵Gravatar mtklein2016-08-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | of https://codereview.chromium.org/2225393002/ ) Reason for revert: New assert triggering in the Chrome roll, https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/208750/steps/webkit_unit_tests%20%28with%20patch%29%20on%20Ubuntu-12.04/logs/FrameThrottlingTest.SynchronousLayoutInAnimationFrameCallback and breaks the SKNX_NO_SIMD bot, https://codereview.chromium.org/2236363004 Original issue's description: > Optimized implementation of quickReject() > > Impl Overview > (1) Keep the device clip bounds up to date. This > requires minimal additional work in a few places > throughout canvas. > (2) Keep track of if the ctm isScaleTranslate. Yes, > there's a function that does this, but it's slow > to call. > (3) Perform the src->device transform in quick reject, > then check intersection/nan. > > Other Notes: > (1) NaN and intersection checks are performed > simultaneously. > (2) We no longer quick reject infinity. > (3) Affine and perspective are both handled in the slow > case. > (4) SkRasterClip::isEmpty() is handled by the intersection > check. > > Performance on Nexus 6P: > 93.2ms -> 59.8ms > > Overall Android Jank Tests Performance Impact: > Should gain us a ms or two on some tests. > > BUG=skia: > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2225393002 > > Committed: https://skia.googlesource.com/skia/+/d22a817ff57986407facd16af36320fc86ce02da TBR=reed@google.com,herb@google.com,msarett@google.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia: Review-Url: https://codereview.chromium.org/2231393003
* Optimized implementation of quickReject()Gravatar msarett2016-08-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Impl Overview (1) Keep the device clip bounds up to date. This requires minimal additional work in a few places throughout canvas. (2) Keep track of if the ctm isScaleTranslate. Yes, there's a function that does this, but it's slow to call. (3) Perform the src->device transform in quick reject, then check intersection/nan. Other Notes: (1) NaN and intersection checks are performed simultaneously. (2) We no longer quick reject infinity. (3) Affine and perspective are both handled in the slow case. (4) SkRasterClip::isEmpty() is handled by the intersection check. Performance on Nexus 6P: 93.2ms -> 59.8ms Overall Android Jank Tests Performance Impact: Should gain us a ms or two on some tests. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2225393002 Review-Url: https://codereview.chromium.org/2225393002
* LightingShader and NormalSource comment and style fixesGravatar dvonbeck2016-08-11
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2237963002 Review-Url: https://codereview.chromium.org/2237963002
* Delete quickRejectY()Gravatar msarett2016-08-11
| | | | | | | | | | This is the first step in a refactor of quickReject(). TBR=reed@google.com BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2241473002 Review-Url: https://codereview.chromium.org/2241473002
* SkLiteDL: turn vtable sidewaysGravatar mtklein2016-08-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of using virtuals for polymorphism, store each polymorphic operation in its own array of function pointers. This removes an indirection when calling the function, and lets us use nullptr as a signal to skip the call entirely. Unfortunately (as the old code is rather simpler) this makes an unambiguous speed improvement. Before: curr/maxrss loops min median mean max stddev samples config bench 21/26 MB 2 44.6µs 46.4µs 48.3µs 274µs 13% 51724 nonrendering desk_nytimes.skp 23/26 MB 11 11.4µs 11.9µs 12.4µs 75.1µs 15% 36716 nonrendering keymobi_nytimes_com_.skp After: 19/26 MB 4 40.4µs 41.6µs 42.5µs 197µs 10% 29379 nonrendering desk_nytimes.skp 20/26 MB 14 10.5µs 10.7µs 11.2µs 59.5µs 13% 31971 nonrendering keymobi_nytimes_com_.skp Broader comparison: keymobi_linkedin.skp 1.76us -> 1.77us 1x keymobi_bing_com_search_q_sloth.skp 5.09us -> 5.05us 0.99x keymobi_digg_com.skp 17.4us -> 17.3us 0.99x keymobi_theverge_com.skp 3.37us -> 3.34us 0.99x top25desk_mail_google_com_mail_.skp 30.8us -> 30.4us 0.99x tabl_gmail.skp 3.44us -> 3.38us 0.98x top25desk_wikipedia__1_tab_.skp 100us -> 97.7us 0.98x keymobi_cnn_com_2012_10_03_poli.skp 52.9us -> 51.7us 0.98x desk_chalkboard.skp 107us -> 104us 0.97x desk_css3gradients.skp 17.8us -> 17.3us 0.97x keymobi_androidpolice_com_2012_.skp 42.3us -> 41.1us 0.97x desk_googlehome.skp 1.94us -> 1.88us 0.97x keymobi_mlb_com_.skp 5.38us -> 5.18us 0.96x top25desk_pinterest.skp 92.1us -> 88.5us 0.96x keymobi_iphone_capitolvolkswage.skp 15.1us -> 14.5us 0.96x keymobi_techcrunch_com.skp 9.45us -> 9.05us 0.96x desk_espn.skp 31.3us -> 30us 0.96x keymobi_slashdot_org_.skp 8.72us -> 8.34us 0.96x desk_tiger8svg.skp 30.6us -> 29.2us 0.96x keymobi_blogger.skp 4.09us -> 3.91us 0.95x keymobi_baidu_com_s_wd_barack_o.skp 11.9us -> 11.3us 0.95x keymobi_cuteoverload_com.skp 54.2us -> 51.6us 0.95x keymobi_deviantart_com_.skp 17.2us -> 16.4us 0.95x desk_mapsvg.skp 163ns -> 155ns 0.95x keymobi_pinterest.skp 6.97us -> 6.62us 0.95x top25desk_games_yahoo_com.skp 94.1us -> 89.3us 0.95x top25desk_google_com_calendar_.skp 18us -> 17us 0.95x keymobi_google_co_uk_search_hl_.skp 11.4us -> 10.8us 0.95x tabl_pravda.skp 38.5us -> 36.4us 0.94x desk_gmailthread.skp 19us -> 17.9us 0.94x keymobi_reddit_com_r_programmin.skp 76.1us -> 71.7us 0.94x top25desk_linkedin.skp 20us -> 18.8us 0.94x tabl_gamedeksiam.skp 118us -> 112us 0.94x keymobi_boingboing_net.skp 20.4us -> 19.1us 0.93x top25desk_google_com__hl_en_q_b.skp 17.6us -> 16.4us 0.93x keymobi_amazon_com_gp_aw_s_ref_.skp 12.5us -> 11.6us 0.93x keymobi_sfgate_com_.skp 16.6us -> 15.5us 0.93x keymobi_booking_com_searchresul.skp 16.2us -> 15.1us 0.93x tabl_digg.skp 28.8us -> 26.8us 0.93x tabl_mozilla.skp 80.4us -> 74.6us 0.93x desk_wowwiki.skp 39.2us -> 36.4us 0.93x top25desk_youtube_com.skp 42us -> 38.9us 0.93x top25desk_facebook.skp 23.7us -> 22us 0.93x top25desk_blogger.skp 38.2us -> 35.4us 0.93x keymobi_online_wsj_com_home_pag.skp 12.8us -> 11.9us 0.93x top25desk_wordpress.skp 28.9us -> 26.8us 0.93x top25desk_answers_yahoo_com.skp 37.2us -> 34.4us 0.93x keymobi_plus_google_com_app_bas.skp 9.56us -> 8.85us 0.93x keymobi_wordpress.skp 16.1us -> 14.9us 0.92x keymobi_mobile_news_sandbox_goo.skp 27.1us -> 24.9us 0.92x top25desk_techcrunch_com.skp 31.1us -> 28.6us 0.92x keymobi_worldjournal_com_.skp 50.7us -> 46.5us 0.92x keymobi_theverge_com_2012_10_28.skp 26.2us -> 24us 0.92x desk_jsfiddlebigcar.skp 1.73us -> 1.59us 0.92x top25desk_weather_com.skp 31.3us -> 28.7us 0.92x top25desk_google_com_search_q_c.skp 48.2us -> 44.1us 0.92x top25desk_twitter.skp 27.8us -> 25.5us 0.92x tabl_worldjournal.skp 29.3us -> 26.8us 0.91x desk_nytimes.skp 46us -> 42us 0.91x top25desk_docs___1_open_documen.skp 6.04us -> 5.51us 0.91x keymobi_wikipedia__1_tab_.skp 59.7us -> 54.4us 0.91x desk_unicodetable.skp 1.12ms -> 1.02ms 0.91x top25desk_ebay_com.skp 17.8us -> 16.2us 0.91x keymobi_ftw_usatoday_com_2014_0.skp 26.8us -> 24.3us 0.91x top25desk_sports_yahoo_com_.skp 49.9us -> 45.3us 0.91x keymobi_cnn_com.skp 9.94us -> 9.03us 0.91x keymobi_m_youtube_com_watch_v_9.skp 13.4us -> 12.2us 0.91x top25desk_amazon_com.skp 26.6us -> 24.1us 0.91x keymobi_news_yahoo_com.skp 17.5us -> 15.8us 0.9x keymobi_wowwiki_com_world_of_wa.skp 11.2us -> 10.2us 0.9x top25desk_plus_google_com_11003.skp 93.5us -> 84.4us 0.9x desk_carsvg.skp 53.5us -> 48.2us 0.9x top25desk_news_yahoo_com.skp 44.7us -> 40.3us 0.9x keymobi_wikipedia__1_tab____del.skp 59.4us -> 53.4us 0.9x desk_googlespreadsheet.skp 66us -> 59.2us 0.9x keymobi_answers_yahoo_com_quest.skp 30.2us -> 27us 0.89x desk_ugamsolutions.skp 13us -> 11.6us 0.89x keymobi_shop_mobileweb_ebay_com.skp 6.96us -> 6.21us 0.89x keymobi_nytimes_com_.skp 12.1us -> 10.8us 0.89x keymobi_gsp_ro.skp 5.54us -> 4.92us 0.89x top25desk_booking_com.skp 54.9us -> 48.6us 0.89x top25desk_espn.skp 37us -> 32.6us 0.88x keymobi_facebook_com_barackobam.skp 23.3us -> 20.4us 0.88x BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2231103002 Committed: https://skia.googlesource.com/skia/+/ac243914af957a806d842318a43dddaf5f941dc3 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_compile_dbg_ng Review-Url: https://codereview.chromium.org/2231103002
* Revert of SkLiteDL: turn vtable sideways (patchset #7 id:120001 of ↵Gravatar reed2016-08-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | https://codereview.chromium.org/2231103002/ ) Reason for revert: speculative revert to fix DEPS roll Original issue's description: > SkLiteDL: turn vtable sideways > > Instead of using virtuals for polymorphism, store each polymorphic operation > in its own array of function pointers. This removes an indirection when calling > the function, and lets us use nullptr as a signal to skip the call entirely. > > Unfortunately (as the old code is rather simpler) this makes an unambiguous speed improvement. > > Before: > curr/maxrss loops min median mean max stddev samples config bench > 21/26 MB 2 44.6µs 46.4µs 48.3µs 274µs 13% 51724 nonrendering desk_nytimes.skp > 23/26 MB 11 11.4µs 11.9µs 12.4µs 75.1µs 15% 36716 nonrendering keymobi_nytimes_com_.skp > > After: > 19/26 MB 4 40.4µs 41.6µs 42.5µs 197µs 10% 29379 nonrendering desk_nytimes.skp > 20/26 MB 14 10.5µs 10.7µs 11.2µs 59.5µs 13% 31971 nonrendering keymobi_nytimes_com_.skp > > Broader comparison: > keymobi_linkedin.skp 1.76us -> 1.77us 1x > keymobi_bing_com_search_q_sloth.skp 5.09us -> 5.05us 0.99x > keymobi_digg_com.skp 17.4us -> 17.3us 0.99x > keymobi_theverge_com.skp 3.37us -> 3.34us 0.99x > top25desk_mail_google_com_mail_.skp 30.8us -> 30.4us 0.99x > tabl_gmail.skp 3.44us -> 3.38us 0.98x > top25desk_wikipedia__1_tab_.skp 100us -> 97.7us 0.98x > keymobi_cnn_com_2012_10_03_poli.skp 52.9us -> 51.7us 0.98x > desk_chalkboard.skp 107us -> 104us 0.97x > desk_css3gradients.skp 17.8us -> 17.3us 0.97x > keymobi_androidpolice_com_2012_.skp 42.3us -> 41.1us 0.97x > desk_googlehome.skp 1.94us -> 1.88us 0.97x > keymobi_mlb_com_.skp 5.38us -> 5.18us 0.96x > top25desk_pinterest.skp 92.1us -> 88.5us 0.96x > keymobi_iphone_capitolvolkswage.skp 15.1us -> 14.5us 0.96x > keymobi_techcrunch_com.skp 9.45us -> 9.05us 0.96x > desk_espn.skp 31.3us -> 30us 0.96x > keymobi_slashdot_org_.skp 8.72us -> 8.34us 0.96x > desk_tiger8svg.skp 30.6us -> 29.2us 0.96x > keymobi_blogger.skp 4.09us -> 3.91us 0.95x > keymobi_baidu_com_s_wd_barack_o.skp 11.9us -> 11.3us 0.95x > keymobi_cuteoverload_com.skp 54.2us -> 51.6us 0.95x > keymobi_deviantart_com_.skp 17.2us -> 16.4us 0.95x > desk_mapsvg.skp 163ns -> 155ns 0.95x > keymobi_pinterest.skp 6.97us -> 6.62us 0.95x > top25desk_games_yahoo_com.skp 94.1us -> 89.3us 0.95x > top25desk_google_com_calendar_.skp 18us -> 17us 0.95x > keymobi_google_co_uk_search_hl_.skp 11.4us -> 10.8us 0.95x > tabl_pravda.skp 38.5us -> 36.4us 0.94x > desk_gmailthread.skp 19us -> 17.9us 0.94x > keymobi_reddit_com_r_programmin.skp 76.1us -> 71.7us 0.94x > top25desk_linkedin.skp 20us -> 18.8us 0.94x > tabl_gamedeksiam.skp 118us -> 112us 0.94x > keymobi_boingboing_net.skp 20.4us -> 19.1us 0.93x > top25desk_google_com__hl_en_q_b.skp 17.6us -> 16.4us 0.93x > keymobi_amazon_com_gp_aw_s_ref_.skp 12.5us -> 11.6us 0.93x > keymobi_sfgate_com_.skp 16.6us -> 15.5us 0.93x > keymobi_booking_com_searchresul.skp 16.2us -> 15.1us 0.93x > tabl_digg.skp 28.8us -> 26.8us 0.93x > tabl_mozilla.skp 80.4us -> 74.6us 0.93x > desk_wowwiki.skp 39.2us -> 36.4us 0.93x > top25desk_youtube_com.skp 42us -> 38.9us 0.93x > top25desk_facebook.skp 23.7us -> 22us 0.93x > top25desk_blogger.skp 38.2us -> 35.4us 0.93x > keymobi_online_wsj_com_home_pag.skp 12.8us -> 11.9us 0.93x > top25desk_wordpress.skp 28.9us -> 26.8us 0.93x > top25desk_answers_yahoo_com.skp 37.2us -> 34.4us 0.93x > keymobi_plus_google_com_app_bas.skp 9.56us -> 8.85us 0.93x > keymobi_wordpress.skp 16.1us -> 14.9us 0.92x > keymobi_mobile_news_sandbox_goo.skp 27.1us -> 24.9us 0.92x > top25desk_techcrunch_com.skp 31.1us -> 28.6us 0.92x > keymobi_worldjournal_com_.skp 50.7us -> 46.5us 0.92x > keymobi_theverge_com_2012_10_28.skp 26.2us -> 24us 0.92x > desk_jsfiddlebigcar.skp 1.73us -> 1.59us 0.92x > top25desk_weather_com.skp 31.3us -> 28.7us 0.92x > top25desk_google_com_search_q_c.skp 48.2us -> 44.1us 0.92x > top25desk_twitter.skp 27.8us -> 25.5us 0.92x > tabl_worldjournal.skp 29.3us -> 26.8us 0.91x > desk_nytimes.skp 46us -> 42us 0.91x > top25desk_docs___1_open_documen.skp 6.04us -> 5.51us 0.91x > keymobi_wikipedia__1_tab_.skp 59.7us -> 54.4us 0.91x > desk_unicodetable.skp 1.12ms -> 1.02ms 0.91x > top25desk_ebay_com.skp 17.8us -> 16.2us 0.91x > keymobi_ftw_usatoday_com_2014_0.skp 26.8us -> 24.3us 0.91x > top25desk_sports_yahoo_com_.skp 49.9us -> 45.3us 0.91x > keymobi_cnn_com.skp 9.94us -> 9.03us 0.91x > keymobi_m_youtube_com_watch_v_9.skp 13.4us -> 12.2us 0.91x > top25desk_amazon_com.skp 26.6us -> 24.1us 0.91x > keymobi_news_yahoo_com.skp 17.5us -> 15.8us 0.9x > keymobi_wowwiki_com_world_of_wa.skp 11.2us -> 10.2us 0.9x > top25desk_plus_google_com_11003.skp 93.5us -> 84.4us 0.9x > desk_carsvg.skp 53.5us -> 48.2us 0.9x > top25desk_news_yahoo_com.skp 44.7us -> 40.3us 0.9x > keymobi_wikipedia__1_tab____del.skp 59.4us -> 53.4us 0.9x > desk_googlespreadsheet.skp 66us -> 59.2us 0.9x > keymobi_answers_yahoo_com_quest.skp 30.2us -> 27us 0.89x > desk_ugamsolutions.skp 13us -> 11.6us 0.89x > keymobi_shop_mobileweb_ebay_com.skp 6.96us -> 6.21us 0.89x > keymobi_nytimes_com_.skp 12.1us -> 10.8us 0.89x > keymobi_gsp_ro.skp 5.54us -> 4.92us 0.89x > top25desk_booking_com.skp 54.9us -> 48.6us 0.89x > top25desk_espn.skp 37us -> 32.6us 0.88x > keymobi_facebook_com_barackobam.skp 23.3us -> 20.4us 0.88x > > BUG=skia: > GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2231103002 > > Committed: https://skia.googlesource.com/skia/+/ac243914af957a806d842318a43dddaf5f941dc3 TBR=mtklein@google.com,mtklein@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=skia: Review-Url: https://codereview.chromium.org/2232183002
* Deserialize pictures with custom image-deserializerGravatar reed2016-08-11
| | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187613002 Review-Url: https://codereview.chromium.org/2187613002
* SkLiteDL: turn vtable sidewaysGravatar mtklein2016-08-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of using virtuals for polymorphism, store each polymorphic operation in its own array of function pointers. This removes an indirection when calling the function, and lets us use nullptr as a signal to skip the call entirely. Unfortunately (as the old code is rather simpler) this makes an unambiguous speed improvement. Before: curr/maxrss loops min median mean max stddev samples config bench 21/26 MB 2 44.6µs 46.4µs 48.3µs 274µs 13% 51724 nonrendering desk_nytimes.skp 23/26 MB 11 11.4µs 11.9µs 12.4µs 75.1µs 15% 36716 nonrendering keymobi_nytimes_com_.skp After: 19/26 MB 4 40.4µs 41.6µs 42.5µs 197µs 10% 29379 nonrendering desk_nytimes.skp 20/26 MB 14 10.5µs 10.7µs 11.2µs 59.5µs 13% 31971 nonrendering keymobi_nytimes_com_.skp Broader comparison: keymobi_linkedin.skp 1.76us -> 1.77us 1x keymobi_bing_com_search_q_sloth.skp 5.09us -> 5.05us 0.99x keymobi_digg_com.skp 17.4us -> 17.3us 0.99x keymobi_theverge_com.skp 3.37us -> 3.34us 0.99x top25desk_mail_google_com_mail_.skp 30.8us -> 30.4us 0.99x tabl_gmail.skp 3.44us -> 3.38us 0.98x top25desk_wikipedia__1_tab_.skp 100us -> 97.7us 0.98x keymobi_cnn_com_2012_10_03_poli.skp 52.9us -> 51.7us 0.98x desk_chalkboard.skp 107us -> 104us 0.97x desk_css3gradients.skp 17.8us -> 17.3us 0.97x keymobi_androidpolice_com_2012_.skp 42.3us -> 41.1us 0.97x desk_googlehome.skp 1.94us -> 1.88us 0.97x keymobi_mlb_com_.skp 5.38us -> 5.18us 0.96x top25desk_pinterest.skp 92.1us -> 88.5us 0.96x keymobi_iphone_capitolvolkswage.skp 15.1us -> 14.5us 0.96x keymobi_techcrunch_com.skp 9.45us -> 9.05us 0.96x desk_espn.skp 31.3us -> 30us 0.96x keymobi_slashdot_org_.skp 8.72us -> 8.34us 0.96x desk_tiger8svg.skp 30.6us -> 29.2us 0.96x keymobi_blogger.skp 4.09us -> 3.91us 0.95x keymobi_baidu_com_s_wd_barack_o.skp 11.9us -> 11.3us 0.95x keymobi_cuteoverload_com.skp 54.2us -> 51.6us 0.95x keymobi_deviantart_com_.skp 17.2us -> 16.4us 0.95x desk_mapsvg.skp 163ns -> 155ns 0.95x keymobi_pinterest.skp 6.97us -> 6.62us 0.95x top25desk_games_yahoo_com.skp 94.1us -> 89.3us 0.95x top25desk_google_com_calendar_.skp 18us -> 17us 0.95x keymobi_google_co_uk_search_hl_.skp 11.4us -> 10.8us 0.95x tabl_pravda.skp 38.5us -> 36.4us 0.94x desk_gmailthread.skp 19us -> 17.9us 0.94x keymobi_reddit_com_r_programmin.skp 76.1us -> 71.7us 0.94x top25desk_linkedin.skp 20us -> 18.8us 0.94x tabl_gamedeksiam.skp 118us -> 112us 0.94x keymobi_boingboing_net.skp 20.4us -> 19.1us 0.93x top25desk_google_com__hl_en_q_b.skp 17.6us -> 16.4us 0.93x keymobi_amazon_com_gp_aw_s_ref_.skp 12.5us -> 11.6us 0.93x keymobi_sfgate_com_.skp 16.6us -> 15.5us 0.93x keymobi_booking_com_searchresul.skp 16.2us -> 15.1us 0.93x tabl_digg.skp 28.8us -> 26.8us 0.93x tabl_mozilla.skp 80.4us -> 74.6us 0.93x desk_wowwiki.skp 39.2us -> 36.4us 0.93x top25desk_youtube_com.skp 42us -> 38.9us 0.93x top25desk_facebook.skp 23.7us -> 22us 0.93x top25desk_blogger.skp 38.2us -> 35.4us 0.93x keymobi_online_wsj_com_home_pag.skp 12.8us -> 11.9us 0.93x top25desk_wordpress.skp 28.9us -> 26.8us 0.93x top25desk_answers_yahoo_com.skp 37.2us -> 34.4us 0.93x keymobi_plus_google_com_app_bas.skp 9.56us -> 8.85us 0.93x keymobi_wordpress.skp 16.1us -> 14.9us 0.92x keymobi_mobile_news_sandbox_goo.skp 27.1us -> 24.9us 0.92x top25desk_techcrunch_com.skp 31.1us -> 28.6us 0.92x keymobi_worldjournal_com_.skp 50.7us -> 46.5us 0.92x keymobi_theverge_com_2012_10_28.skp 26.2us -> 24us 0.92x desk_jsfiddlebigcar.skp 1.73us -> 1.59us 0.92x top25desk_weather_com.skp 31.3us -> 28.7us 0.92x top25desk_google_com_search_q_c.skp 48.2us -> 44.1us 0.92x top25desk_twitter.skp 27.8us -> 25.5us 0.92x tabl_worldjournal.skp 29.3us -> 26.8us 0.91x desk_nytimes.skp 46us -> 42us 0.91x top25desk_docs___1_open_documen.skp 6.04us -> 5.51us 0.91x keymobi_wikipedia__1_tab_.skp 59.7us -> 54.4us 0.91x desk_unicodetable.skp 1.12ms -> 1.02ms 0.91x top25desk_ebay_com.skp 17.8us -> 16.2us 0.91x keymobi_ftw_usatoday_com_2014_0.skp 26.8us -> 24.3us 0.91x top25desk_sports_yahoo_com_.skp 49.9us -> 45.3us 0.91x keymobi_cnn_com.skp 9.94us -> 9.03us 0.91x keymobi_m_youtube_com_watch_v_9.skp 13.4us -> 12.2us 0.91x top25desk_amazon_com.skp 26.6us -> 24.1us 0.91x keymobi_news_yahoo_com.skp 17.5us -> 15.8us 0.9x keymobi_wowwiki_com_world_of_wa.skp 11.2us -> 10.2us 0.9x top25desk_plus_google_com_11003.skp 93.5us -> 84.4us 0.9x desk_carsvg.skp 53.5us -> 48.2us 0.9x top25desk_news_yahoo_com.skp 44.7us -> 40.3us 0.9x keymobi_wikipedia__1_tab____del.skp 59.4us -> 53.4us 0.9x desk_googlespreadsheet.skp 66us -> 59.2us 0.9x keymobi_answers_yahoo_com_quest.skp 30.2us -> 27us 0.89x desk_ugamsolutions.skp 13us -> 11.6us 0.89x keymobi_shop_mobileweb_ebay_com.skp 6.96us -> 6.21us 0.89x keymobi_nytimes_com_.skp 12.1us -> 10.8us 0.89x keymobi_gsp_ro.skp 5.54us -> 4.92us 0.89x top25desk_booking_com.skp 54.9us -> 48.6us 0.89x top25desk_espn.skp 37us -> 32.6us 0.88x keymobi_facebook_com_barackobam.skp 23.3us -> 20.4us 0.88x BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2231103002 Review-Url: https://codereview.chromium.org/2231103002
* Implement GPU occluded blur mask filterGravatar robertphillips2016-08-10
| | | | | | | | Spawned off: https://codereview.chromium.org/2214163003/ (Minor clean up related to blur mask filters) GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2201133002 Review-Url: https://codereview.chromium.org/2201133002
* update textblob api to use sk_spGravatar reed2016-08-10
| | | | | | | | | BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2236013002 NOTREECHECKS=True Review-Url: https://codereview.chromium.org/2236013002