Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(247)

Issue 2298543002: Check if the opacity set from animation is same as the target value

Created:
4 years, 3 months ago by oshima
Modified:
4 years, 1 month ago
CC:
chromium-reviews, Ian Vollick, sievers+watch_chromium.org, jbauman+watch_chromium.org, piman+watch_chromium.org, kalyank, danakj+watch_chromium.org, cc-bugs_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Check if the opacity set from animation is same as the target value to avoid unnecessary redraw. Setting the same opacity was causing redrawing. BUG=642223

Patch Set 1 #

Patch Set 2 : Skip if the target value is same #

Total comments: 2

Patch Set 3 : Check if the value set from animation is same as the target value #

Total comments: 1

Patch Set 4 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -1 line) Patch
M ui/compositor/layer.cc View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 33 (21 generated)
oshima
4 years, 3 months ago (2016-08-31 02:55:02 UTC) #13
oshima
ping?
4 years, 3 months ago (2016-09-02 01:04:49 UTC) #14
oshima
friendly ping
4 years, 2 months ago (2016-09-30 21:30:29 UTC) #15
oshima
loyso@, can you review this?
4 years, 1 month ago (2016-11-15 16:16:19 UTC) #17
loyso (OOO)
We miss vollick@. https://codereview.chromium.org/2298543002/diff/20001/ui/compositor/layer.cc File ui/compositor/layer.cc (right): https://codereview.chromium.org/2298543002/diff/20001/ui/compositor/layer.cc#newcode913 ui/compositor/layer.cc:913: return; We have similar check inside ...
4 years, 1 month ago (2016-11-15 23:10:07 UTC) #19
oshima
https://codereview.chromium.org/2298543002/diff/20001/ui/compositor/layer.cc File ui/compositor/layer.cc (right): https://codereview.chromium.org/2298543002/diff/20001/ui/compositor/layer.cc#newcode913 ui/compositor/layer.cc:913: return; On 2016/11/15 23:10:07, loyso wrote: > We have ...
4 years, 1 month ago (2016-11-16 14:27:35 UTC) #23
loyso (OOO)
On 2016/11/16 14:27:35, oshima wrote: > It looks like brightness and grayscale may not it ...
4 years, 1 month ago (2016-11-17 00:11:53 UTC) #24
oshima
https://codereview.chromium.org/2298543002/diff/60001/ui/compositor/layer.cc File ui/compositor/layer.cc (right): https://codereview.chromium.org/2298543002/diff/60001/ui/compositor/layer.cc#newcode1024 ui/compositor/layer.cc:1024: cc_layer_->SetOpacity(opacity); Is it ok to change SetOpacity to return ...
4 years, 1 month ago (2016-11-17 00:15:01 UTC) #25
loyso (OOO)
cc::Layer::SetOpacity has that check and it calls SetNeedsCommit if needed. All you need is to ...
4 years, 1 month ago (2016-11-17 00:16:39 UTC) #26
oshima
On 2016/11/17 00:16:39, loyso wrote: > cc::Layer::SetOpacity has that check and it calls SetNeedsCommit if ...
4 years, 1 month ago (2016-11-17 14:12:45 UTC) #27
loyso (OOO)
non-owner LGTM.
4 years, 1 month ago (2016-11-17 23:48:39 UTC) #32
loyso (OOO)
4 years, 1 month ago (2016-11-17 23:49:43 UTC) #33
p.s. Could you add a unit test, please?

Powered by Google App Engine
This is Rietveld 408576698