New issue
Advanced search Search tips

Issue 605124 link

Starred by 1 user
Status: Untriaged
Owner: ----
Cc:
sadrul@chromium.org
sky@chromium.org
osh...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Easy to create an inconsistent display::Display instance

Project Member Reported by sadrul@chromium.org, Apr 20 2016 
Consider the following code:

  int64_t display_id = ...;
  gfx::Rect bounds_in_pixel = ...;
  gfx::Rect work_area_in_pixel = bounds_in_pixel;
  gfx::Display display(display_id, bounds_in_pixel);
  display.set_work_area(work_area_in_pixel);
  EXPECT_EQ(display.work_area(), display.bounds());

In normal cases, this works and runs fine. If, however, the --force-device-scale-factor=2 flag is used, then it fails. The bounds sent when creating the gfx::Display instance is in pixel-space, and when the cmdline flag is used to set the DSF, it automagically converts the bounds to DIP-space. However, the rest of the functions, e.g. set_bounds(), set_work_area() etc. do not do any pixel-to-DIP conversion, which makes it really easy to unknowingly create a gfx::Display instance with incorrect values.

For an example, see https://codereview.chromium.org/1903003002/

It would be nice to change the gfx::Display API so that it's not easy to create an inconsistent gfx::Display instance.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 20 2016 

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

commit 5b115788789b08177b4f38d9a0206a6c069ec4e6
Author: sadrul <sadrul@chromium.org>
Date: Wed Apr 20 16:02:19 2016

mus/views: Fix creating gfx::Display in high-dpi.

It's very easy to create a gfx::Display instance with incorrect values
for bounds/work-area when device-scale factor ≠ 1. This is because
when the gfx::Display is created with a specified bounds, it attempts
to automatically convert the bounds from pixel to DIP values. However,
work-area needs to set separately, and so the caller can use different
scaling factor when setting the bounds vs. setting the work-area. To
fix this, have the gfx::Display creator explicitly do the pixel-to-DIP
conversion before setting the bounds and work-area on the gfx::Display
instances.

BUG= 600815 , 605124

Review URL: https://codereview.chromium.org/1903003002

Cr-Commit-Position: refs/heads/master@{#388507}

[modify] https://crrev.com/5b115788789b08177b4f38d9a0206a6c069ec4e6/ui/aura/test/aura_test_helper.cc
[modify] https://crrev.com/5b115788789b08177b4f38d9a0206a6c069ec4e6/ui/views/mus/BUILD.gn
[modify] https://crrev.com/5b115788789b08177b4f38d9a0206a6c069ec4e6/ui/views/mus/screen_mus.cc
[add] https://crrev.com/5b115788789b08177b4f38d9a0206a6c069ec4e6/ui/views/mus/screen_mus_unittest.cc

Comment 2 by sadrul@chromium.org, May 15 2016

Summary: Easy to create an inconsistent display::Display instance (was: Easy to create an inconsistent gfx::Display instance)
Project Member

Comment 3 by sheriffbot@chromium.org, May 16 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been available for more than 365 days, and should be re-evaluated. Please re-triage this issue.
The Hotlist-Recharge-Cold label is applied for tracking purposes, and should not be removed after re-triaging the issue.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
►

Sign in to add a comment