New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 760738 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocked on:
issue 765569



Sign in to add a comment

DevTools: [regression] inspect mode is lagging

Project Member Reported by pfeldman@chromium.org, Aug 30 2017

Issue description

- Open devtools
- Select "inspect" tool in the top left corner of devtools
- Hover over the page elements

everything is very slow. Bisect range is:

https://chromium.googlesource.com/chromium/src/+log/a773c51e18c77d4bd644b3267676e705d46c6025..041bc8660d3eeb6988505c9593e7b56d2a7a8ed9
 
Labels: ReleaseBlock-Stable M-61
Owner: junov@chromium.org
Status: Assigned (was: Unconfirmed)
Summary: DevTools: [regression] inspect mode is lagging (was: DevTools: inspect mode is lagging )
Passing --force-display-list-2d-canvas does not recover it, because GetCanvasColorParams().UsesOutputSpaceBlending() is now false. Which is probably due to https://chromium-review.googlesource.com/c/chromium/src/+/598500.

The inspection experience is quite bad, so I'm marking this as RBS of M61.

Comment 3 by gov...@chromium.org, Aug 30 2017

URGENT - PTAL.
Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. Thank you!

Please note we're planning to cut M61 Stable RC this week on Thursday/Friday. 
Here's a trace.


trace_inspect-element-lag-gpu-playback.json.gz
2.5 MB Download
Screen Shot 2017-08-30 at 4.14.04 PM.png
154 KB View Download

Comment 5 by vmi...@chromium.org, Aug 30 2017

Cc: chrishtr@chromium.org fs...@chromium.org vmi...@chromium.org
Components: Blink>Canvas
Labels: -Pri-3 OS-Chrome OS-Mac OS-Windows Pri-1

Comment 6 by ketakid@google.com, Aug 30 2017

junov@ can you please take a look at this and put in merge right away?

Comment 8 by vmi...@chromium.org, Aug 30 2017

junov@ does it seem safe to enable display-list mode?  I'm concerned we have ~6 weeks of rot on this path, and need to bake this more before re-enabling in Stable.

The trace is surprising.  If we aren't in display-list mode why are we busy in Raster Workers?

Comment 9 by vmi...@chromium.org, Aug 31 2017

Cc: vmp...@chromium.org
The Inspector Overlay has accelerated compositing disabled and is painted in a unique way.

See: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/inspector/InspectorOverlayAgent.cpp?l=502

The canvas up rendered from a bitmap via a DrawImageRect, and somehow missing Skia caching such that the cost is multiplied in every tile.
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0b71497e605218c9945829c54a74e30ad45ddde2

commit 0b71497e605218c9945829c54a74e30ad45ddde2
Author: Pavel Feldman <pfeldman@chromium.org>
Date: Thu Aug 31 03:19:03 2017

Canvas: introduce forceDisplayList2dCanvasEnabled setting and set it to true for inspector overlay.

Bug:  760738 
Change-Id: I24313ae93f92b2bed8ddbdf5589af2dd23629d23
Reviewed-on: https://chromium-review.googlesource.com/644616
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Pavel Feldman <pfeldman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498744}
[modify] https://crrev.com/0b71497e605218c9945829c54a74e30ad45ddde2/third_party/WebKit/Source/core/frame/Settings.json5
[modify] https://crrev.com/0b71497e605218c9945829c54a74e30ad45ddde2/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/0b71497e605218c9945829c54a74e30ad45ddde2/third_party/WebKit/Source/core/inspector/InspectorOverlayAgent.cpp

Display list was disabled in

r485371, Jul 10

branch point for M61 was

r488528, Jul 30

so we are talking about three weeks of changes w/o Canary coverage. I enabled it for the overlay today (Linux, disabled the color space check) and everything seems to work as expected. I can't assess the potential impact here though, so deferring to junov@ for the informed resolution.



forceDisplayList2dCanvasEnabled setting is landed in case we consider it safe and/or don't find a better solution by the M61 stable. We can merge it into M61 branch and it will enable display list for the overlay only.

Comment 12 by junov@chromium.org, Aug 31 2017

Display list mode was disabled for good reasons: It produces corrupted output on some MacOS and Windows systems. Sseems to be driver or GPU dependent, no one at Google was able to repro, but problem was relatively wide-spread.

The RasterWorkers threads being so busy is surprising IMHO.  I will focus on investigating that.

Comment 13 by junov@chromium.org, Aug 31 2017

I've removed the "overlay_settings.SetForceDisplayList2dCanvasEnabled(true);" in my local build, and the bug does not repro.  The inspector is super responsive. The GPU compositing takes 2.4ms per frame.  Has something else changed?

I am on Linux.

Comment 14 by junov@chromium.org, Aug 31 2017

Can't repro on Windows either (with fix from #10 reverted)
junov@ are you able to repro in M61 Beta?  I've seen it run slowly on Linux & Mac.
Pls apply Android label if applicable as Android is planning to cut M61 Stable RC this week.

Comment 17 by junov@chromium.org, Aug 31 2017

No repro with beta or dev channels.  To whoever can repro this bug, could you please attach a copy of your Chrome://gpu page to this bug?

Re # 11, correction on M61 branch date (M61 was branched on July 20th),
r488528, Jul 20.

Comment 19 by junov@chromium.org, Aug 31 2017

