Commit d2b94f45 authored by Shimi Zhang's avatar Shimi Zhang Committed by Commit Bot

aw: Add rounding to BVR::SetTotalRootLayerScrollOffset() calculation.

The intermediate multiplication result was too large to store in float
precisely, therefore there was a rounding error. To mitigate this, we
apply a rounding for the integer conversion.

We only need this for scale up since the granularity is finer.

Bug: 881458
Change-Id: I9c07fdfb71fcbc59f36f5094bf813053cac8d8e8
Reviewed-on: https://chromium-review.googlesource.com/1214686
Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
Reviewed-by: default avatarBo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590147}
parent 8062b71b
...@@ -631,13 +631,15 @@ void BrowserViewRenderer::SetTotalRootLayerScrollOffset( ...@@ -631,13 +631,15 @@ void BrowserViewRenderer::SetTotalRootLayerScrollOffset(
// For an explanation as to why this is done this way see the comment in // For an explanation as to why this is done this way see the comment in
// BrowserViewRenderer::ScrollTo. // BrowserViewRenderer::ScrollTo.
if (max_scroll_offset_unscaled_.x()) { if (max_scroll_offset_unscaled_.x()) {
scroll_offset.set_x((scroll_offset_unscaled.x() * max_offset.x()) / scroll_offset.set_x(
max_scroll_offset_unscaled_.x()); gfx::ToRoundedInt((scroll_offset_unscaled.x() * max_offset.x()) /
max_scroll_offset_unscaled_.x()));
} }
if (max_scroll_offset_unscaled_.y()) { if (max_scroll_offset_unscaled_.y()) {
scroll_offset.set_y((scroll_offset_unscaled.y() * max_offset.y()) / scroll_offset.set_y(
max_scroll_offset_unscaled_.y()); gfx::ToRoundedInt((scroll_offset_unscaled.y() * max_offset.y()) /
max_scroll_offset_unscaled_.y()));
} }
DCHECK_LE(0, scroll_offset.x()); DCHECK_LE(0, scroll_offset.x());
......
...@@ -547,4 +547,51 @@ class RenderThreadManagerSwitchTest : public ResourceRenderingTest { ...@@ -547,4 +547,51 @@ class RenderThreadManagerSwitchTest : public ResourceRenderingTest {
RENDERING_TEST_F(RenderThreadManagerSwitchTest); RENDERING_TEST_F(RenderThreadManagerSwitchTest);
// Test for https://crbug.com/881458, this test is to make sure we will reach
// the maximal scroll offset.
class DidReachMaximalScrollOffsetTest : public RenderingTest {
public:
void StartTest() override {
browser_view_renderer_->SetDipScale(kDipScale);
// |UpdateRootLayerState()| will call |SetTotalRootLayerScrollOffset()|.
browser_view_renderer_->UpdateRootLayerState(
ActiveCompositor(), kTotalScrollOffset, kTotalMaxScrollOffset,
kScrollableSize, kPageScaleFactor, kMinPageScaleFactor,
kMaxPageScaleFactor);
}
void ScrollContainerViewTo(const gfx::Vector2d& new_value) override {
EXPECT_EQ(kExpectedScrollOffset.ToString(), new_value.ToString());
EndTest();
}
private:
static constexpr float kDipScale = 2.625f;
static const gfx::Vector2dF kTotalScrollOffset;
static const gfx::Vector2dF kTotalMaxScrollOffset;
static const gfx::SizeF kScrollableSize;
static constexpr float kPageScaleFactor = 1.f;
// These two are not used in this test.
static constexpr float kMinPageScaleFactor = 1.f;
static constexpr float kMaxPageScaleFactor = 5.f;
static const gfx::Vector2d kExpectedScrollOffset;
};
// The current scroll offset in logical pixel, which is at the end.
const gfx::Vector2dF DidReachMaximalScrollOffsetTest::kTotalScrollOffset =
gfx::Vector2dF(0.f, 6132.f);
// The maximum possible scroll offset in logical pixel.
const gfx::Vector2dF DidReachMaximalScrollOffsetTest::kTotalMaxScrollOffset =
gfx::Vector2dF(0.f, 6132.f);
// This is what passed to CTS test, not used for this test.
const gfx::SizeF DidReachMaximalScrollOffsetTest::kScrollableSize =
gfx::SizeF(412.f, 6712.f);
// In max_scroll_offset() we are using ceiling rounding for scaled scroll
// offset. Therefore ceiling(2.625 * 6132 = 16096.5) = 16097.
const gfx::Vector2d DidReachMaximalScrollOffsetTest::kExpectedScrollOffset =
gfx::Vector2d(0, 16097);
RENDERING_TEST_F(DidReachMaximalScrollOffsetTest);
} // namespace android_webview } // namespace android_webview
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment