New issue
Advanced search Search tips

Issue 805425 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug-Regression



Sign in to add a comment

SnapLayerToPhysicalPixelBoundary trigger dcheck if parent has scale transform

Project Member Reported by rog...@vewd.com, Jan 24 2018

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3278.0 Safari/537.36 OPR/51.0.2809.0 (Edition developer)

Steps to reproduce the problem:
In our integration we sometimes apply a scale transform to a ui::Layer, this recently started triggering a dcheck:
FATAL:dip_util.cc(71)] Check failed: diff < kEplison (0.333344 vs. 0.0003)

It turns out that the ui::SnapLayerToPhysicalPixelBoundary does not handle the case when a scale transform is applied.

What is the expected behavior?
No DCHECK and ui::SnapLayerToPhysicalPixelBoundary should snap to physical pixel boundaries even if the "layer to snap" has a parent with a scaling transform applied.

What went wrong?
DCHECK and wrong subpixel offset calculated.

Did this work before? Yes No dcheck trigged before https://chromium-review.googlesource.com/691338 landed, but the offset coordinates were likely wrong.

Chrome version: 66.0.3331.0  Channel: n/a
OS Version: 
Flash Version: 

I will post a CL with a unit test and possible fix soon.
 
Labels: Needs-Triage-M66
Cc: dtapu...@chromium.org
Components: UI>Input
Owner: eirage@chromium.org
Status: Assigned (was: Unconfirmed)
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/41dd59f3e21750566d3acb5f57a17b6e208f6ece

commit 41dd59f3e21750566d3acb5f57a17b6e208f6ece
Author: Roger Johannesson <rogerj@vewd.com>
Date: Thu Jan 25 19:05:06 2018

Take the layer transform scale into account when snapping to physical pixels

SnapLayerToPhysicalPixelBoundary calculates the sub pixel offset that is needed
for a layer to end up on a physical pixel boundary but it does not take into
account that a layer can have a scaling transform applied to it.

This showed up as a problem when https://chromium-review.googlesource.com/691338
landed since it made layers with scale transforms applied start triggering
DCHECKs in CheckSnapped. The reason there was no problem before that CL is that
ConvertPointToLayer used to return floored coordinates.

This patch attempts to fix this problem by applying the transform scale to the
offset vector, similar to how the device scale factor is applied.

Bug:  805425 
Change-Id: I401de72a465b83b00762a92ad2634ac2fc40dff5
Reviewed-on: https://chromium-review.googlesource.com/883782
Commit-Queue: Antoine Labour <piman@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531967}
[modify] https://crrev.com/41dd59f3e21750566d3acb5f57a17b6e208f6ece/ui/compositor/dip_util.cc
[modify] https://crrev.com/41dd59f3e21750566d3acb5f57a17b6e208f6ece/ui/compositor/layer_unittest.cc

Comment 4 by rog...@vewd.com, Jan 26 2018

Status: Fixed (was: Assigned)
I'll consider this issue fixed now with the CL that just landed. So I'm closing this.

Sign in to add a comment