Steps to repro on a Linux HP z840:

1) Make sure GPU rasterization is enabled (check chrome://gpu, change it on chrome://flags)
2) Maximize window
3) navigate to a page with lots of elements (webkit.org works)
4) Open dev tool
5) Ctrl+Shif+M to toggle the device toolbar
6) In the device toolbar, Select "Responsive"
7) Stretch the device region to cover the entire window
8) In the three dots menu of the device toolbar, select "add device pixel ratio"
9) Set the device pixel ratio to 2 (or anything other than 1)
10) Click on the "inspect" tool.
11) Wiggle your mouse pointer around the page

Result: you're getting about 15fps instead of 60 and the trace graph looks similar to the one in comment #4

Comment 20 by junov@chromium.org, Aug 31 2017

Performance seems to be gated by texture uploads from the software-rendered canvas.  The obvious solution here would be to make the canvases GPU accelerated.  Unfortunately, that does not work because of how overlay rendering currently works.  The overlay page has to paint its contents into a single GraphicsLayer via LoaclFrameView::Paint, which cannot capture child accelerated compositing layers.

IMHO, to get this to work properly we'd have to graft the compositing tree of the overlay page onto the main compositing tree.
@junov: the question is how can we unblock M61? Di you think merging r498744 into it mitigates it in M61?
Cc: jmukthavaram@chromium.org
Labels: Needs-Feedback
junov@,

Tested this issue on Ubuntu 14.04 & Windows 7 using chrome#62.0.3202.0 & #61.0.3115.0 as per the steps mentioned in c#19.

Observed fps value is changing from 10 to 60 (not constant) while moving mouse pointer around the page on both the builds. (M61 & M62)

Please find the attached screencast of both M61 & M62 & confirm on the fix & the expected behavior.

Thanks in advance..!
760738-M61.mp4
12.0 MB View Download
760738-M62.mp4
14.7 MB View Download
Cc: abdulsyed@chromium.org
Labels: Merge-Request-61
@#22:The slow part is displaying the overlay, so if you just move the pointer really fast (to constantly change what is highlighted in blue), you will get a consistently slow frame rate.

For M61, The fix from #10 makes sense, so let's merge that ASAP.

But let's keep investigating what to do for future releases because the display-list-2d-canvas code path is eventually going away. And also, there are other sources of slowness involved.

Project Member

Comment 25 by sheriffbot@chromium.org, Sep 1 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: We are only 3 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Before we approve merge to M61 for Cl listed at #10 (r498744), could you pls confirm change is well baked/verified in Canary, covered by automation tests and safe to merge to M61?
As per comments #11 and #24:

- We should merge https://chromium-review.googlesource.com/644616 into M61.
- We consider it safe:
  - It has been running on the trunk for 2 days with no issues.
  - It is equivalent to the revert of https://chromium-review.googlesource.com/c/chromium/src/+/563519, but localized to inspector code paths


Labels: -Merge-Review-61 Merge-Approved-61
Approving merge to M61 branch 3163 based on comment #27. Please merge ASAP. Thank you.
Project Member

Comment 29 by bugdroid1@chromium.org, Sep 1 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/05d09614034869644be40e10136d7077dd30ff46

commit 05d09614034869644be40e10136d7077dd30ff46
Author: Pavel Feldman <pfeldman@chromium.org>
Date: Fri Sep 01 16:59:16 2017

Canvas: introduce forceDisplayList2dCanvasEnabled setting and set it to true for inspector overlay.

TBR=pfeldman@chromium.org

(cherry picked from commit 0b71497e605218c9945829c54a74e30ad45ddde2)

Bug:  760738 
Change-Id: I24313ae93f92b2bed8ddbdf5589af2dd23629d23
Reviewed-on: https://chromium-review.googlesource.com/644616
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Pavel Feldman <pfeldman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#498744}
Reviewed-on: https://chromium-review.googlesource.com/647342
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#1065}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/05d09614034869644be40e10136d7077dd30ff46/third_party/WebKit/Source/core/frame/Settings.json5
[modify] https://crrev.com/05d09614034869644be40e10136d7077dd30ff46/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/05d09614034869644be40e10136d7077dd30ff46/third_party/WebKit/Source/core/inspector/InspectorOverlayAgent.cpp

Labels: -M-61 M-62
Thanks! Keeping this issue open for follow-up work.  Now that M-61 is fixed, I'm changing the milestone to M-62 which still has the bug.
Tested the issue using #61.0.3163.79 on Mac 10.12.6, Win 7 as per the steps mentioned in comment #19. Can observe 60fps, but the value is keep changing on scrolling the page up/down and Wiggling mouse pointer around the page.

@junov: Could you please find the attached video and confirm the behavior.

Thanks!!
Sep 5 2017 4-37 PM.webm
16.0 MB Download
I figured out what to do for M-62.  Perf is particularly accute on Mac because we've enabled color-correct rendering on Mac.  As vmiura@ pointed out the full canvas image is being re-uploaded to the GPU for each compositor tile AND we're re-applying color correction each time.  The problem is in skia's SkColorSpaceXformCanvas, which applies color space transforms to images before drawing them to a wrapped SkCanvas.  There are several problems with the implementation:
1) The entire source image gets color corrected even when only part of it is drawn.
2) Applying the color space transform generates a new image with a new generation ID, so we never get cache hits one images that are drawn repeatedly.
3) If the source image is in main RAM (as opposed to GPU memory), the color correction is performed on the CPU, which is a lot slower than on the GPU. We do this even when we know the image is to be drawn to a GPU-accelerated canvas

The solution I am experimenting with consists in making SkColorSpaceXformCanvas upload source images to the GPU before the color correction step when the destination canvas is known to be on the GPU.  The effect of this change is that the color correction is now being performed on the GPU, and re-uploads are avoided because the image being uploaded to the GPU is the original (non-cc'ed) images, so the Ganesh cache is now getting hit.

@#31: Behavior looks pretty snappy in that video.  The page is not animated so the FPS meter is not a great way to measure the problem. The way I have been measuring the problem is by looking at trace graphs to see how much time is needed to rasterize the overlay.
Exact steps on clean profile:

1) navigate to webkit.org, make window large, open devtools via inspect element context menu
2) enter inspect mode via clicking top left corner toolbar button
3) start hovering headers, images and text in a page. drive circles over the page
4) see how blue highlight sticks to your cursor, refreshes immediately

it did not do it before the fix, there was an observable latency
Labels: -M-62 M-61
Still slow on mac in Chrome 61.  Fix mentioned in #32 is needed for M61
Fix is awaiting code review: https://skia-review.googlesource.com/c/skia/+/42422
Project Member

Comment 37 by bugdroid1@chromium.org, Sep 5 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/19c8726a0829e506c5c42d22d67b3fe7505ed24e

commit 19c8726a0829e506c5c42d22d67b3fe7505ed24e
Author: Justin Novosad <junov@chromium.org>
Date: Tue Sep 05 17:50:25 2017

Optimize SkColorSpaceXformCanvas for GPU-acceleration

This change ensures that SkImages are uploaded to the GPU before
applying the xform when the destination canvas is on the GPU. This
makes it possible to get hits in the texture cache and it ensure
that transforms get computed on the GPU.

This fixes a severe performance regression in Chrome that happened
when color correction was enabled.

BUG= chromium:760738 

Change-Id: I52032a9f06ccbe1a7fd56a91db15377925143b26
Reviewed-on: https://skia-review.googlesource.com/42422
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Justin Novosad <junov@chromium.org>

[modify] https://crrev.com/19c8726a0829e506c5c42d22d67b3fe7505ed24e/src/core/SkColorSpaceXformCanvas.cpp

Labels: -Hotlist-Merge-Review -Needs-Feedback -merge-merged-3163 Merge-Request-62
Requesting permission to merge fix from #37 to the M62 branch
Project Member

Comment 39 by bugdroid1@chromium.org, Sep 5 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/afb0bd4f447c3c58007ced563f31b3e856346d3b

commit afb0bd4f447c3c58007ced563f31b3e856346d3b
Author: Robert Phillips <robertphillips@google.com>
Date: Tue Sep 05 21:06:53 2017

Revert "Optimize SkColorSpaceXformCanvas for GPU-acceleration"

This reverts commit 19c8726a0829e506c5c42d22d67b3fe7505ed24e.

Reason for revert: Breaking Chrome roll

Original change's description:
> Optimize SkColorSpaceXformCanvas for GPU-acceleration
> 
> This change ensures that SkImages are uploaded to the GPU before
> applying the xform when the destination canvas is on the GPU. This
> makes it possible to get hits in the texture cache and it ensure
> that transforms get computed on the GPU.
> 
> This fixes a severe performance regression in Chrome that happened
> when color correction was enabled.
> 
> BUG= chromium:760738 
> 
> Change-Id: I52032a9f06ccbe1a7fd56a91db15377925143b26
> Reviewed-on: https://skia-review.googlesource.com/42422
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Justin Novosad <junov@chromium.org>

TBR=brianosman@google.com,junov@chromium.org

Change-Id: Ia117d6f95c34324383aaf3bfc990bb13e48bed6e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:760738 
Reviewed-on: https://skia-review.googlesource.com/42720
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

[modify] https://crrev.com/afb0bd4f447c3c58007ced563f31b3e856346d3b/src/core/SkColorSpaceXformCanvas.cpp

junov@ can we close this issue if this is fixed on M61?
Project Member

Comment 41 by bugdroid1@chromium.org, Sep 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4debc9c11ba69a927af7d9438e7b5b141f3ea6a5

commit 4debc9c11ba69a927af7d9438e7b5b141f3ea6a5
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Tue Sep 05 23:43:08 2017

Roll src/third_party/skia/ 4748d8188..afb0bd4f4 (11 commits)

https://skia.googlesource.com/skia.git/+log/4748d81886e6..afb0bd4f447c

$ git log 4748d8188..afb0bd4f4 --date=short --no-merges --format='%ad %ae %s'
2017-09-05 robertphillips Revert "Optimize SkColorSpaceXformCanvas for GPU-acceleration"
2017-09-01 jvanverth More iOS Fixes for SkiaSDLExample.
2017-09-05 benjaminwagner Omit flaky encode-platform test for pic-8888 Mac CPU.
2017-09-05 brianosman Unbind xfer buffer in a couple more places
2017-09-05 mtklein merge 0,1,2,3,... and 0.5f
2017-09-05 brianosman Unbind xfer buffer before trying to call TexImage2D
2017-09-05 scroggo Stop using SkRasterPipeline inside GIF
2017-09-05 angle-deps-roller Roll skia/third_party/externals/angle2/ 8a7b3a0ce..7d738e266 (1 commit)
2017-09-05 lsalzman fix analytic AA to work with even rounding
2017-09-05 junov Optimize SkColorSpaceXformCanvas for GPU-acceleration
2017-09-05 liyuqian Upload the missing change in 42101

