New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 6 users
Status: Accepted
Owner:
Cc:
Area: ----
Priority: Medium
Type: Defect



Sign in to add a comment
Disallow expanding clips in SkCanvas
Project Member Reported by m...@reedtribe.org, Dec 6 2014 Back to list
Today SkCanvas can expand its clip, via several SkRegion ops:
- union, reverse-diff, xor, replace

This expandability makes certain optimizations much harder in pictures (e.g. culling)

Goal: change SkCanvas API to remove ops that allow expansion

Observation:
- blink never uses any of these ops, they only seem to use Intersect and Difference
- chrome uses Replace in a few places, but hopefully those can be changed
- unknown how prevalent Android apps use these "bad" ops

Strategy:
1. offer new API for SkCanvas clipping (using new enum type SkClipOp) -- skia
2. change blink and chrome to use this new enum, and rename any remaining call-sites "legacy" -- blink, chrome
3. fix legacy callers (hard) -- chrome
4. remove support for "legacy" -- skia

 
Project Member Comment 1 by m...@reedtribe.org, Dec 6 2014
Project Member Comment 2 by m...@reedtribe.org, Dec 7 2014
Cc: fmalita@chromium.org schenney@chromium.org
Project Member Comment 4 by hcm@google.com, Dec 7 2015
Labels: Hotlist-Fixit
Project Member Comment 6 by bugdroid1@chromium.org, Mar 15 2017
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/844158bb22c9b0008e11689bf3370f14295cfe65

commit 844158bb22c9b0008e11689bf3370f14295cfe65
Author: reed <reed@google.com>
Date: Wed Mar 15 22:56:21 2017

use correct clipop names

pure enum renaming -- no logic change.

BUG=skia:3191
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
TBR=

Review-Url: https://codereview.chromium.org/2751833010
Cr-Commit-Position: refs/heads/master@{#457248}

[modify] https://crrev.com/844158bb22c9b0008e11689bf3370f14295cfe65/cc/output/software_renderer.cc
[modify] https://crrev.com/844158bb22c9b0008e11689bf3370f14295cfe65/skia/config/SkUserConfig.h

Project Member Comment 7 by bugdroid1@chromium.org, May 10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/949391a2d37d179287f6a8b16675449ac1e9bcb7

commit 949391a2d37d179287f6a8b16675449ac1e9bcb7
Author: reed <reed@google.com>
Date: Wed May 10 16:46:52 2017

add guard for deprecated clipops

pending skia CL: https://skia-review.googlesource.com/c/16365/

BUG=skia:3191

Review-Url: https://codereview.chromium.org/2877493002
Cr-Commit-Position: refs/heads/master@{#470610}

[modify] https://crrev.com/949391a2d37d179287f6a8b16675449ac1e9bcb7/skia/config/SkUserConfig.h

Project Member Comment 9 by reed@google.com, May 11
Cc: vandebo@chromium.org reed@google.com
 Issue 228  has been merged into this issue.
Project Member Comment 10 by bugdroid1@chromium.org, May 11
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96bef856446afe2cd5a7fc1b5ca46105b3cf87cd

commit 96bef856446afe2cd5a7fc1b5ca46105b3cf87cd
Author: reed <reed@google.com>
Date: Thu May 11 20:49:30 2017

remove unneeded SK_SUPPORT_DEPRECATED_CLIPOPS flag

BUG=skia:3191

Review-Url: https://codereview.chromium.org/2875043004
Cr-Commit-Position: refs/heads/master@{#471064}

[modify] https://crrev.com/96bef856446afe2cd5a7fc1b5ca46105b3cf87cd/skia/config/SkUserConfig.h

Project Member Comment 11 by reed@google.com, May 11
Now the only call-sites are in Android, and these are deprecated at the Java level.

Holding breath...
Sign in to add a comment