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

Issue 1321503002: cc: Do not create separate tilings for almost equal scale factors.

Created:
5 years, 3 months ago by prashant.n
Modified:
5 years, 1 month ago
CC:
cc-bugs_chromium.org, chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

cc: Do not create separate tilings for almost equal scale factors. Different scale factors which are almost equal (i.e. differ by some threshold magnitude of floating point epsilon) create different tilings when content is rigourously zoomed. e.g. If scales are 7.33907556533813f and 7.33907508850098f, different tilings get created, when scale is changed from one to other. Although tiles are shared between different tilings, the contents at previous scale needs to be re-rastered with new scale. This causes unnecessary performance hit. The new function added converts the scale factor with given floating point to fixed precision floating point. This makes above two scale factors considered as equal. The above two scales will get converted as given below, when precision is set as 4. 7.33907556533813 -> 7.339000225067138671875 7.33907508850098 -> 7.339000225067138671875 BUG=327166 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Patch Set 1 #

Patch Set 2 : Removed #if #

Patch Set 3 : #

Total comments: 2

Patch Set 4 : Addressed few review comments. #

Total comments: 2

Patch Set 5 : Added unit test. #

Patch Set 6 : Removed unrelated changes. #

Total comments: 3

Patch Set 7 : Review comments. #

Patch Set 8 : Updated unit test. #

Patch Set 9 : Small nit. #

Patch Set 10 : Made comment more readable. #

Patch Set 11 : nit. #

Total comments: 8

Patch Set 12 : Review comments + algorithm modified. #

Total comments: 2

Patch Set 13 : moved roundto function to ideal scales. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+103 lines, -4 lines) Patch
M cc/base/math_util.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +14 lines, -0 lines 0 comments Download
M cc/base/math_util.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +26 lines, -0 lines 0 comments Download
M cc/base/math_util_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +50 lines, -0 lines 0 comments Download
M cc/layers/picture_layer_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +13 lines, -4 lines 1 comment Download

Depends on Patchset:

Messages

Total messages: 19 (2 generated)
prashant.n
PTAL.
5 years, 3 months ago (2015-08-27 16:57:50 UTC) #2
enne (OOO)
Test? https://codereview.chromium.org/1321503002/diff/40001/cc/layers/picture_layer_impl.cc File cc/layers/picture_layer_impl.cc (right): https://codereview.chromium.org/1321503002/diff/40001/cc/layers/picture_layer_impl.cc#newcode79 cc/layers/picture_layer_impl.cc:79: const float factor = pow(10, kScalePrecision); It is ...
5 years, 3 months ago (2015-08-27 18:08:34 UTC) #3
prashant.n
I'll add the test once RoundTo method is finalized. https://codereview.chromium.org/1321503002/diff/60001/cc/layers/picture_layer_impl.cc File cc/layers/picture_layer_impl.cc (right): https://codereview.chromium.org/1321503002/diff/60001/cc/layers/picture_layer_impl.cc#newcode65 cc/layers/picture_layer_impl.cc:65: ...
5 years, 3 months ago (2015-09-01 16:55:25 UTC) #4
prashant.n
PTAL.
5 years, 2 months ago (2015-10-16 14:44:44 UTC) #5
vmpstr
https://codereview.chromium.org/1321503002/diff/100001/cc/base/math_util.cc File cc/base/math_util.cc (right): https://codereview.chromium.org/1321503002/diff/100001/cc/base/math_util.cc#newcode124 cc/base/math_util.cc:124: const int kScalePrecision = 4; Can you just write ...
5 years, 2 months ago (2015-10-16 17:58:17 UTC) #6
prashant.n
https://codereview.chromium.org/1321503002/diff/100001/cc/base/math_util.cc File cc/base/math_util.cc (right): https://codereview.chromium.org/1321503002/diff/100001/cc/base/math_util.cc#newcode124 cc/base/math_util.cc:124: const int kScalePrecision = 4; On 2015/10/16 17:58:17, vmpstr ...
5 years, 2 months ago (2015-10-17 01:28:13 UTC) #7
prashant.n
PTAL.
5 years, 2 months ago (2015-10-17 14:44:05 UTC) #8
ericrk
https://codereview.chromium.org/1321503002/diff/200001/cc/base/math_util.cc File cc/base/math_util.cc (right): https://codereview.chromium.org/1321503002/diff/200001/cc/base/math_util.cc#newcode125 cc/base/math_util.cc:125: if (precision < 1 || precision > (std::numeric_limits<float>::digits10 - ...
5 years, 2 months ago (2015-10-19 16:40:50 UTC) #10
vmpstr
https://codereview.chromium.org/1321503002/diff/200001/cc/base/math_util.cc File cc/base/math_util.cc (right): https://codereview.chromium.org/1321503002/diff/200001/cc/base/math_util.cc#newcode125 cc/base/math_util.cc:125: if (precision < 1 || precision > (std::numeric_limits<float>::digits10 - ...
5 years, 2 months ago (2015-10-19 18:58:38 UTC) #11
prashant.n
Pls. take a look at rounding algorithm modified. I'll check the contents rastered with different ...
5 years, 2 months ago (2015-10-20 18:45:24 UTC) #12
enne (OOO)
On 2015/10/20 at 18:45:24, prashant.n wrote: > Pls. take a look at rounding algorithm modified. ...
5 years, 2 months ago (2015-10-20 18:54:57 UTC) #13
vmpstr
https://codereview.chromium.org/1321503002/diff/200001/cc/layers/picture_layer_impl.cc File cc/layers/picture_layer_impl.cc (right): https://codereview.chromium.org/1321503002/diff/200001/cc/layers/picture_layer_impl.cc#newcode954 cc/layers/picture_layer_impl.cc:954: MathUtil::RoundToFixedPrecision(ideal_contents_scale_, kScalePrecision); On 2015/10/20 18:45:24, prashant.n wrote: > We ...
5 years, 2 months ago (2015-10-20 18:55:23 UTC) #14
prashant.n
> When we shipped impl-side painting, we introduced an additional 0.5 pixels to > the ...
5 years, 1 month ago (2015-10-26 15:53:49 UTC) #15
prashant.n
https://codereview.chromium.org/1321503002/diff/220001/cc/base/math_util.cc File cc/base/math_util.cc (right): https://codereview.chromium.org/1321503002/diff/220001/cc/base/math_util.cc#newcode134 cc/base/math_util.cc:134: int digits10_in_integral_part = This additional code adds ~25% overhead. ...
5 years, 1 month ago (2015-10-26 15:54:19 UTC) #16
prashant.n
https://codereview.chromium.org/1321503002/diff/240001/cc/layers/picture_layer_impl.cc File cc/layers/picture_layer_impl.cc (right): https://codereview.chromium.org/1321503002/diff/240001/cc/layers/picture_layer_impl.cc#newcode1203 cc/layers/picture_layer_impl.cc:1203: std::max(GetIdealContentsScale(), min_contents_scale), kScalePrecision); Modifying ideal page and contents scales ...
5 years, 1 month ago (2015-10-26 16:35:04 UTC) #17
prashant.n
I guess the main problem would be smooth sequence of scale factors in zooming.
5 years, 1 month ago (2015-10-26 19:11:35 UTC) #18
prashant.n
5 years, 1 month ago (2015-10-28 14:29:34 UTC) #19
On 2015/10/26 19:11:35, prashant.n wrote:
> I guess the main problem would be smooth sequence of scale factors in zooming.

Dear All,

I did further analysis on this and attached my analysis in the bug. Kindly check
and do let me know the opinion for proceeding on this patch.

Powered by Google App Engine
This is Rietveld 408576698