Created with:
  roll-dep src/third_party/skia
BUG= 760738 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=robertphillips@chromium.org

Change-Id: I4395d4967791bdcddc269ff9cad0c9268891b1de
Reviewed-on: https://chromium-review.googlesource.com/651566
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499801}
[modify] https://crrev.com/4debc9c11ba69a927af7d9438e7b5b141f3ea6a5/DEPS

Project Member

Comment 42 by sheriffbot@chromium.org, Sep 6 2017

Labels: -Merge-Request-62 Merge-Review-62 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-62 Merge-Approved-62
Approving merge for M62. 
Project Member

Comment 44 by bugdroid1@chromium.org, Sep 7 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/1d3df3848ff297e353a2d0f0fae6ec4a55ab7ba6

commit 1d3df3848ff297e353a2d0f0fae6ec4a55ab7ba6
Author: Justin Novosad <junov@chromium.org>
Date: Thu Sep 07 17:47:51 2017

Optimize SkColorSpaceXformCanvas for GPU-acceleration

This change ensures that SkImages are uploaded to the GPU before
applying the xform when the destination canvas is on the GPU. This
makes it possible to get hits in the texture cache and it ensures
that transforms get computed on the GPU.

This fixes a severe performance regression in Chrome that happened
when color correction was enabled.

Associated chromium patch for layout test rebaselines:
https://chromium-review.googlesource.com/c/chromium/src/+/655483

Merge dependency: Merging this change to the M-62 and M-61
branches also requires merging the following change, otherwise
there will be rendering errors:
https://skia-review.googlesource.com/c/skia/+/43562

BUG= chromium:760738 

Change-Id: I49fd5ef7968272d311249c3824fe15bee4648b73
Reviewed-on: https://skia-review.googlesource.com/43183
Commit-Queue: Justin Novosad <junov@chromium.org>
Reviewed-by: Brian Osman <brianosman@google.com>

[modify] https://crrev.com/1d3df3848ff297e353a2d0f0fae6ec4a55ab7ba6/src/core/SkColorSpaceXformCanvas.cpp

Project Member

Comment 45 by bugdroid1@chromium.org, Sep 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/61d8f1c8c17afa0dfe0a1748ac4b73a46a887fe0

commit 61d8f1c8c17afa0dfe0a1748ac4b73a46a887fe0
Author: Justin Novosad <junov@chromium.org>
Date: Thu Sep 07 17:58:56 2017

Manual rebaselines for upcoming skia DEPS roll

Manual rebaselines need for CL:
https://skia-review.googlesource.com/c/skia/+/43183/

BUG= 760738 
TBR=xidachen@chromium.org
NOTRY=true

Change-Id: Ia326bb518fb711b1db1781021d64f52c3b9df78d
Reviewed-on: https://chromium-review.googlesource.com/655483
Commit-Queue: Justin Novosad <junov@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500333}
[modify] https://crrev.com/61d8f1c8c17afa0dfe0a1748ac4b73a46a887fe0/third_party/WebKit/LayoutTests/TestExpectations

Project Member

Comment 46 by bugdroid1@chromium.org, Sep 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c14495093d88203534ab85a4ce22febf215b5084

commit c14495093d88203534ab85a4ce22febf215b5084
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Thu Sep 07 20:14:55 2017

Roll src/third_party/skia/ 6d9e4ee14..e5ede4b13 (23 commits)

https://skia.googlesource.com/skia.git/+log/6d9e4ee14e7e..e5ede4b138e8

$ git log 6d9e4ee14..e5ede4b13 --date=short --no-merges --format='%ad %ae %s'
2017-09-07 csmartdalton Revert "Improve GrPathRendererChain heuristics"
2017-09-07 bsalomon Update mtl backend to use GrSamplerState
2017-09-07 junov Optimize SkColorSpaceXformCanvas for GPU-acceleration
2017-09-04 csmartdalton Improve GrPathRendererChain heuristics
2017-09-07 bsalomon Rework GrSamplerParams to be more compact and use its own wrap mode enum.
2017-09-07 bsalomon Revert "Remove "content" rect from GrTextureAdjuster."
2017-09-07 brianosman Blacklist more flaky GMs in gltestthreading config
2017-09-07 bsalomon Remove "content" rect from GrTextureAdjuster.
2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ 61e710b65..7cadfcc70 (1 commit)
2017-09-07 brianosman Make SkImage_Lazy always report the color space of its data
2017-09-06 rmistry Add links to autoroller cloud logs for roboCops
2017-09-07 ethannicholas Fix for Google3 roll failure
2017-09-07 rmistry Revert "Whitespace change to test Android autoroller"
2017-09-07 rmistry Whitespace change to test Android autoroller
2017-07-10 nagarajan.n Remove loop unrolling code in load_gamut
2017-07-19 nagarajan.n Remove loop unrolling code in onQueryYUV8
2017-07-19 nagarajan.n Update libjpeg buffer status when it has has to be refilled.
2017-09-06 liyuqian Fix SkASSERT for convex paths with DAA
2017-09-07 ethannicholas Revert "Revert "Initial checkin of SkSL lexical analyzer generator (not actually in use yet).""
2017-09-06 brianosman Output encoded PNGs when gltestthreading or serialize fails
2017-08-30 bsalomon Add rect clip optimization to GrRenderTargetContext::drawTextureAffine
2017-09-07 caryclark remove useless reset
2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ c1d4e5509..61e710b65 (1 commit)

