Skip to content

Commit

Permalink
[ozone/wayland] Fixed some issues related to scaling on HiDPI screens.
Browse files Browse the repository at this point in the history
There are two independent scale factors that were not used properly, which
caused incorrect scaling behaviour on HiDPI screens.

The first scale factor is property of the window manager (Wayland); it is
integral and expected to be used as scale of buffers that back the window
surface (see wl_surface_set_buffer_scale).  This value is provided by
the window manager, and neither user nor browser can affect it directly.
I will refer to this value as 'platform scale'.

The second scale factor defines how much the browser UI should be scaled.
This value is real (floating point), and it can be set via the aforementioned
--force-device-scale-factor command line flag.  The default base scale is
equal to the platform scale, which results in UI that looks more or less
the same size as other applications.  This scale is set in display::Display,
and I will refer to it as 'UI scale'.

So here is what this CL fixes.

1. The platform scale had been forwarded to the UI scale with no regard to
   the forcing flag, which caused issue 910797 (the flag did not have any
   effect).
2. The platform scale had not been used to scale the backing buffers, which
   caused the issue 929871 (blurry contents of windows on HDPI screens).
3. The coordinate-aware events (mouse and touch) and window sizing logic
   were not aware of the buffer scaling.
4. Both parameters had been named 'device scale factors' which was confusing.

There are some issues that will be fixed in consequent CLs.  The forced
device scale factor is not propagated properly through display::Display.
For fixing that, https://crbug.com/950313 is filed.

