Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2803853007:
Ignore rounding error between clip_layer_length_ and scroll_layer_length_ (Closed)
DescriptionIgnore rounding error between clip_layer_length_ and scroll_layer_length_
We determined overlay scrollbar should show by comparing
clip_layer_length with scroll_layer_length of scrollbar_layer_impl.
clip_layer_length_ has rounding error form pagescale division. The
scrollbar will appear when the rounding error makes clip_layer_length <
scroll_layer_length.
In this patch, we change to use ceil of clip length in device pixel and
floor of scroll length in device pixel for
compare. That means we only appear the scrollbar when clip_layer_length
is at least 1 physical px smaller than scroll_layer_length.
BUG=709062
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2803853007
Cr-Commit-Position: refs/heads/master@{#466570}
Committed: https://chromium.googlesource.com/chromium/src/+/beb19669855bab47cda70ea40fbbe154eb3bfdc5
Patch Set 1 #
Total comments: 1
Patch Set 2 : bokan comment addressed #Patch Set 3 : add test #
Total comments: 5
Patch Set 4 : remove pagescale & move test to ScrollbarLayerTest #Patch Set 5 : rebase update #
Total comments: 1
Patch Set 6 : add page_scale #
Total comments: 1
Patch Set 7 : change scroll to floor #Patch Set 8 : change to epsilon #
Total comments: 2
Patch Set 9 : expose IsFloatNearlyTheSame & add comment #
Messages
Total messages: 72 (45 generated)
Description was changed from ========== Ignore rounding error between clip_layer_length_ and scroll_layer_length_ This issue is caused by clip_layer_length_ is calculate from viewport size and page scale factor. In the patch we add a float equality check in |ScrollbarLayerImplBase::CanScrollOrientation| to skip the scrollbar paint when clip_layer_length_ and scroll_layer_length_ only has rounding difference. BUG=709062 ========== to ========== Ignore rounding error between clip_layer_length_ and scroll_layer_length_ This issue is caused by clip_layer_length_ is calculate from viewport size and page scale factor. In the patch we add a float equality check in |ScrollbarLayerImplBase::CanScrollOrientation| to skip the scrollbar paint when clip_layer_length_ and scroll_layer_length_ only has rounding difference. BUG=709062 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run
chaopeng@chromium.org changed reviewers: + bokan@chromium.org
PTAL. Thank you.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
Patchset #1 (id:1) has been deleted
The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2803853007/diff/20001/cc/layers/scrollbar_lay... File cc/layers/scrollbar_layer_impl_base.cc (right): https://codereview.chromium.org/2803853007/diff/20001/cc/layers/scrollbar_lay... cc/layers/scrollbar_layer_impl_base.cc:71: !MathUtil::IsFloatNearlyTheSame(clip_layer_length_, Thinking about this some more: this limit is kind of arbitrary. How about this instead: convert the clip and scroll sizes into device pixels using layer_tree_impl_->device_scale_factor() and ceil that. In that case, we're not losing anything since its impossible to scroll by less than a device pixel. Not sure if we should ceil the scroll layer or floor it...I think it might not matter as I expect that to always be a whole number.
Patchset #2 (id:40001) has been deleted
Use ceil to resolve the rounding error. PTAL. Thank you.
this looks fine to me. But it'll need a test.
Description was changed from ========== Ignore rounding error between clip_layer_length_ and scroll_layer_length_ This issue is caused by clip_layer_length_ is calculate from viewport size and page scale factor. In the patch we add a float equality check in |ScrollbarLayerImplBase::CanScrollOrientation| to skip the scrollbar paint when clip_layer_length_ and scroll_layer_length_ only has rounding difference. BUG=709062 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Ignore rounding error between clip_layer_length_ and scroll_layer_length_ We determined overlay scrollbar should show by comparing clip_layer_length with scroll_layer_length of scrollbar_layer_impl. clip_layer_length_ has rounding error form pagescale division. The scrollbar will appear when the rounding error makes clip_layer_length < scroll_layer_length. In this patch, we change to use ceilling of length in device pixel for compare. That means we only appear the scrollbar when clip_layer_length is at least 1 physical px smaller than scroll_layer_length. BUG=709062 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
Added test. PTAL. Thank you.
https://codereview.chromium.org/2803853007/diff/80001/cc/layers/scrollbar_lay... File cc/layers/scrollbar_layer_impl_base.cc (right): https://codereview.chromium.org/2803853007/diff/80001/cc/layers/scrollbar_lay... cc/layers/scrollbar_layer_impl_base.cc:70: float page_scale_factor = layer_tree_impl()->current_page_scale_factor(); I don't think this is right, why'd you add page scale from the previous patch? The page scale factor is already applied as needed from UpdateScrollbars. Further, it should only be applied for viewport scrollbars. This looked fine to me in the previous patch. https://codereview.chromium.org/2803853007/diff/80001/cc/layers/solid_color_s... File cc/layers/solid_color_scrollbar_layer_impl_unittest.cc (right): https://codereview.chromium.org/2803853007/diff/80001/cc/layers/solid_color_s... cc/layers/solid_color_scrollbar_layer_impl_unittest.cc:80: TEST(SolidColorScrollbarLayerImplTest, RoundingError) { This isn't specific to solid color scrollbar layers, lets put it in scrollbar_layer_unittest or somewhere more general. Also, please use a more descriptive name for the test. Perhaps SubPixelCanScrollOrientation https://codereview.chromium.org/2803853007/diff/80001/cc/layers/solid_color_s... cc/layers/solid_color_scrollbar_layer_impl_unittest.cc:107: EXPECT_FLOAT_EQ(100.0f, scrollbar_layer->scroll_layer_length()); No need to test these. https://codereview.chromium.org/2803853007/diff/80001/cc/layers/solid_color_s... cc/layers/solid_color_scrollbar_layer_impl_unittest.cc:117: impl.host_impl()->active_tree()->current_page_scale_factor()); No need to test any of the above lines. If you really want a "sanity-check", you should ASSERT rather than EXPECT. A failing ASSERT means the test itself is broken, failing EXPECT means what we're checking is broken. https://codereview.chromium.org/2803853007/diff/80001/cc/layers/solid_color_s... cc/layers/solid_color_scrollbar_layer_impl_unittest.cc:119: EXPECT_FALSE(scrollbar_layer->CanScrollOrientation()); You should also test the converse, that if the clip layer has a device pixel of difference from the scroll layer it is scrollable.
Updated, PTAL. Thank you.
The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2803853007/diff/120001/cc/layers/scrollbar_la... File cc/layers/scrollbar_layer_impl_base.cc (right): https://codereview.chromium.org/2803853007/diff/120001/cc/layers/scrollbar_la... cc/layers/scrollbar_layer_impl_base.cc:76: int clip_layer_length = std::ceil(clip_layer_length_ * device_scale_factor); Sorry to flip flop again, but I think you were right before - sorry! We should multiply here by page_scale_factor. This gets us to physical pixels in all cases. For the viewport, the clip layer will have been divided by PSF in UpdateScrollbars so we'll undo that here and multiply by DSF to get the real viewport size in device pixels. Multiplying the scroll layer below by PSF and DSF makes it so that we're "keeping the viewport fixed in size while scaling scroll layer" for zooming. And for non viewport layers, we also want to multiply by PSF because as we zoom in, we can scroll those layers in smaller and smaller increments so a scroller that wasn't scrollable at a small zoom might become scrollable at higher zooms. e.g. At PSF=1 DSF=2 with scroll_length=100.3 and clip_length=100 - since DSF=2, there's 2 DP per 1 DIP so there's no scrolling possible while PSF is 1. If we zoom in to PSF=2, the scroll_length=401.2 and the clip_length=400 in DP. So now we have 1 device pixel of scrolling. As the example demonstrates though, ceil is the wrong thing to do for scroll_layer_length. I think we should ceil for clip and floor for scroll. My floating point knowledge is rusty but I think the introduced error from multiplying two ints can in general go in different directions (scroll might be slightly over an integer and clip might be slightly under). I don't think it'll matter in practice but flooring clip means we'll err to not scrolling which I think is right thing to do. Does that make sense? If so, lets bring back PSF, floor clip, and I'll stamp. (And to make this clearer, call the variables "clip_device_pixels" and "scroll_device_pixels".)
On 2017/04/20 21:34:36, bokan wrote: > https://codereview.chromium.org/2803853007/diff/120001/cc/layers/scrollbar_la... > File cc/layers/scrollbar_layer_impl_base.cc (right): > > https://codereview.chromium.org/2803853007/diff/120001/cc/layers/scrollbar_la... > cc/layers/scrollbar_layer_impl_base.cc:76: int clip_layer_length = > std::ceil(clip_layer_length_ * device_scale_factor); > Sorry to flip flop again, but I think you were right before - sorry! We should > multiply here by page_scale_factor. This gets us to physical pixels in all > cases. > > For the viewport, the clip layer will have been divided by PSF in > UpdateScrollbars so we'll undo that here and multiply by DSF to get the real > viewport size in device pixels. Multiplying the scroll layer below by PSF and > DSF makes it so that we're "keeping the viewport fixed in size while scaling > scroll layer" for zooming. > > And for non viewport layers, we also want to multiply by PSF because as we zoom > in, we can scroll those layers in smaller and smaller increments so a scroller > that wasn't scrollable at a small zoom might become scrollable at higher zooms. > e.g. At PSF=1 DSF=2 with scroll_length=100.3 and clip_length=100 - since DSF=2, > there's 2 DP per 1 DIP so there's no scrolling possible while PSF is 1. If we > zoom in to PSF=2, the scroll_length=401.2 and the clip_length=400 in DP. So now > we have 1 device pixel of scrolling. > > As the example demonstrates though, ceil is the wrong thing to do for > scroll_layer_length. I think we should ceil for clip and floor for scroll. My > floating point knowledge is rusty but I think the introduced error from > multiplying two ints can in general go in different directions (scroll might be > slightly over an integer and clip might be slightly under). I don't think it'll > matter in practice but flooring clip means we'll err to not scrolling which I > think is right thing to do. > > Does that make sense? If so, lets bring back PSF, floor clip, and I'll stamp. > (And to make this clearer, call the variables "clip_device_pixels" and > "scroll_device_pixels".) Yes, I added PSF back. PTAL. Thank you.
The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
https://codereview.chromium.org/2803853007/diff/140001/cc/layers/scrollbar_la... File cc/layers/scrollbar_layer_impl_base.cc (right): https://codereview.chromium.org/2803853007/diff/140001/cc/layers/scrollbar_la... cc/layers/scrollbar_layer_impl_base.cc:80: std::ceil(scroll_layer_length_ * page_scale_factor * device_scale_factor); We should floor scroll instead of ceil, see previous comment for why. E.g. DSF = 1 PSF = 2.3804 Clip = 49.99 Scroll = 50 So in DP: Clip = 118.996196 Scroll = 119.02 So if we ceil both we'll get 119 < 120 which we clearly didn't intend.
bokan@chromium.org changed reviewers: + aelias@chromium.org
Thanks, lgtm +aelias@ for CC owner since he's helped us deal with precision issues in the past.
On 2017/04/20 22:09:49, bokan wrote: > https://codereview.chromium.org/2803853007/diff/140001/cc/layers/scrollbar_la... > File cc/layers/scrollbar_layer_impl_base.cc (right): > > https://codereview.chromium.org/2803853007/diff/140001/cc/layers/scrollbar_la... > cc/layers/scrollbar_layer_impl_base.cc:80: std::ceil(scroll_layer_length_ * > page_scale_factor * device_scale_factor); > We should floor scroll instead of ceil, see previous comment for why. > > E.g. > > DSF = 1 > PSF = 2.3804 > Clip = 49.99 > Scroll = 50 > > So in DP: > > Clip = 118.996196 > Scroll = 119.02 > > So if we ceil both we'll get 119 < 120 which we clearly didn't intend. Right, we should floor scroll length. Updated, PTAL. Thank you.
Also, please update the commit message, the approach has changed somewhat since it was written.
The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...) win_clang on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)
Description was changed from ========== Ignore rounding error between clip_layer_length_ and scroll_layer_length_ We determined overlay scrollbar should show by comparing clip_layer_length with scroll_layer_length of scrollbar_layer_impl. clip_layer_length_ has rounding error form pagescale division. The scrollbar will appear when the rounding error makes clip_layer_length < scroll_layer_length. In this patch, we change to use ceilling of length in device pixel for compare. That means we only appear the scrollbar when clip_layer_length is at least 1 physical px smaller than scroll_layer_length. BUG=709062 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Ignore rounding error between clip_layer_length_ and scroll_layer_length_ We determined overlay scrollbar should show by comparing clip_layer_length with scroll_layer_length of scrollbar_layer_impl. clip_layer_length_ has rounding error form pagescale division. The scrollbar will appear when the rounding error makes clip_layer_length < scroll_layer_length. In this patch, we change to use ceil of clip length in device pixel and floor of scroll length in device pixel for compare. That means we only appear the scrollbar when clip_layer_length is at least 1 physical px smaller than scroll_layer_length. BUG=709062 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
I'm OK with the idea of using ceil() on the clip layer -- in general using ceil() on viewport size instead of floor() seems like a best practice in my experience. However, the scrollbar layer is not necessarily the only place that will run into this problem. For instance, the overscroll glow effect is also interested in axis-scrollability, and I've also encountered various 1px-fringe bugs. Could you try changing the calculation to ceil() at the original place where the viewport size is converted to DIP instead (which likely has a floor() today, possibly an implicit one, or a round()), which feeds into clip_layer_length_?
On 2017/04/21 00:36:47, aelias wrote: > I'm OK with the idea of using ceil() on the clip layer -- in general using > ceil() on viewport size instead of floor() seems like a best practice in my > experience. However, the scrollbar layer is not necessarily the only place that > will run into this problem. For instance, the overscroll glow effect is also > interested in axis-scrollability, and I've also encountered various 1px-fringe > bugs. Could you try changing the calculation to ceil() at the original place > where the viewport size is converted to DIP instead (which likely has a floor() > today, possibly an implicit one, or a round()), which feeds into > clip_layer_length_? I have tried only apply ceil to clip_layer_length_. But is it possible we have case like: clip_layer_length_ have 0.5px smaller than scroll_layer_length in DIP and 1px in DP.
OK, never mind what I said above, I looked at the bug history and see it is specific to scrollbars, since it was introduced in https://cs.chromium.org/chromium/src/cc/trees/layer_tree_impl.cc?l=242 . Since the problem you observed is that it's off by an epsilon due to actual floating-point error as opposed to DIP clamping, could you just replace it with an epsilon comparison instead of doing all those multiplications, ceils and floors? I.e. (clip_layer_length_ + 0.001f < scroll_layer_length_)
OK, I see that was your original solution. Well, apologies for the code reviewer disagreement/whiplash but I do like it better than the one bokan@ prefers. From my POV the epsilon is not that arbitrary, it's higher than the amount of error floating point can realistically accumulate with the operations we're doing, and lower than any zoom level increment any user cares about. On the other hand bokan@'s proposal makes a valiant attempt at being principled but what it ultimately does is use the scale factors as an variable-sized, much larger epsilon which isn't truly calibrated to the original sources of error even if it may use some of the same input numbers. Anyway, I'm OK in principle with using epsilons to counter floating-point errors for the purposes of pinch zoom and scrolling. I've never seen it cause a bug.
Note your new test is currently red because the epsilon yielded by bokan@'s method is large enough that 99.65f is also considered nonscrollable. Anyway, that being said I don't think any of these principled distinctions will ever matter in practice, and I do think applying some manner of epsilon here is OK, so in the spirit of avoiding bikeshedding lgtm regardless of how you choose to fix the test.
On 2017/04/21 01:50:41, aelias wrote: > Note your new test is currently red because the epsilon yielded by bokan@'s > method is large enough that 99.65f is also considered nonscrollable. > > Anyway, that being said I don't think any of these principled distinctions will > ever matter in practice, and I do think applying some manner of epsilon here is > OK, so in the spirit of avoiding bikeshedding lgtm regardless of how you choose > to fix the test. Thank you.
One more thing, since you need to change it anyway, could you rewrite the test to test the specific values you observed in the bug? (979.999939, 980.000000, and whatever page scale and device scale). I think that will make the test more useful.
I agree in practice an epsilon will likely work well enough so aelias' suggestion sgtm too. Sorry Chao for the back-and-forth.
chaopeng@chromium.org changed reviewers: + enne@chromium.org
Updated, PTAL. +enne for math. Thank you.
The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2803853007/diff/180001/cc/base/math_util.cc File cc/base/math_util.cc (right): https://codereview.chromium.org/2803853007/diff/180001/cc/base/math_util.cc#n... cc/base/math_util.cc:820: bool MathUtil::IsFloatNearlyTheSame(float left, float right) { I'm not sure we need both IsFloatNearlyTheSame and IsNearlyTheSameForTesting and IsNearlyTheSame. I think maybe just call this IsFloatNearlyTheSame or expose IsNearlyTheSame? https://codereview.chromium.org/2803853007/diff/180001/cc/layers/scrollbar_la... File cc/layers/scrollbar_layer_impl_base.cc (right): https://codereview.chromium.org/2803853007/diff/180001/cc/layers/scrollbar_la... cc/layers/scrollbar_layer_impl_base.cc:71: !MathUtil::IsFloatNearlyTheSame(clip_layer_length_, Can you leave a comment here about why you need this check, and that the intended behavior is not to show the scrollbar when there's a rounding error and these are nearly the same?
Patchset #9 (id:200001) has been deleted
On 2017/04/21 22:54:33, enne wrote: > https://codereview.chromium.org/2803853007/diff/180001/cc/base/math_util.cc > File cc/base/math_util.cc (right): > > https://codereview.chromium.org/2803853007/diff/180001/cc/base/math_util.cc#n... > cc/base/math_util.cc:820: bool MathUtil::IsFloatNearlyTheSame(float left, float > right) { > I'm not sure we need both IsFloatNearlyTheSame and IsNearlyTheSameForTesting and > IsNearlyTheSame. I think maybe just call this IsFloatNearlyTheSame or expose > IsNearlyTheSame? > > https://codereview.chromium.org/2803853007/diff/180001/cc/layers/scrollbar_la... > File cc/layers/scrollbar_layer_impl_base.cc (right): > > https://codereview.chromium.org/2803853007/diff/180001/cc/layers/scrollbar_la... > cc/layers/scrollbar_layer_impl_base.cc:71: > !MathUtil::IsFloatNearlyTheSame(clip_layer_length_, > Can you leave a comment here about why you need this check, and that the > intended behavior is not to show the scrollbar when there's a rounding error and > these are nearly the same? Expose IsFloatNearlyTheSame and add comment . PTAL. Thank you.
The CQ bit was checked by chaopeng@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by chaopeng@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org, aelias@chromium.org Link to the patchset: https://codereview.chromium.org/2803853007/#ps220001 (title: "expose IsFloatNearlyTheSame & add comment")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 220001, "attempt_start_ts": 1492985829236540,
"parent_rev": "583e49bd24f395a014cc4ac9c51cc8b359fe97a3", "commit_rev":
"beb19669855bab47cda70ea40fbbe154eb3bfdc5"}
Message was sent while issue was closed.
Description was changed from ========== Ignore rounding error between clip_layer_length_ and scroll_layer_length_ We determined overlay scrollbar should show by comparing clip_layer_length with scroll_layer_length of scrollbar_layer_impl. clip_layer_length_ has rounding error form pagescale division. The scrollbar will appear when the rounding error makes clip_layer_length < scroll_layer_length. In this patch, we change to use ceil of clip length in device pixel and floor of scroll length in device pixel for compare. That means we only appear the scrollbar when clip_layer_length is at least 1 physical px smaller than scroll_layer_length. BUG=709062 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Ignore rounding error between clip_layer_length_ and scroll_layer_length_ We determined overlay scrollbar should show by comparing clip_layer_length with scroll_layer_length of scrollbar_layer_impl. clip_layer_length_ has rounding error form pagescale division. The scrollbar will appear when the rounding error makes clip_layer_length < scroll_layer_length. In this patch, we change to use ceil of clip length in device pixel and floor of scroll length in device pixel for compare. That means we only appear the scrollbar when clip_layer_length is at least 1 physical px smaller than scroll_layer_length. BUG=709062 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2803853007 Cr-Commit-Position: refs/heads/master@{#466570} Committed: https://chromium.googlesource.com/chromium/src/+/beb19669855bab47cda70ea40fbb... ==========
Message was sent while issue was closed.
Committed patchset #9 (id:220001) as https://chromium.googlesource.com/chromium/src/+/beb19669855bab47cda70ea40fbb... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