Created with:
  roll-dep src/third_party/skia
BUG= 760738 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=robertphillips@chromium.org

Change-Id: I2dbf077343bff9386a6f2a948261c7e190f03733
Reviewed-on: https://chromium-review.googlesource.com/655762
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500366}
[modify] https://crrev.com/c14495093d88203534ab85a4ce22febf215b5084/DEPS

Project Member

Comment 47 by bugdroid1@chromium.org, Sep 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b81dee7bb2af6c7940eec41db2db93e281bc2e59

commit b81dee7bb2af6c7940eec41db2db93e281bc2e59
Author: Matt Giuca <mgiuca@chromium.org>
Date: Fri Sep 08 03:51:15 2017

Revert "Roll src/third_party/skia/ 6d9e4ee14..e5ede4b13 (23 commits)"

This reverts commit a2ac4a2a3faa0d2307e1cc4d7d7e82675a796f0a and
c14495093d88203534ab85a4ce22febf215b5084 (two rolls have been reverted).

Reason for revert: Suspect broke Mac layout tests; see bug.

Bug: 763197

Original change's description:
> Roll src/third_party/skia/ 6d9e4ee14..e5ede4b13 (23 commits)
>
> https://skia.googlesource.com/skia.git/+log/6d9e4ee14e7e..e5ede4b138e8
>
> $ git log 6d9e4ee14..e5ede4b13 --date=short --no-merges --format='%ad %ae %s'
> 2017-09-07 csmartdalton Revert "Improve GrPathRendererChain heuristics"
> 2017-09-07 bsalomon Update mtl backend to use GrSamplerState
> 2017-09-07 junov Optimize SkColorSpaceXformCanvas for GPU-acceleration
> 2017-09-04 csmartdalton Improve GrPathRendererChain heuristics
> 2017-09-07 bsalomon Rework GrSamplerParams to be more compact and use its own wrap mode enum.
> 2017-09-07 bsalomon Revert "Remove "content" rect from GrTextureAdjuster."
> 2017-09-07 brianosman Blacklist more flaky GMs in gltestthreading config
> 2017-09-07 bsalomon Remove "content" rect from GrTextureAdjuster.
> 2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ 61e710b65..7cadfcc70 (1 commit)
> 2017-09-07 brianosman Make SkImage_Lazy always report the color space of its data
> 2017-09-06 rmistry Add links to autoroller cloud logs for roboCops
> 2017-09-07 ethannicholas Fix for Google3 roll failure
> 2017-09-07 rmistry Revert "Whitespace change to test Android autoroller"
> 2017-09-07 rmistry Whitespace change to test Android autoroller
> 2017-07-10 nagarajan.n Remove loop unrolling code in load_gamut
> 2017-07-19 nagarajan.n Remove loop unrolling code in onQueryYUV8
> 2017-07-19 nagarajan.n Update libjpeg buffer status when it has has to be refilled.
> 2017-09-06 liyuqian Fix SkASSERT for convex paths with DAA
> 2017-09-07 ethannicholas Revert "Revert "Initial checkin of SkSL lexical analyzer generator (not actually in use yet).""
> 2017-09-06 brianosman Output encoded PNGs when gltestthreading or serialize fails
> 2017-08-30 bsalomon Add rect clip optimization to GrRenderTargetContext::drawTextureAffine
> 2017-09-07 caryclark remove useless reset
> 2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ c1d4e5509..61e710b65 (1 commit)
>
> Created with:
>   roll-dep src/third_party/skia
> BUG= 760738 
>
>
> Documentation for the AutoRoller is here:
> https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
>
> If the roll is causing failures, see:
> http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls
>
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
> TBR=robertphillips@chromium.org
>
> Change-Id: I2dbf077343bff9386a6f2a948261c7e190f03733
> Reviewed-on: https://chromium-review.googlesource.com/655762
> Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
> Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#500366}

TBR=skia-deps-roller@chromium.org,robertphillips@google.com

Change-Id: I996f952cec94afb43d558e279eef2f8b2c8d02a1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  760738 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/656738
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500501}
[modify] https://crrev.com/b81dee7bb2af6c7940eec41db2db93e281bc2e59/DEPS

Project Member

Comment 48 by bugdroid1@chromium.org, Sep 8 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/7231c50f9c21cb13f428efe2a3bdbcf29543799a

commit 7231c50f9c21cb13f428efe2a3bdbcf29543799a
Author: Robert Phillips <robertphillips@google.com>
Date: Fri Sep 08 11:44:34 2017

Revert "Optimize SkColorSpaceXformCanvas for GPU-acceleration"

This reverts commit 1d3df3848ff297e353a2d0f0fae6ec4a55ab7ba6.

Reason for revert: Mac timeouts? crbug.com/763197

Original change's description:
> Optimize SkColorSpaceXformCanvas for GPU-acceleration
> 
> This change ensures that SkImages are uploaded to the GPU before
> applying the xform when the destination canvas is on the GPU. This
> makes it possible to get hits in the texture cache and it ensures
> that transforms get computed on the GPU.
> 
> This fixes a severe performance regression in Chrome that happened
> when color correction was enabled.
> 
> Associated chromium patch for layout test rebaselines:
> https://chromium-review.googlesource.com/c/chromium/src/+/655483
> 
> Merge dependency: Merging this change to the M-62 and M-61
> branches also requires merging the following change, otherwise
> there will be rendering errors:
> https://skia-review.googlesource.com/c/skia/+/43562
> 
> BUG= chromium:760738 
> 
> Change-Id: I49fd5ef7968272d311249c3824fe15bee4648b73
> Reviewed-on: https://skia-review.googlesource.com/43183
> Commit-Queue: Justin Novosad <junov@chromium.org>
> Reviewed-by: Brian Osman <brianosman@google.com>

TBR=brianosman@google.com,junov@chromium.org

Change-Id: I1a43e9b277075d1aee0efd651c899e15ac3fa53b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:760738 
Reviewed-on: https://skia-review.googlesource.com/44160
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

[modify] https://crrev.com/7231c50f9c21cb13f428efe2a3bdbcf29543799a/src/core/SkColorSpaceXformCanvas.cpp

Project Member

Comment 49 by bugdroid1@chromium.org, Sep 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38466dba33f853a33047738d8b9fecee29802142

commit 38466dba33f853a33047738d8b9fecee29802142
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Fri Sep 08 14:11:16 2017

Roll src/third_party/skia/ 6d9e4ee14..7231c50f9 (35 commits)

https://skia.googlesource.com/skia.git/+log/6d9e4ee14e7e..7231c50f9c21

$ git log 6d9e4ee14..7231c50f9 --date=short --no-merges --format='%ad %ae %s'
2017-09-08 robertphillips Revert "Optimize SkColorSpaceXformCanvas for GPU-acceleration"
2017-09-08 angle-deps-roller Roll skia/third_party/externals/angle2/ 1de29abad..95644f92d (1 commit)
2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ fe8b5989d..1de29abad (1 commit)
2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ e98e16eab..fe8b5989d (1 commit)
2017-09-07 csmartdalton Only run CPU SVG tests on a single bot
2017-09-07 csmartdalton Improve GrPathRendererChain heuristics
2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ 3ed604246..e98e16eab (3 commits)
2017-09-07 robertphillips Revert "Make TextureOp use multitexturing to batch draws of different SkImages."
2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ 7cadfcc70..3ed604246 (3 commits)
2017-09-06 chz add heic extension to DM test
2017-09-07 egdaniel Update GenerateMipMapsAndUploadtoTextureProxy to respect color space when setting up SurfaceDesc
2017-09-07 bsalomon Make TextureOp use multitexturing to batch draws of different SkImages.
2017-09-07 csmartdalton Revert "Improve GrPathRendererChain heuristics"
2017-09-07 bsalomon Update mtl backend to use GrSamplerState
2017-09-07 junov Optimize SkColorSpaceXformCanvas for GPU-acceleration
2017-09-04 csmartdalton Improve GrPathRendererChain heuristics
2017-09-07 bsalomon Rework GrSamplerParams to be more compact and use its own wrap mode enum.
2017-09-07 bsalomon Revert "Remove "content" rect from GrTextureAdjuster."
2017-09-07 brianosman Blacklist more flaky GMs in gltestthreading config
2017-09-07 bsalomon Remove "content" rect from GrTextureAdjuster.
2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ 61e710b65..7cadfcc70 (1 commit)
2017-09-07 brianosman Make SkImage_Lazy always report the color space of its data
2017-09-06 rmistry Add links to autoroller cloud logs for roboCops
2017-09-07 ethannicholas Fix for Google3 roll failure
2017-09-07 rmistry Revert "Whitespace change to test Android autoroller"
2017-09-07 rmistry Whitespace change to test Android autoroller
2017-07-10 nagarajan.n Remove loop unrolling code in load_gamut
2017-07-19 nagarajan.n Remove loop unrolling code in onQueryYUV8
2017-07-19 nagarajan.n Update libjpeg buffer status when it has has to be refilled.
2017-09-06 liyuqian Fix SkASSERT for convex paths with DAA
2017-09-07 ethannicholas Revert "Revert "Initial checkin of SkSL lexical analyzer generator (not actually in use yet).""
2017-09-06 brianosman Output encoded PNGs when gltestthreading or serialize fails
2017-08-30 bsalomon Add rect clip optimization to GrRenderTargetContext::drawTextureAffine
2017-09-07 caryclark remove useless reset
2017-09-07 angle-deps-roller Roll skia/third_party/externals/angle2/ c1d4e5509..61e710b65 (1 commit)

Created with:
  roll-dep src/third_party/skia
BUG= 760738 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=robertphillips@chromium.org

Change-Id: Ie734409540b449dcfd7d610dbc23166529261a2c
Reviewed-on: https://chromium-review.googlesource.com/657478
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500583}
[modify] https://crrev.com/38466dba33f853a33047738d8b9fecee29802142/DEPS

Project Member

Comment 50 by bugdroid1@chromium.org, Sep 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0789a62ba6376f0ccb522ded0c8e006d2180232b

commit 0789a62ba6376f0ccb522ded0c8e006d2180232b
Author: Justin Novosad <junov@chromium.org>
Date: Fri Sep 08 19:25:53 2017

Marking some tests as slow before re-landing skia CL that slows them

This change marks the following tests as slow on mac:
virtual/gpu-rasterization/images/color-profile-border-radius.html
virtual/gpu-rasterization/images/color-profile-image-canvas-svg.html

This will allow https://skia-review.googlesource.com/c/skia/+/43183
to reland in skia without causing failures.

The reason the test are becoming slow is that the skia change
causes color space conversion work to be performed on the GPU
instead of the CPU.  The GPU path is slower when running layout
tests because for layout tests we use mesa instead of real GPUs.

TBR=xidachen@chromium.org
NOTRY=true
BUG=763197,  760738 

Change-Id: I4e0dcf30add90022d71bb6e3ac2d90b48380d625
Reviewed-on: https://chromium-review.googlesource.com/657883
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500639}
[modify] https://crrev.com/0789a62ba6376f0ccb522ded0c8e006d2180232b/third_party/WebKit/LayoutTests/SlowTests

Project Member

Comment 51 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/8b1360dcfab9ae748361268284068049cd3796a7

commit 8b1360dcfab9ae748361268284068049cd3796a7
Author: Justin Novosad <junov@chromium.org>
Date: Mon Sep 11 14:01:00 2017

Reland "Optimize SkColorSpaceXformCanvas for GPU-acceleration"

This is a reland of 1d3df3848ff297e353a2d0f0fae6ec4a55ab7ba6
Original change's description:
> Optimize SkColorSpaceXformCanvas for GPU-acceleration
> 
> This change ensures that SkImages are uploaded to the GPU before
> applying the xform when the destination canvas is on the GPU. This
> makes it possible to get hits in the texture cache and it ensures
> that transforms get computed on the GPU.
> 
> This fixes a severe performance regression in Chrome that happened
> when color correction was enabled.
> 
> Associated chromium patch for layout test rebaselines:
> https://chromium-review.googlesource.com/c/chromium/src/+/655483
> 
> Merge dependency: Merging this change to the M-62 and M-61
> branches also requires merging the following change, otherwise
> there will be rendering errors:
> https://skia-review.googlesource.com/c/skia/+/43562
> 
> BUG= chromium:760738 
> 
> Change-Id: I49fd5ef7968272d311249c3824fe15bee4648b73
> Reviewed-on: https://skia-review.googlesource.com/43183
> Commit-Queue: Justin Novosad <junov@chromium.org>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug:  chromium:760738 
Change-Id: I94a3482713dc4ff824ae21ea640c04d06bb7e29d
Reviewed-on: https://skia-review.googlesource.com/44760
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>

[modify] https://crrev.com/8b1360dcfab9ae748361268284068049cd3796a7/src/core/SkColorSpaceXformCanvas.cpp

Project Member

Comment 52 by sheriffbot@chromium.org, Sep 11 2017

Cc: gov...@chromium.org abdulsyed@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 53 by junov@chromium.org, Sep 11 2017

Labels: Disable-Nags

Comment 54 by junov@chromium.org, Sep 11 2017

Update: the last re-land should stick. Will wait for lkgr to go past the next skia roll before merging to the M62 branch

Comment 55 by ketakid@google.com, Sep 11 2017

Labels: -ReleaseBlock-Stable
Removing RBS.

Comment 56 by junov@chromium.org, Sep 11 2017

update: Fix will not make the cut for next stable channel update. Will apply for merge to stable in coming days after fix is proven in canary/dev channels.  Expect merge to M62 (beta) very soon.
Project Member

Comment 57 by bugdroid1@chromium.org, Sep 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0f8116eea7f2b68901db6f0fde752ea86a05bd20

commit 0f8116eea7f2b68901db6f0fde752ea86a05bd20
Author: Justin Novosad <junov@chromium.org>
Date: Mon Sep 11 23:24:54 2017

Rebaselining some layout tests after skia roll

Changes to test results related to:
https://skia.googlesource.com/skia.git/+/8b1360dcfab9ae748361268284068049cd3796a7

BUG= 760738 
TBR=xidachen@chromium.org

Change-Id: Ibb5b00df40d4fbd8be2856b561f5b5edfa35f306
Reviewed-on: https://chromium-review.googlesource.com/661617
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501091}
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/linux/virtual/gpu-rasterization/images/color-profile-background-image-space-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/linux/virtual/gpu-rasterization/images/color-profile-svg-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/linux/virtual/gpu-rasterization/images/color-profile-svg-fill-text-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/linux/virtual/gpu-rasterization/images/cross-fade-background-size-expected.png
[add] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/gpu-rasterization/images/color-profile-background-image-space-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac-mac10.10/virtual/gpu-rasterization/images/color-profile-svg-fill-text-expected.png
[copy] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/gpu-rasterization/images/color-profile-background-image-space-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac-mac10.9/virtual/gpu-rasterization/images/color-profile-svg-fill-text-expected.png
[rename] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/gpu-rasterization/images/color-profile-background-image-space-expected.png
[rename] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/gpu-rasterization/images/color-profile-svg-expected.png
[add] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-background-image-space-expected.png
[add] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-svg-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/color-profile-svg-fill-text-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/mac/virtual/gpu-rasterization/images/cross-fade-background-size-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-background-image-space-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-svg-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/color-profile-svg-fill-text-expected.png
[modify] https://crrev.com/0f8116eea7f2b68901db6f0fde752ea86a05bd20/third_party/WebKit/LayoutTests/platform/win/virtual/gpu-rasterization/images/cross-fade-background-size-expected.png

