Commit f6880b2e authored by danakj's avatar danakj Committed by Commit Bot

Remove ConvertSizeToDIP() methods.

The Convert*ToDIP() and Convert*ToPixel() functions are problematic in
that they hide what decision is being taken in the underlying code
when moving from floating point to integer, and consumers should be
thinking about what they want for correctness. They also act in
inconsistent ways when converting a Rect vs its components. This
removes the functions that perform float->int conversions in order to
have callers show what they intend to happen and make it clear that
data is being lost in some fashion.

R=sky@chromium.org

Bug: 1130050
Change-Id: I21eb1070750c92f59e89f288f63e385570dc966c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419621
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809846}
parent 256b7f7c
...@@ -40,6 +40,7 @@ ...@@ -40,6 +40,7 @@
#include "ui/display/screen.h" #include "ui/display/screen.h"
#include "ui/events/cocoa/cocoa_event_utils.h" #include "ui/events/cocoa/cocoa_event_utils.h"
#include "ui/gfx/geometry/dip_util.h" #include "ui/gfx/geometry/dip_util.h"
#include "ui/gfx/geometry/size_conversions.h"
#import "ui/gfx/mac/coordinate_conversion.h" #import "ui/gfx/mac/coordinate_conversion.h"
#import "ui/gfx/mac/nswindow_frame_controls.h" #import "ui/gfx/mac/nswindow_frame_controls.h"
...@@ -1226,8 +1227,9 @@ void NativeWidgetNSWindowBridge::SetCALayerParams( ...@@ -1226,8 +1227,9 @@ void NativeWidgetNSWindowBridge::SetCALayerParams(
const gfx::CALayerParams& ca_layer_params) { const gfx::CALayerParams& ca_layer_params) {
// Ignore frames arriving "late" for an old size. A frame at the new size // Ignore frames arriving "late" for an old size. A frame at the new size
// should arrive soon. // should arrive soon.
gfx::Size frame_dip_size = gfx::ConvertSizeToDIP(ca_layer_params.scale_factor, // TODO(danakj): We should avoid lossy conversions to integer DIPs.
ca_layer_params.pixel_size); gfx::Size frame_dip_size = gfx::ToFlooredSize(gfx::ConvertSizeToDips(
ca_layer_params.pixel_size, ca_layer_params.scale_factor));
if (content_dip_size_ != frame_dip_size) if (content_dip_size_ != frame_dip_size)
return; return;
compositor_frame_dip_size_ = frame_dip_size; compositor_frame_dip_size_ = frame_dip_size;
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "ui/base/layout.h" #include "ui/base/layout.h"
#include "ui/gfx/geometry/dip_util.h" #include "ui/gfx/geometry/dip_util.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
#include "ui/gfx/geometry/size_conversions.h"
#include "ui/gfx/native_widget_types.h" #include "ui/gfx/native_widget_types.h"
namespace content { namespace content {
...@@ -84,8 +85,9 @@ class WebContentsVideoCaptureDevice::FrameTracker ...@@ -84,8 +85,9 @@ class WebContentsVideoCaptureDevice::FrameTracker
// that has a different device scale factor while being captured. // that has a different device scale factor while being captured.
gfx::Size preferred_size; gfx::Size preferred_size;
if (auto* view = GetCurrentView()) { if (auto* view = GetCurrentView()) {
preferred_size = // TODO(danakj): Should this be rounded?
gfx::ConvertSizeToDIP(view->GetDeviceScaleFactor(), capture_size); preferred_size = gfx::ToFlooredSize(
gfx::ConvertSizeToDips(capture_size, view->GetDeviceScaleFactor()));
} }
if (preferred_size.IsEmpty()) { if (preferred_size.IsEmpty()) {
preferred_size = capture_size; preferred_size = capture_size;
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "ui/compositor/recyclable_compositor_mac.h" #include "ui/compositor/recyclable_compositor_mac.h"
#include "ui/display/screen.h" #include "ui/display/screen.h"
#include "ui/gfx/geometry/dip_util.h" #include "ui/gfx/geometry/dip_util.h"
#include "ui/gfx/geometry/size_conversions.h"
namespace content { namespace content {
...@@ -158,8 +159,9 @@ void BrowserCompositorMac::UpdateSurfaceFromChild( ...@@ -158,8 +159,9 @@ void BrowserCompositorMac::UpdateSurfaceFromChild(
child_local_surface_id_allocation)) { child_local_surface_id_allocation)) {
if (auto_resize_enabled) { if (auto_resize_enabled) {
dfh_display_.set_device_scale_factor(new_device_scale_factor); dfh_display_.set_device_scale_factor(new_device_scale_factor);
dfh_size_dip_ = gfx::ConvertSizeToDIP(dfh_display_.device_scale_factor(), // TODO(danakj): We should avoid lossy conversions to integer DIPs.
new_size_in_pixels); dfh_size_dip_ = gfx::ToFlooredSize(gfx::ConvertSizeToDips(
new_size_in_pixels, dfh_display_.device_scale_factor()));
dfh_size_pixels_ = new_size_in_pixels; dfh_size_pixels_ = new_size_in_pixels;
root_layer_->SetBounds(gfx::Rect(dfh_size_dip_)); root_layer_->SetBounds(gfx::Rect(dfh_size_dip_));
if (recyclable_compositor_) { if (recyclable_compositor_) {
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/lazy_instance.h" #include "base/lazy_instance.h"
#include "base/mac/mac_util.h" #include "base/mac/mac_util.h"
#include "ui/gfx/geometry/dip_util.h" #include "ui/gfx/geometry/dip_util.h"
#include "ui/gfx/geometry/size_conversions.h"
namespace ui { namespace ui {
namespace { namespace {
...@@ -56,8 +57,9 @@ bool AcceleratedWidgetMac::HasFrameOfSize( ...@@ -56,8 +57,9 @@ bool AcceleratedWidgetMac::HasFrameOfSize(
const gfx::Size& dip_size) const { const gfx::Size& dip_size) const {
if (!last_ca_layer_params_valid_) if (!last_ca_layer_params_valid_)
return false; return false;
gfx::Size last_swap_size_dip = gfx::ConvertSizeToDIP( // TODO(danakj): We should avoid lossy conversions to integer DIPs.
last_ca_layer_params_.scale_factor, last_ca_layer_params_.pixel_size); gfx::Size last_swap_size_dip = gfx::ToFlooredSize(gfx::ConvertSizeToDips(
last_ca_layer_params_.pixel_size, last_ca_layer_params_.scale_factor));
return last_swap_size_dip == dip_size; return last_swap_size_dip == dip_size;
} }
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "ui/base/cocoa/animation_utils.h" #include "ui/base/cocoa/animation_utils.h"
#include "ui/base/cocoa/remote_layer_api.h" #include "ui/base/cocoa/remote_layer_api.h"
#include "ui/gfx/geometry/dip_util.h" #include "ui/gfx/geometry/dip_util.h"
#include "ui/gfx/geometry/size_conversions.h"
@interface CALayer (PrivateAPI) @interface CALayer (PrivateAPI)
- (void)setContentsChanged; - (void)setContentsChanged;
...@@ -144,7 +145,10 @@ void DisplayCALayerTree::GotIOSurfaceFrame( ...@@ -144,7 +145,10 @@ void DisplayCALayerTree::GotIOSurfaceFrame(
[io_surface_layer_ setContentsChanged]; [io_surface_layer_ setContentsChanged];
else else
[io_surface_layer_ setContents:new_contents]; [io_surface_layer_ setContents:new_contents];
gfx::Size bounds_dip = gfx::ConvertSizeToDIP(scale_factor, pixel_size); // TODO(danakj): We should avoid lossy conversions to integer DIPs. The OS
// wants a floating point value.
gfx::Size bounds_dip =
gfx::ToFlooredSize(gfx::ConvertSizeToDips(pixel_size, scale_factor));
[io_surface_layer_ [io_surface_layer_
setBounds:CGRectMake(0, 0, bounds_dip.width(), bounds_dip.height())]; setBounds:CGRectMake(0, 0, bounds_dip.width(), bounds_dip.height())];
if ([io_surface_layer_ contentsScale] != scale_factor) if ([io_surface_layer_ contentsScale] != scale_factor)
......
...@@ -23,11 +23,6 @@ ...@@ -23,11 +23,6 @@
namespace ui { namespace ui {
gfx::Size ConvertSizeToDIP(const Layer* layer,
const gfx::Size& size_in_pixel) {
return gfx::ConvertSizeToDIP(layer->device_scale_factor(), size_in_pixel);
}
gfx::Rect ConvertRectToDIP(const Layer* layer, gfx::Rect ConvertRectToDIP(const Layer* layer,
const gfx::Rect& rect_in_pixel) { const gfx::Rect& rect_in_pixel) {
return gfx::ConvertRectToDIP(layer->device_scale_factor(), rect_in_pixel); return gfx::ConvertRectToDIP(layer->device_scale_factor(), rect_in_pixel);
......
...@@ -18,9 +18,6 @@ class Layer; ...@@ -18,9 +18,6 @@ class Layer;
// Utility functions that convert point/size/rect between // Utility functions that convert point/size/rect between
// DIP and pixel coordinates system. // DIP and pixel coordinates system.
COMPOSITOR_EXPORT gfx::Size ConvertSizeToDIP(
const Layer* layer,
const gfx::Size& size_in_pixel);
COMPOSITOR_EXPORT gfx::Rect ConvertRectToDIP( COMPOSITOR_EXPORT gfx::Rect ConvertRectToDIP(
const Layer* layer, const Layer* layer,
const gfx::Rect& rect_in_pixel); const gfx::Rect& rect_in_pixel);
......
...@@ -34,6 +34,15 @@ PointF ConvertPointToPixels(const PointF& point_in_dips, ...@@ -34,6 +34,15 @@ PointF ConvertPointToPixels(const PointF& point_in_dips,
return ScalePoint(point_in_dips, device_scale_factor); return ScalePoint(point_in_dips, device_scale_factor);
} }
SizeF ConvertSizeToDips(const Size& size_in_pixels, float device_scale_factor) {
return ScaleSize(SizeF(size_in_pixels), 1.f / device_scale_factor);
}
SizeF ConvertSizeToDips(const SizeF& size_in_pixels,
float device_scale_factor) {
return ScaleSize(size_in_pixels, 1.f / device_scale_factor);
}
Insets ConvertInsetsToDIP(float scale_factor, Insets ConvertInsetsToDIP(float scale_factor,
const gfx::Insets& insets_in_pixel) { const gfx::Insets& insets_in_pixel) {
if (scale_factor == 1.f) if (scale_factor == 1.f)
...@@ -41,12 +50,6 @@ Insets ConvertInsetsToDIP(float scale_factor, ...@@ -41,12 +50,6 @@ Insets ConvertInsetsToDIP(float scale_factor,
return insets_in_pixel.Scale(1.f / scale_factor); return insets_in_pixel.Scale(1.f / scale_factor);
} }
Size ConvertSizeToDIP(float scale_factor, const Size& size_in_pixel) {
if (scale_factor == 1.f)
return size_in_pixel;
return ScaleToFlooredSize(size_in_pixel, 1.f / scale_factor);
}
Rect ConvertRectToDIP(float scale_factor, const Rect& rect_in_pixel) { Rect ConvertRectToDIP(float scale_factor, const Rect& rect_in_pixel) {
if (scale_factor == 1.f) if (scale_factor == 1.f)
return rect_in_pixel; return rect_in_pixel;
......
...@@ -14,6 +14,7 @@ class Point; ...@@ -14,6 +14,7 @@ class Point;
class PointF; class PointF;
class Rect; class Rect;
class Size; class Size;
class SizeF;
// This file contains helper functions to move between DIPs (device-independent // This file contains helper functions to move between DIPs (device-independent
// pixels) and physical pixels, by multiplying or dividing by device scale // pixels) and physical pixels, by multiplying or dividing by device scale
...@@ -41,11 +42,14 @@ GEOMETRY_EXPORT gfx::PointF ConvertPointToPixels( ...@@ -41,11 +42,14 @@ GEOMETRY_EXPORT gfx::PointF ConvertPointToPixels(
const gfx::PointF& point_in_dips, const gfx::PointF& point_in_dips,
float device_scale_factor); float device_scale_factor);
GEOMETRY_EXPORT gfx::SizeF ConvertSizeToDips(const gfx::Size& size_in_pixels,
float device_scale_factor);
GEOMETRY_EXPORT gfx::SizeF ConvertSizeToDips(const gfx::SizeF& size_in_pixels,
float device_scale_factor);
GEOMETRY_EXPORT gfx::Insets ConvertInsetsToDIP( GEOMETRY_EXPORT gfx::Insets ConvertInsetsToDIP(
float scale_factor, float scale_factor,
const gfx::Insets& insets_in_pixel); const gfx::Insets& insets_in_pixel);
GEOMETRY_EXPORT gfx::Size ConvertSizeToDIP(float scale_factor,
const gfx::Size& size_in_pixel);
GEOMETRY_EXPORT gfx::Rect ConvertRectToDIP(float scale_factor, GEOMETRY_EXPORT gfx::Rect ConvertRectToDIP(float scale_factor,
const gfx::Rect& rect_in_pixel); const gfx::Rect& rect_in_pixel);
......
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