Bug: 910797
Change-Id: I51a95e7218306295e3ab1f90d4b088b8bee93951
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1472617
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Reviewed-by: Maksim Sisov <msisov@igalia.com>
Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Alexander Dunaev <adunaev@igalia.com>
Auto-Submit: Alexander Dunaev <adunaev@igalia.com>
Cr-Commit-Position: refs/heads/master@{#656423}
  • Loading branch information
alex-voodoo authored and Commit Bot committed May 3, 2019
1 parent a1720b5 commit 605d15d
Show file tree
Hide file tree
Showing 39 changed files with 382 additions and 160 deletions.
4 changes: 2 additions & 2 deletions content/browser/renderer_host/browser_compositor_view_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
const viz::LocalSurfaceIdAllocation& child_local_surface_id_allocation) {
if (dfh_local_surface_id_allocator_.UpdateFromChild(
child_local_surface_id_allocation)) {
dfh_display_.set_device_scale_factor(new_device_scale_factor);
dfh_display_.SetDeviceScaleFactor(new_device_scale_factor);
dfh_size_dip_ = gfx::ConvertSizeToDIP(dfh_display_.device_scale_factor(),
new_size_in_pixels);
dfh_size_pixels_ = new_size_in_pixels;
Expand Down Expand Up @@ -402,7 +402,7 @@

bool BrowserCompositorMac::ForceNewSurfaceForTesting() {
display::Display new_display(dfh_display_);
new_display.set_device_scale_factor(new_display.device_scale_factor() * 2.0f);
new_display.SetDeviceScaleFactor(new_display.device_scale_factor() * 2.0f);
return UpdateSurfaceFromNSView(dfh_size_dip_, new_display);
}

Expand Down
4 changes: 2 additions & 2 deletions content/common/cursors/webcursor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ TEST(WebCursorTest, CursorScaleFactor) {
WebCursor cursor(info);

display::Display display;
display.set_device_scale_factor(4.2f);
display.SetDeviceScaleFactor(4.2f);
cursor.SetDisplayInfo(display);

#if defined(USE_OZONE)
Expand Down Expand Up @@ -171,7 +171,7 @@ void ScaleCursor(float scale, int hotspot_x, int hotspot_y) {
WebCursor cursor(info);

display::Display display;
display.set_device_scale_factor(scale);
display.SetDeviceScaleFactor(scale);
cursor.SetDisplayInfo(display);

HCURSOR windows_cursor_handle = cursor.GetNativeCursor().platform();
Expand Down
2 changes: 1 addition & 1 deletion ui/android/display_android_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void DisplayAndroidManager::DoUpdateDisplay(display::Display* display,
int bitsPerComponent,
bool isWideColorGamut) {
if (!Display::HasForceDeviceScaleFactor())
display->set_device_scale_factor(dipScale);
display->SetDeviceScaleFactor(dipScale);
if (!Display::HasForceDisplayColorProfile()) {
if (isWideColorGamut) {
display->set_color_space(gfx::ColorSpace::CreateDisplayP3D65());
Expand Down
33 changes: 13 additions & 20 deletions ui/aura/window_tree_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "ui/compositor/compositor_switches.h"
#include "ui/compositor/dip_util.h"
#include "ui/compositor/layer.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
#include "ui/events/keycodes/dom/dom_code.h"
#include "ui/gfx/geometry/insets.h"
Expand Down Expand Up @@ -108,9 +107,7 @@ WindowTreeHost* WindowTreeHost::GetForAcceleratedWidget(
}

void WindowTreeHost::InitHost() {
display::Display display =
display::Screen::GetScreen()->GetDisplayNearestWindow(window());
device_scale_factor_ = display.device_scale_factor();
device_scale_factor_ = GetDisplay().device_scale_factor();

UpdateRootWindowSizeInPixels();
InitCompositor();
Expand Down Expand Up @@ -278,7 +275,7 @@ ui::EventSink* WindowTreeHost::GetEventSink() {
}

int64_t WindowTreeHost::GetDisplayId() {
return display::Screen::GetScreen()->GetDisplayNearestWindow(window()).id();
return GetDisplay().id();
}

void WindowTreeHost::Show() {
Expand Down Expand Up @@ -351,8 +348,11 @@ WindowTreeHost::WindowTreeHost(std::unique_ptr<Window> window)
if (!window_)
window_ = new Window(nullptr);
display::Screen::GetScreen()->AddObserver(this);
auto display = display::Screen::GetScreen()->GetDisplayNearestWindow(window_);
device_scale_factor_ = display.device_scale_factor();
device_scale_factor_ = GetDisplay().device_scale_factor();
}

display::Display WindowTreeHost::GetDisplay() const {
return display::Screen::GetScreen()->GetDisplayNearestWindow(window_);
}

void WindowTreeHost::IntializeDeviceScaleFactor(float device_scale_factor) {
Expand All @@ -379,7 +379,7 @@ void WindowTreeHost::DestroyDispatcher() {
// ~Window, but by that time any calls to virtual methods overriden here (such
// as GetRootWindow()) result in Window's implementation. By destroying here
// we ensure GetRootWindow() still returns this.
//window()->RemoveOrDestroyChildren();
// window()->RemoveOrDestroyChildren();
}

void WindowTreeHost::CreateCompositor(
Expand Down Expand Up @@ -420,8 +420,7 @@ void WindowTreeHost::InitCompositor() {
window()->GetLocalSurfaceIdAllocation());
compositor_->SetRootLayer(window()->layer());

display::Display display =
display::Screen::GetScreen()->GetDisplayNearestWindow(window());
display::Display display = GetDisplay();
compositor_->SetDisplayColorSpace(display.color_space(),
display.sdr_white_level());
}
Expand All @@ -446,9 +445,7 @@ void WindowTreeHost::OnHostResizedInPixels(
const viz::LocalSurfaceIdAllocation& new_local_surface_id_allocation) {
// TODO(jonross) Unify all OnHostResizedInPixels to have both
// viz::LocalSurfaceId and allocation time as optional parameters.
display::Display display =
display::Screen::GetScreen()->GetDisplayNearestWindow(window());
device_scale_factor_ = display.device_scale_factor();
device_scale_factor_ = GetDisplay().device_scale_factor();
UpdateRootWindowSizeInPixels();

// Allocate a new LocalSurfaceId for the new state.
Expand Down Expand Up @@ -479,8 +476,7 @@ void WindowTreeHost::OnHostWorkspaceChanged() {
void WindowTreeHost::OnHostDisplayChanged() {
if (!compositor_)
return;
display::Display display =
display::Screen::GetScreen()->GetDisplayNearestWindow(window());
display::Display display = GetDisplay();
compositor_->SetDisplayColorSpace(display.color_space(),
display.sdr_white_level());
}
Expand Down Expand Up @@ -535,11 +531,8 @@ void WindowTreeHost::MoveCursorToInternal(const gfx::Point& root_location,
last_cursor_request_position_in_host_ = host_location;
MoveCursorToScreenLocationInPixels(host_location);
client::CursorClient* cursor_client = client::GetCursorClient(window());
if (cursor_client) {
const display::Display& display =
display::Screen::GetScreen()->GetDisplayNearestWindow(window());
cursor_client->SetDisplay(display);
}
if (cursor_client)
cursor_client->SetDisplay(GetDisplay());
dispatcher()->OnCursorMovedToRootLocation(root_location);
}

Expand Down
8 changes: 6 additions & 2 deletions ui/aura/window_tree_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "ui/base/cursor/cursor.h"
#include "ui/base/ime/input_method_delegate.h"
#include "ui/compositor/compositor_observer.h"
#include "ui/display/display.h"
#include "ui/display/display_observer.h"
#include "ui/events/event_source.h"
#include "ui/events/platform_event.h"
Expand All @@ -34,7 +35,7 @@ class Point;
class Rect;
class Size;
class Transform;
}
} // namespace gfx

namespace ui {
class Compositor;
Expand All @@ -43,7 +44,7 @@ class EventSink;
class InputMethod;
class ViewProp;
struct PlatformWindowInitProperties;
}
} // namespace ui

namespace aura {

Expand Down Expand Up @@ -256,6 +257,9 @@ class AURA_EXPORT WindowTreeHost : public ui::internal::InputMethodDelegate,

explicit WindowTreeHost(std::unique_ptr<Window> window = nullptr);

// Gets the display that this window tree host resides at.
display::Display GetDisplay() const;

// Set the cached display device scale factor. This should only be called
// during subclass initialization, when the value is needed before InitHost().
void IntializeDeviceScaleFactor(float device_scale_factor);
Expand Down
3 changes: 1 addition & 2 deletions ui/aura/window_tree_host_platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,6 @@ void WindowTreeHostPlatform::OnAcceleratedWidgetDestroyed() {
widget_ = gfx::kNullAcceleratedWidget;
}

void WindowTreeHostPlatform::OnActivationChanged(bool active) {
}
void WindowTreeHostPlatform::OnActivationChanged(bool active) {}

} // namespace aura
6 changes: 6 additions & 0 deletions ui/display/display.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,12 @@ Display Display::GetDefaultDisplay() {
return Display(kDefaultDisplayId, gfx::Rect(0, 0, 1920, 1080));
}

void Display::SetDeviceScaleFactor(float scale) {
if (HasForceDeviceScaleFactor())
return;
device_scale_factor_ = scale;
}

int Display::RotationAsDegree() const {
switch (rotation_) {
case ROTATE_0:
Expand Down
18 changes: 8 additions & 10 deletions ui/display/display.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,12 @@ class DISPLAY_EXPORT Display final {
const gfx::Rect& work_area() const { return work_area_; }
void set_work_area(const gfx::Rect& work_area) { work_area_ = work_area; }

// Output device's pixel scale factor. This specifies how much the
// UI should be scaled when the actual output has more pixels than
// standard displays (which is around 100~120dpi.) The potential return
// values depend on each platforms.
// Pixel scale factor. This specifies how much the UI should be scaled when
// rendering on the actual output. This is needed when the latter has more
// pixels than standard displays (which is around 100~120dpi). The potential
// return values depend on each platforms.
float device_scale_factor() const { return device_scale_factor_; }
void set_device_scale_factor(float scale) { device_scale_factor_ = scale; }
void SetDeviceScaleFactor(float scale);

Rotation rotation() const { return rotation_; }
void set_rotation(Rotation rotation) { rotation_ = rotation; }
Expand All @@ -178,7 +178,7 @@ class DISPLAY_EXPORT Display final {
gfx::Insets GetWorkAreaInsets() const;

// Sets the device scale factor and display bounds in pixel. This
// updates the work are using the same insets between old bounds and
// updates the work area using the same insets between old bounds and
// work area.
void SetScaleAndBounds(float device_scale_factor,
const gfx::Rect& bounds_in_pixel);
Expand All @@ -187,7 +187,7 @@ class DISPLAY_EXPORT Display final {
// between old bounds and work area.
void SetSize(const gfx::Size& size_in_pixel);

// Computes and updates the display's work are using
// Computes and updates the display's work area using
// |work_area_insets| and the bounds.
void UpdateWorkAreaFromInsets(const gfx::Insets& work_area_insets);

Expand Down Expand Up @@ -239,9 +239,7 @@ class DISPLAY_EXPORT Display final {

// The number of bits per pixel. Used by media query APIs.
int color_depth() const { return color_depth_; }
void set_color_depth(int color_depth) {
color_depth_ = color_depth;
}
void set_color_depth(int color_depth) { color_depth_ = color_depth; }

// The number of bits per color component (all color components are assumed to
// have the same number of bits). Used by media query APIs.
Expand Down
12 changes: 6 additions & 6 deletions ui/display/display_change_notifier_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,9 @@ TEST(DisplayChangeNotifierTest, NotifyDisplaysChanged_Changed_DSF) {

std::vector<Display> old_displays, new_displays;
old_displays.push_back(Display(1));
old_displays[0].set_device_scale_factor(1.f);
old_displays[0].SetDeviceScaleFactor(1.f);
new_displays.push_back(Display(1));
new_displays[0].set_device_scale_factor(2.f);
new_displays[0].SetDeviceScaleFactor(2.f);

change_notifier.NotifyDisplaysChanged(old_displays, new_displays);
EXPECT_EQ(1, observer.display_changed());
Expand All @@ -436,8 +436,8 @@ TEST(DisplayChangeNotifierTest, NotifyDisplaysChanged_Changed_Multi_Displays) {
new_displays.push_back(Display(2));
new_displays.push_back(Display(3));

old_displays[0].set_device_scale_factor(1.f);
new_displays[0].set_device_scale_factor(2.f);
old_displays[0].SetDeviceScaleFactor(1.f);
new_displays[0].SetDeviceScaleFactor(2.f);

old_displays[1].set_bounds(gfx::Rect(0, 0, 200, 200));
new_displays[1].set_bounds(gfx::Rect(0, 0, 400, 400));
Expand All @@ -456,11 +456,11 @@ TEST(DisplayChangeNotifierTest, NotifyDisplaysChanged_Changed_Multi_Metrics) {

std::vector<Display> old_displays, new_displays;
old_displays.push_back(Display(1, gfx::Rect(0, 0, 200, 200)));
old_displays[0].set_device_scale_factor(1.f);
old_displays[0].SetDeviceScaleFactor(1.f);
old_displays[0].SetRotationAsDegree(0);

new_displays.push_back(Display(1, gfx::Rect(100, 100, 200, 200)));
new_displays[0].set_device_scale_factor(2.f);
new_displays[0].SetDeviceScaleFactor(2.f);
new_displays[0].SetRotationAsDegree(90);

change_notifier.NotifyDisplaysChanged(old_displays, new_displays);
Expand Down
2 changes: 1 addition & 1 deletion ui/display/display_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ uint32_t DisplayList::UpdateDisplay(const Display& display, Type type) {
changed_values |= DisplayObserver::DISPLAY_METRIC_ROTATION;
}
if (local_display->device_scale_factor() != display.device_scale_factor()) {
local_display->set_device_scale_factor(display.device_scale_factor());
local_display->SetDeviceScaleFactor(display.device_scale_factor());
changed_values |= DisplayObserver::DISPLAY_METRIC_DEVICE_SCALE_FACTOR;
}
if (local_display->color_space() != display.color_space() ||
Expand Down
2 changes: 1 addition & 1 deletion ui/display/ios/screen_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
UIScreen* mainScreen = [UIScreen mainScreen];
CHECK(mainScreen);
Display display(0, gfx::Rect(mainScreen.bounds));
display.set_device_scale_factor([mainScreen scale]);
display.SetDeviceScaleFactor([mainScreen scale]);
ProcessDisplayChanged(display, true /* is_primary */);
}

Expand Down
2 changes: 1 addition & 1 deletion ui/display/mac/screen_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ Display BuildDisplayForScreen(NSScreen* screen) {
CGFloat scale = [screen backingScaleFactor];
if (Display::HasForceDeviceScaleFactor())
scale = Display::GetForcedDeviceScaleFactor();
display.set_device_scale_factor(scale);
display.SetDeviceScaleFactor(scale);

// Compute the color profile.
gfx::ICCProfile icc_profile;
Expand Down
2 changes: 1 addition & 1 deletion ui/display/mojo/display_struct_traits.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ bool StructTraits<display::mojom::DisplayDataView, display::Display>::Read(
if (!data.ReadWorkArea(&out->work_area_))
return false;

out->set_device_scale_factor(data.device_scale_factor());
out->SetDeviceScaleFactor(data.device_scale_factor());

if (!data.ReadRotation(&out->rotation_))
return false;
Expand Down
2 changes: 1 addition & 1 deletion ui/display/mojo/display_struct_traits_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ TEST(DisplayStructTraitsTest, SetAllDisplayValues) {

Display input(246345234, bounds);
input.set_work_area(work_area);
input.set_device_scale_factor(2.0f);
input.SetDeviceScaleFactor(2.0f);
input.set_rotation(Display::ROTATE_270);
input.set_touch_support(Display::TouchSupport::AVAILABLE);
input.set_accelerometer_support(Display::AccelerometerSupport::UNAVAILABLE);
Expand Down
2 changes: 1 addition & 1 deletion ui/display/win/screen_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ Display CreateDisplayFromDisplayInfo(const DisplayInfo& display_info,
bool hdr_enabled) {
Display display(display_info.id());
float scale_factor = display_info.device_scale_factor();
display.set_device_scale_factor(scale_factor);
display.SetDeviceScaleFactor(scale_factor);
display.set_work_area(
gfx::ScaleToEnclosingRect(display_info.screen_work_rect(),
1.0f / scale_factor));
Expand Down
2 changes: 1 addition & 1 deletion ui/display/win/screen_win_display.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace {
Display CreateDisplayFromDisplayInfo(const DisplayInfo& display_info) {
Display display(display_info.id());
float scale_factor = display_info.device_scale_factor();
display.set_device_scale_factor(scale_factor);
display.SetDeviceScaleFactor(scale_factor);
display.set_work_area(
gfx::ScaleToEnclosingRect(display_info.screen_work_rect(),
1.0f / scale_factor));
Expand Down
2 changes: 1 addition & 1 deletion ui/ozone/platform/scenic/scenic_screen.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void ScenicScreen::OnWindowMetrics(int32_t window_id,
});
DCHECK(display_it != displays_.end());

display_it->set_device_scale_factor(device_pixel_ratio);
display_it->SetDeviceScaleFactor(device_pixel_ratio);
for (auto& observer : observers_) {
observer.OnDisplayMetricsChanged(
*display_it,
Expand Down
10 changes: 10 additions & 0 deletions ui/ozone/platform/wayland/host/wayland_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,16 @@ WaylandWindow* WaylandConnection::GetCurrentKeyboardFocusedWindow() const {
return nullptr;
}

std::vector<WaylandWindow*> WaylandConnection::GetWindowsOnDisplay(
uint32_t display_id) {
std::vector<WaylandWindow*> result;
for (auto entry : window_map_) {
if (entry.second->GetEnteredOutputsIds().count(display_id) > 0)
result.push_back(entry.second);
}
return result;
}

void WaylandConnection::AddWindow(gfx::AcceleratedWidget widget,
WaylandWindow* window) {
window_map_[widget] = window;
Expand Down
5 changes: 4 additions & 1 deletion ui/ozone/platform/wayland/host/wayland_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ class WaylandConnection : public PlatformEventSource,
WaylandWindow* GetWindowWithLargestBounds() const;
WaylandWindow* GetCurrentFocusedWindow() const;
WaylandWindow* GetCurrentKeyboardFocusedWindow() const;
// TODO(adunaev) remove this in favor of targeted subscription of windows to
// their displays.
std::vector<WaylandWindow*> GetWindowsOnDisplay(uint32_t display_id);
void AddWindow(gfx::AcceleratedWidget widget, WaylandWindow* window);
void RemoveWindow(gfx::AcceleratedWidget widget);

Expand Down Expand Up @@ -229,7 +232,7 @@ class WaylandConnection : public PlatformEventSource,
// xdg_shell_listener
static void Ping(void* data, xdg_shell* shell, uint32_t serial);

std::map<gfx::AcceleratedWidget, WaylandWindow*> window_map_;
base::flat_map<gfx::AcceleratedWidget, WaylandWindow*> window_map_;

wl::Object<wl_display> display_;
wl::Object<wl_registry> registry_;
Expand Down
Loading

0 comments on commit 605d15d

Please sign in to comment.