Project Member

Comment 58 by bugdroid1@chromium.org, Sep 12 2017

Labels: merge-merged-m62
The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/3407883a09eb3b3e33fac6d146a9b1d43041643b

commit 3407883a09eb3b3e33fac6d146a9b1d43041643b
Author: Brian Osman <brianosman@google.com>
Date: Tue Sep 12 16:42:01 2017

Make SkImage_Lazy always report the color space of its data

Legacy clients are likely to ignore this information, but there are
cases (using SkColorSpaceXformCanvas) where getting accurate information
is important.

This is necessary for https://skia-review.googlesource.com/c/skia/+/43742

No-Tree-Checks: true
No-Try: true
No-Presubmit: true
Bug:  chromium:760738 
Change-Id: I0709e437f781ebee863625af9cff38fb64d5cd0e
Reviewed-On: https://skia-review.googlesource.com/43562
Reviewed-By: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-on: https://skia-review.googlesource.com/43741
Reviewed-by: Brian Osman <brianosman@google.com>

[modify] https://crrev.com/3407883a09eb3b3e33fac6d146a9b1d43041643b/src/image/SkImage_Lazy.cpp
[modify] https://crrev.com/3407883a09eb3b3e33fac6d146a9b1d43041643b/src/image/SkImage_Gpu.cpp

Project Member

Comment 59 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/616b4641a875a99181b61380a559036aaa6ee254

commit 616b4641a875a99181b61380a559036aaa6ee254
Author: Brian Osman <brianosman@google.com>
Date: Tue Sep 12 16:44:21 2017

Optimize SkColorSpaceXformCanvas for GPU-acceleration

This change ensures that SkImages are uploaded to the GPU before
applying the xform when the destination canvas is on the GPU. This
makes it possible to get hits in the texture cache and it ensures
that transforms get computed on the GPU.

This fixes a severe performance regression in Chrome that happened
when color correction was enabled.

Associated chromium patch for layout test rebaselines:
https://chromium-review.googlesource.com/c/chromium/src/+/655483

Merge dependency: Merging this change to the M-62 and M-61
branches also requires merging the following change, otherwise
there will be rendering errors:
https://skia-review.googlesource.com/c/skia/+/43562

BUG= chromium:760738 

No-Tree-Checks: true
No-Try: true
No-Presubmit: true
Reviewed-On: https://skia-review.googlesource.com/43183
Commit-Queue: Justin Novosad <junov@chromium.org>
Reviewed-By: Brian Osman <brianosman@google.com>
Change-Id: I2046cefb32d644f9a73fd3f1231b87505d0f6ee3
Reviewed-on: https://skia-review.googlesource.com/43742
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Heather Miller <hcm@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

[modify] https://crrev.com/616b4641a875a99181b61380a559036aaa6ee254/src/core/SkColorSpaceXformCanvas.cpp

Comment 60 by junov@chromium.org, Sep 12 2017

Cc: brianosman@chromium.org pfeldman@chromium.org
Okay, now that this merged successfully to m62, let's ponder whether we need to push this to m61.

The patches that would need to be merged are the ones from #58 and #59

@brianosman: care to comment on the risk that these changes represent?

I don't have a good grasp on how severe/urgent this issue is to justify putting it in a stable channel update. @pfeldman: care to comment?
Risk: I think the second CL is reasonably safe, given that the first CL landed. The first CL is the one I'm more worried about - I convinced myself (and others) that it's correct, but there are several paths that hit the affected code, and we've had surprising failure modes before. (As it was, it took junov@ and I several tries to land on this version so that it fixed the issue without breaking something else).

Comment 62 by junov@chromium.org, Sep 13 2017

Status: Fixed (was: Assigned)
I'm inclined to suggest that we can live with the current state of things for the remainder of the M61 cycle.  If anyone has a good reason to disagree, please speak-up.

For now, I'm closing this issue.
Blockedon: 765569
 Issue 765569  seems to be the m61 incarnation of this issue.

And another developer mentioned what seems to be a related report: https://twitter.com/AlexanderTrefz/status/910133756379398144 But not sure if this is truly the same.

Comment 65 by junov@chromium.org, Sep 25 2017

Cc: krajshree@chromium.org junov@chromium.org pnangunoori@chromium.org susanjuniab@chromium.org
 Issue 765569  has been merged into this issue.
Labels: -Merge-Approved-62
Since this has been merged to M62, removing Merge-Approved label. 

krajshree@/jmukthavaram@ - can you please verify the fix in Beta?
Cc: divya.pa...@techmahindra.com
 Issue 773089  has been merged into this issue.

Comment 68 by junov@chromium.org, Oct 13 2017

 Issue 773827  has been merged into this issue.

Sign in to add a comment