Skip to content

Commit

Permalink
Use the host's scale factor if the client's scale factor is close.
Browse files Browse the repository at this point in the history 
A client is expected to send the same value as exo uses. However this
is not guaranteed because walyand doesn't support float as data type,
and existing impl uses logical size + pixel size to compute the
scale factor, which can have an error. (and this becomes bigger when
display zoom is applied).

We'll work on a new extension protocol to fix this (1412420), and
meanwhile, this CL will mitigate the issue by using the same scale
factor as host when the value is close.

This also updates the logic to compute the scale factor + zoom factor
so that the logical size x scale factor will produce near integer
size.

Bug: 1411793, 1412420
Test: DisplayResolutionTest.DisplayZoom

Change-Id: I73ffb1712bb51a4fc5a27bf0c5609e87a781dc98
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4210142
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1101154}
  • Loading branch information
@mitoshima
mitoshima authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent 62b6819 commit 4f70de1
Show file tree
Hide file tree
Showing 10 changed files with 422 additions and 204 deletions.
2 changes: 1 addition & 1 deletion ash/capture_mode/capture_mode_demo_tools_unittests.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ TEST_P(CaptureModeDemoToolsTestWithAllSources, DeviceScaleFactorTest) {
const float kDeviceScaleFactors[] = {0.5f, 1.2f, 2.5f};
for (const float dsf : kDeviceScaleFactors) {
SetDeviceScaleFactor(dsf);
EXPECT_EQ(dsf, window()->GetHost()->device_scale_factor());
EXPECT_NEAR(dsf, window()->GetHost()->device_scale_factor(), 0.01);
VerifyKeyComboWidgetPosition();
}
}
Expand Down
4 changes: 2 additions & 2 deletions ash/display/display_manager_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2202,12 +2202,12 @@ TEST_F(DisplayManagerTest, UpdateMouseCursorAfterRotateZoom) {
generator2.MoveMouseToInHost(200, 250);
EXPECT_EQ(gfx::Point(700, 125), env->last_mouse_location());
UpdateDisplay("600x400,400x300*2@1.5");
EXPECT_EQ(gfx::Point(666, 83), env->last_mouse_location());
EXPECT_EQ(gfx::Point(666, 84), env->last_mouse_location());

// The native location is now outside, so move to the
// center of closest display.
UpdateDisplay("600x400,400x200*2@1.5");
EXPECT_EQ(gfx::Point(666, 67), env->last_mouse_location());
EXPECT_EQ(gfx::Point(665, 66), env->last_mouse_location());
}

class TestDisplayObserver : public display::DisplayObserver {
Expand Down
11 changes: 10 additions & 1 deletion components/exo/surface_tree_host.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "ui/compositor/layer.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
#include "ui/display/types/display_constants.h"
#include "ui/gfx/color_space.h"
#include "ui/gfx/display_color_spaces.h"
#include "ui/gfx/geometry/dip_util.h"
Expand Down Expand Up @@ -453,8 +454,16 @@ void SurfaceTreeHost::HandleContextLost() {
}

float SurfaceTreeHost::GetScaleFactor() {
if (scale_factor_)
if (scale_factor_) {
// TODO(crbug.com/1412420): Remove this once the scale factor precision
// issue is fixed.
if (std::abs(scale_factor_.value() -
host_window_->layer()->device_scale_factor()) <
display::kDeviceScaleFactorErrorTolerance) {
return host_window_->layer()->device_scale_factor();
}
return scale_factor_.value();
}
return host_window_->layer()->device_scale_factor();
}

Expand Down
264 changes: 176 additions & 88 deletions tools/metrics/histograms/enums.xml
View file

Large diffs are not rendered by default.

166 changes: 88 additions & 78 deletions ui/display/manager/display_change_observer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ void DisplayChangeObserver::OnDisplayModeChanged(
if (!mode_info)
continue;

displays.emplace_back(CreateManagedDisplayInfo(state, mode_info));
displays.emplace_back(CreateManagedDisplayInfoInternal(state, mode_info));
}

display_manager_->touch_device_manager()->AssociateTouchscreens(
Expand Down Expand Up @@ -246,46 +246,41 @@ void DisplayChangeObserver::OnInputDeviceConfigurationChanged(
}
}

void DisplayChangeObserver::UpdateInternalDisplay(
const DisplayConfigurator::DisplayStateList& display_states) {
bool force_first_display_internal = ForceFirstDisplayInternal();

for (auto* state : display_states) {
if (state->type() == DISPLAY_CONNECTION_TYPE_INTERNAL ||
(force_first_display_internal &&
(!HasInternalDisplay() || IsInternalDisplayId(state->display_id())))) {
if (HasInternalDisplay())
DCHECK_EQ(Display::InternalDisplayId(), state->display_id());
SetInternalDisplayIds({state->display_id()});
// static
float DisplayChangeObserver::FindDeviceScaleFactor(
float dpi,
const gfx::Size& size_in_pixels) {
// Nocturne has special scale factor 3000/1332=2.252.. for the panel 3kx2k.
constexpr gfx::Size k225DisplaySizeHackNocturne(3000, 2000);
// Keep the Chell's scale factor 2.252 until we make decision.
constexpr gfx::Size k2DisplaySizeHackChell(3200, 1800);
constexpr gfx::Size k18DisplaySizeHackCoachZ(2160, 1440);

if (state->native_mode() &&
(!display_manager_->IsDisplayIdValid(state->display_id()) ||
!state->current_mode())) {
// Register the internal display info if
// 1) If it's not already registered. It'll be treated as
// new display in |UpdateDisplaysWith()|.
// 2) If it's not connected, because the display info will not
// be updated in |UpdateDisplaysWith()|, which will skips the
// disconnected displays.
ManagedDisplayInfo new_info =
CreateManagedDisplayInfo(state, state->native_mode());
display_manager_->UpdateInternalDisplay(new_info);
}
return;
if (size_in_pixels == k225DisplaySizeHackNocturne) {
return kDsf_2_252;
}
if (size_in_pixels == k2DisplaySizeHackChell) {
return 2.f;
}
if (size_in_pixels == k18DisplaySizeHackCoachZ) {
return kDsf_1_8;
}
for (size_t i = 0; i < std::size(kThresholdTableForInternal); ++i) {
if (dpi >= kThresholdTableForInternal[i].dpi) {
return kThresholdTableForInternal[i].device_scale_factor;
}
}
return 1.0f;
}

// static
ManagedDisplayInfo DisplayChangeObserver::CreateManagedDisplayInfo(
const DisplaySnapshot* snapshot,
const DisplayMode* mode_info) {
std::string name = (snapshot->type() == DISPLAY_CONNECTION_TYPE_INTERNAL)
? l10n_util::GetStringUTF8(IDS_DISPLAY_NAME_INTERNAL)
: snapshot->display_name();

if (name.empty())
name = l10n_util::GetStringUTF8(IDS_DISPLAY_NAME_UNKNOWN);

const DisplayMode* mode_info,
bool native,
float device_scale_factor,
float dpi,
const std::string& name) {
const bool has_overscan = snapshot->has_overscan();
const int64_t id = snapshot->display_id();

Expand All @@ -310,23 +305,7 @@ ManagedDisplayInfo DisplayChangeObserver::CreateManagedDisplayInfo(
new_info.set_sys_path(snapshot->sys_path());
new_info.set_from_native_platform(true);

float device_scale_factor = 1.0f;
// Sets dpi only if the screen size is valid.
const float dpi = IsDisplaySizeValid(snapshot->physical_size())
? kInchInMm * mode_info->size().width() /
snapshot->physical_size().width()
: 0;
if (snapshot->type() == DISPLAY_CONNECTION_TYPE_INTERNAL) {
new_info.set_native(true);
device_scale_factor = FindDeviceScaleFactor(dpi, mode_info->size());
} else {
ManagedDisplayMode mode;
if (display_manager_->GetSelectedModeForDisplayId(snapshot->display_id(),
&mode)) {
device_scale_factor = mode.device_scale_factor();
new_info.set_native(mode.native());
}
}
new_info.set_native(native);
new_info.set_device_scale_factor(device_scale_factor);

const gfx::Rect display_bounds(snapshot->origin(), mode_info->size());
Expand All @@ -336,13 +315,6 @@ ManagedDisplayInfo DisplayChangeObserver::CreateManagedDisplayInfo(
if (dpi)
new_info.set_device_dpi(dpi);

#if !BUILDFLAG(IS_CHROMEOS_ASH)
// TODO(crbug.com/1012846): This should configure the HDR color spaces.
gfx::DisplayColorSpaces display_color_spaces(
snapshot->color_space(), DisplaySnapshot::PrimaryFormat());
new_info.set_display_color_spaces(display_color_spaces);
new_info.set_bits_per_channel(snapshot->bits_per_channel());
#else
// TODO(crbug.com/1012846): Remove kEnableUseHDRTransferFunction usage when
// HDR is fully supported on ChromeOS.
const bool allow_high_bit_depth =
Expand All @@ -354,7 +326,6 @@ ManagedDisplayInfo DisplayChangeObserver::CreateManagedDisplayInfo(
constexpr int32_t kNormalBitDepth = 8;
new_info.set_bits_per_channel(
allow_high_bit_depth ? snapshot->bits_per_channel() : kNormalBitDepth);
#endif

new_info.set_refresh_rate(mode_info->refresh_rate());
new_info.set_is_interlaced(mode_info->is_interlaced());
Expand All @@ -378,29 +349,68 @@ ManagedDisplayInfo DisplayChangeObserver::CreateManagedDisplayInfo(
return new_info;
}

// static
float DisplayChangeObserver::FindDeviceScaleFactor(
float dpi,
const gfx::Size& size_in_pixels) {
// Nocturne has special scale factor 3000/1332=2.252.. for the panel 3kx2k.
constexpr gfx::Size k225DisplaySizeHackNocturne(3000, 2000);
// Keep the Chell's scale factor 2.252 until we make decision.
constexpr gfx::Size k2DisplaySizeHackChell(3200, 1800);
constexpr gfx::Size k18DisplaySizeHackCoachZ(2160, 1440);
void DisplayChangeObserver::UpdateInternalDisplay(
const DisplayConfigurator::DisplayStateList& display_states) {
bool force_first_display_internal = ForceFirstDisplayInternal();

if (size_in_pixels == k225DisplaySizeHackNocturne) {
return kDsf_2_252;
} else if (size_in_pixels == k2DisplaySizeHackChell) {
return 2.f;
} else if (size_in_pixels == k18DisplaySizeHackCoachZ) {
return kDsf_1_8;
for (auto* state : display_states) {
if (state->type() == DISPLAY_CONNECTION_TYPE_INTERNAL ||
(force_first_display_internal &&
(!HasInternalDisplay() || IsInternalDisplayId(state->display_id())))) {
if (HasInternalDisplay()) {
DCHECK_EQ(Display::InternalDisplayId(), state->display_id());
}
SetInternalDisplayIds({state->display_id()});

if (state->native_mode() &&
(!display_manager_->IsDisplayIdValid(state->display_id()) ||
!state->current_mode())) {
// Register the internal display info if
// 1) If it's not already registered. It'll be treated as
// new display in |UpdateDisplaysWith()|.
// 2) If it's not connected, because the display info will not
// be updated in |UpdateDisplaysWith()|, which will skips the
// disconnected displays.
ManagedDisplayInfo new_info =
CreateManagedDisplayInfoInternal(state, state->native_mode());
display_manager_->UpdateInternalDisplay(new_info);
}
return;
}
}
}

ManagedDisplayInfo DisplayChangeObserver::CreateManagedDisplayInfoInternal(
const DisplaySnapshot* snapshot,
const DisplayMode* mode_info) {
bool native = false;
float device_scale_factor = 1.0f;
// Sets dpi only if the screen size is valid.
const float dpi = IsDisplaySizeValid(snapshot->physical_size())
? kInchInMm * mode_info->size().width() /
snapshot->physical_size().width()
: 0;
if (snapshot->type() == DISPLAY_CONNECTION_TYPE_INTERNAL) {
native = true;
device_scale_factor = FindDeviceScaleFactor(dpi, mode_info->size());
} else {
for (size_t i = 0; i < std::size(kThresholdTableForInternal); ++i) {
if (dpi >= kThresholdTableForInternal[i].dpi)
return kThresholdTableForInternal[i].device_scale_factor;
ManagedDisplayMode mode;
if (display_manager_->GetSelectedModeForDisplayId(snapshot->display_id(),
&mode)) {
device_scale_factor = mode.device_scale_factor();
native = mode.native();
}
}
return 1.0f;
std::string name = (snapshot->type() == DISPLAY_CONNECTION_TYPE_INTERNAL)
? l10n_util::GetStringUTF8(IDS_DISPLAY_NAME_INTERNAL)
: snapshot->display_name();

if (name.empty()) {
name = l10n_util::GetStringUTF8(IDS_DISPLAY_NAME_UNKNOWN);
}

return CreateManagedDisplayInfo(snapshot, mode_info, native,
device_scale_factor, dpi, name);
}

} // namespace display
15 changes: 12 additions & 3 deletions ui/display/manager/display_change_observer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,28 @@ class DISPLAY_MANAGER_EXPORT DisplayChangeObserver
// Overriden from ui::InputDeviceEventObserver:
void OnInputDeviceConfigurationChanged(uint8_t input_device_types) override;

// Exposed for testing.
// Static methods exposed for testing.
DISPLAY_EXPORT static float FindDeviceScaleFactor(
float dpi,
const gfx::Size& size_in_pixels);

static ManagedDisplayInfo CreateManagedDisplayInfo(
const DisplaySnapshot* snapshot,
const DisplayMode* mode_info,
bool native,
float device_scale_factor,
float dpi,
const std::string& name);

private:
friend class DisplayChangeObserverTest;

void UpdateInternalDisplay(
const DisplayConfigurator::DisplayStateList& display_states);

ManagedDisplayInfo CreateManagedDisplayInfo(const DisplaySnapshot* snapshot,
const DisplayMode* mode_info);
ManagedDisplayInfo CreateManagedDisplayInfoInternal(
const DisplaySnapshot* snapshot,
const DisplayMode* mode_info);

// |display_manager_| is not owned and must outlive DisplayChangeObserver.
DisplayManager* display_manager_;
Expand Down

0 comments on commit 4f70de1

Please sign in to comment.