Commit 386e18a7 authored by Jinsong Fan's avatar Jinsong Fan Committed by Commit Bot

Fix the selection rect when it's clipped to position the selection menu

In the current implementation, the full selection rect between the handles
is always used to calculate the location of the context menu, but the
selection rect may be clipped, so in this case, the position of the context
menu will be far from the visible rect of the entire selection rect.

The CL computes the clipped selection rect for the selection menu.

Bug: 1015274
Change-Id: I899473bec1a061622d189c191fc5fb494928c6bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1877338
Commit-Queue: Jinsong Fan <fanjinsong@sogou-inc.com>
Reviewed-by: default avatarMohsen Izadi <mohsen@chromium.org>
Reviewed-by: default avatarSadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Reviewed-by: default avatarKen Buchanan <kenrb@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713000}
parent 1a035880
...@@ -98,6 +98,18 @@ class ViewportAnchor { ...@@ -98,6 +98,18 @@ class ViewportAnchor {
LayerTreeImpl* tree_impl_; LayerTreeImpl* tree_impl_;
gfx::ScrollOffset viewport_in_content_coordinates_; gfx::ScrollOffset viewport_in_content_coordinates_;
}; };
std::pair<gfx::PointF, gfx::PointF> GetVisibleSelectionEndPoints(
const gfx::RectF& rect,
const gfx::PointF& top,
const gfx::PointF& bottom) {
gfx::PointF start(base::ClampToRange(top.x(), rect.x(), rect.right()),
base::ClampToRange(top.y(), rect.y(), rect.bottom()));
gfx::PointF end =
start + gfx::Vector2dF(bottom.x() - top.x(), bottom.y() - top.y());
return {start, end};
}
} // namespace } // namespace
void LayerTreeLifecycle::AdvanceTo(LifecycleState next_state) { void LayerTreeLifecycle::AdvanceTo(LifecycleState next_state) {
...@@ -2327,6 +2339,31 @@ static gfx::SelectionBound ComputeViewportSelectionBound( ...@@ -2327,6 +2339,31 @@ static gfx::SelectionBound ComputeViewportSelectionBound(
PointHitsLayer(layer, visibility_point, &intersect_distance)); PointHitsLayer(layer, visibility_point, &intersect_distance));
} }
if (viewport_bound.visible()) {
viewport_bound.SetVisibleEdge(viewport_bound.edge_top(),
viewport_bound.edge_bottom());
} else {
// The |layer_top| and |layer_bottom| might be clipped.
gfx::RectF visible_layer_rect(layer->visible_layer_rect());
auto visible_layer_start = layer_top;
auto visible_layer_end = layer_bottom;
if (!visible_layer_rect.Contains(visible_layer_start) &&
!visible_layer_rect.Contains(visible_layer_end))
std::tie(visible_layer_start, visible_layer_end) =
GetVisibleSelectionEndPoints(visible_layer_rect, layer_top,
layer_bottom);
gfx::PointF visible_screen_start = MathUtil::MapPoint(
screen_space_transform, visible_layer_start, &clipped);
gfx::PointF visible_screen_end =
MathUtil::MapPoint(screen_space_transform, visible_layer_end, &clipped);
viewport_bound.SetVisibleEdgeTop(
gfx::ScalePoint(visible_screen_start, inv_scale));
viewport_bound.SetVisibleEdgeBottom(
gfx::ScalePoint(visible_screen_end, inv_scale));
}
return viewport_bound; return viewport_bound;
} }
......
This diff is collapsed.
...@@ -241,7 +241,8 @@ void TouchSelectionControllerClientAura::ShowQuickMenu() { ...@@ -241,7 +241,8 @@ void TouchSelectionControllerClientAura::ShowQuickMenu() {
if (!ui::TouchSelectionMenuRunner::GetInstance()) if (!ui::TouchSelectionMenuRunner::GetInstance())
return; return;
gfx::RectF rect = rwhva_->selection_controller()->GetRectBetweenBounds(); gfx::RectF rect =
rwhva_->selection_controller()->GetVisibleRectBetweenBounds();
// Clip rect, which is in |rwhva_|'s window's coordinate space, to client // Clip rect, which is in |rwhva_|'s window's coordinate space, to client
// bounds. // bounds.
...@@ -472,7 +473,7 @@ void TouchSelectionControllerClientAura::ExecuteCommand(int command_id, ...@@ -472,7 +473,7 @@ void TouchSelectionControllerClientAura::ExecuteCommand(int command_id,
void TouchSelectionControllerClientAura::RunContextMenu() { void TouchSelectionControllerClientAura::RunContextMenu() {
gfx::RectF anchor_rect = gfx::RectF anchor_rect =
rwhva_->selection_controller()->GetRectBetweenBounds(); rwhva_->selection_controller()->GetVisibleRectBetweenBounds();
gfx::PointF anchor_point = gfx::PointF anchor_point =
gfx::PointF(anchor_rect.CenterPoint().x(), anchor_rect.y()); gfx::PointF(anchor_rect.CenterPoint().x(), anchor_rect.y());
RenderWidgetHostImpl* host = rwhva_->host(); RenderWidgetHostImpl* host = rwhva_->host();
......
...@@ -181,7 +181,7 @@ void TouchSelectionControllerClientChildFrame::ExecuteCommand(int command_id, ...@@ -181,7 +181,7 @@ void TouchSelectionControllerClientChildFrame::ExecuteCommand(int command_id,
void TouchSelectionControllerClientChildFrame::RunContextMenu() { void TouchSelectionControllerClientChildFrame::RunContextMenu() {
gfx::RectF anchor_rect = gfx::RectF anchor_rect =
manager_->GetTouchSelectionController()->GetRectBetweenBounds(); manager_->GetTouchSelectionController()->GetVisibleRectBetweenBounds();
gfx::PointF anchor_point = gfx::PointF anchor_point =
gfx::PointF(anchor_rect.CenterPoint().x(), anchor_rect.y()); gfx::PointF(anchor_rect.CenterPoint().x(), anchor_rect.y());
gfx::PointF origin = rwhv_->TransformPointToRootCoordSpaceF(gfx::PointF()); gfx::PointF origin = rwhv_->TransformPointToRootCoordSpaceF(gfx::PointF());
......
...@@ -131,11 +131,11 @@ std::unique_ptr<ui::TouchSelectionController> CreateSelectionController( ...@@ -131,11 +131,11 @@ std::unique_ptr<ui::TouchSelectionController> CreateSelectionController(
} }
gfx::RectF GetSelectionRect(const ui::TouchSelectionController& controller) { gfx::RectF GetSelectionRect(const ui::TouchSelectionController& controller) {
// When the touch handles are on the same line, or one of them is invisible, // When the touch handles are on the same line, the rect may become simply a
// the rect may become a line, and still need to union the handle rect to // one-dimensional rect, and still need to union the handle rect to avoid the
// avoid the context menu covering the touch handle. See detailed comments in // context menu covering the touch handle. See detailed comments in
// TouchSelectionController::GetRectBetweenBounds(). // TouchSelectionController::GetRectBetweenBounds().
gfx::RectF rect = controller.GetRectBetweenBounds(); gfx::RectF rect = controller.GetVisibleRectBetweenBounds();
rect.Union(controller.GetStartHandleRect()); rect.Union(controller.GetStartHandleRect());
rect.Union(controller.GetEndHandleRect()); rect.Union(controller.GetEndHandleRect());
return rect; return rect;
......
...@@ -19,6 +19,8 @@ struct SelectionBound { ...@@ -19,6 +19,8 @@ struct SelectionBound {
SelectionBoundType type; SelectionBoundType type;
gfx.mojom.PointF edge_top; gfx.mojom.PointF edge_top;
gfx.mojom.PointF edge_bottom; gfx.mojom.PointF edge_bottom;
gfx.mojom.PointF visible_edge_top;
gfx.mojom.PointF visible_edge_bottom;
bool visible; bool visible;
}; };
...@@ -61,6 +61,14 @@ struct StructTraits<gfx::mojom::SelectionBoundDataView, gfx::SelectionBound> { ...@@ -61,6 +61,14 @@ struct StructTraits<gfx::mojom::SelectionBoundDataView, gfx::SelectionBound> {
return input.edge_bottom(); return input.edge_bottom();
} }
static gfx::PointF visible_edge_top(const gfx::SelectionBound& input) {
return input.visible_edge_top();
}
static gfx::PointF visible_edge_bottom(const gfx::SelectionBound& input) {
return input.visible_edge_bottom();
}
static bool visible(const gfx::SelectionBound& input) { static bool visible(const gfx::SelectionBound& input) {
return input.visible(); return input.visible();
} }
...@@ -69,9 +77,14 @@ struct StructTraits<gfx::mojom::SelectionBoundDataView, gfx::SelectionBound> { ...@@ -69,9 +77,14 @@ struct StructTraits<gfx::mojom::SelectionBoundDataView, gfx::SelectionBound> {
gfx::SelectionBound* out) { gfx::SelectionBound* out) {
gfx::PointF edge_top; gfx::PointF edge_top;
gfx::PointF edge_bottom; gfx::PointF edge_bottom;
if (!data.ReadEdgeTop(&edge_top) || !data.ReadEdgeBottom(&edge_bottom)) gfx::PointF visible_edge_top;
gfx::PointF visible_edge_bottom;
if (!data.ReadEdgeTop(&edge_top) || !data.ReadEdgeBottom(&edge_bottom) ||
!data.ReadVisibleEdgeTop(&visible_edge_top) ||
!data.ReadVisibleEdgeBottom(&visible_edge_bottom))
return false; return false;
out->SetEdge(edge_top, edge_bottom); out->SetEdge(edge_top, edge_bottom);
out->SetVisibleEdge(visible_edge_top, visible_edge_bottom);
out->set_type(MojoSelectionBoundTypeToGfx(data.type())); out->set_type(MojoSelectionBoundTypeToGfx(data.type()));
out->set_visible(data.visible()); out->set_visible(data.visible());
return true; return true;
......
...@@ -24,17 +24,31 @@ void SelectionBound::SetEdgeTop(const gfx::PointF& value) { ...@@ -24,17 +24,31 @@ void SelectionBound::SetEdgeTop(const gfx::PointF& value) {
edge_top_rounded_ = gfx::ToRoundedPoint(value); edge_top_rounded_ = gfx::ToRoundedPoint(value);
} }
void SelectionBound::SetVisibleEdgeTop(const gfx::PointF& value) {
visible_edge_top_ = value;
}
void SelectionBound::SetEdgeBottom(const gfx::PointF& value) { void SelectionBound::SetEdgeBottom(const gfx::PointF& value) {
edge_bottom_ = value; edge_bottom_ = value;
edge_bottom_rounded_ = gfx::ToRoundedPoint(value); edge_bottom_rounded_ = gfx::ToRoundedPoint(value);
} }
void SelectionBound::SetVisibleEdgeBottom(const gfx::PointF& value) {
visible_edge_bottom_ = value;
}
void SelectionBound::SetEdge(const gfx::PointF& top, void SelectionBound::SetEdge(const gfx::PointF& top,
const gfx::PointF& bottom) { const gfx::PointF& bottom) {
SetEdgeTop(top); SetEdgeTop(top);
SetEdgeBottom(bottom); SetEdgeBottom(bottom);
} }
void SelectionBound::SetVisibleEdge(const gfx::PointF& top,
const gfx::PointF& bottom) {
SetVisibleEdgeTop(top);
SetVisibleEdgeBottom(bottom);
}
int SelectionBound::GetHeight() const { int SelectionBound::GetHeight() const {
return edge_bottom_rounded_.y() - edge_top_rounded_.y(); return edge_bottom_rounded_.y() - edge_top_rounded_.y();
} }
...@@ -49,7 +63,9 @@ std::string SelectionBound::ToString() const { ...@@ -49,7 +63,9 @@ std::string SelectionBound::ToString() const {
bool operator==(const SelectionBound& lhs, const SelectionBound& rhs) { bool operator==(const SelectionBound& lhs, const SelectionBound& rhs) {
return lhs.type() == rhs.type() && lhs.visible() == rhs.visible() && return lhs.type() == rhs.type() && lhs.visible() == rhs.visible() &&
lhs.edge_top() == rhs.edge_top() && lhs.edge_top() == rhs.edge_top() &&
lhs.edge_bottom() == rhs.edge_bottom(); lhs.edge_bottom() == rhs.edge_bottom() &&
lhs.visible_edge_top() == rhs.visible_edge_top() &&
lhs.visible_edge_bottom() == rhs.visible_edge_bottom();
} }
bool operator!=(const SelectionBound& lhs, const SelectionBound& rhs) { bool operator!=(const SelectionBound& lhs, const SelectionBound& rhs) {
...@@ -88,4 +104,23 @@ gfx::RectF RectFBetweenSelectionBounds(const SelectionBound& b1, ...@@ -88,4 +104,23 @@ gfx::RectF RectFBetweenSelectionBounds(const SelectionBound& b1,
return gfx::RectF(top_left, gfx::SizeF(diff.x(), diff.y())); return gfx::RectF(top_left, gfx::SizeF(diff.x(), diff.y()));
} }
gfx::RectF RectFBetweenVisibleSelectionBounds(const SelectionBound& b1,
const SelectionBound& b2) {
// The selection bound is determined by the |top| and |bottom| points. For the
// writing-mode is vertical-*, the bounds are horizontal, the |bottom| might
// be on the left side of the |top|.
gfx::PointF top_left(b1.visible_edge_top());
top_left.SetToMin(b1.visible_edge_bottom());
top_left.SetToMin(b2.visible_edge_top());
top_left.SetToMin(b2.visible_edge_bottom());
gfx::PointF bottom_right(b1.visible_edge_top());
bottom_right.SetToMax(b1.visible_edge_bottom());
bottom_right.SetToMax(b2.visible_edge_top());
bottom_right.SetToMax(b2.visible_edge_bottom());
gfx::Vector2dF diff = bottom_right - top_left;
return gfx::RectF(top_left, gfx::SizeF(diff.x(), diff.y()));
}
} // namespace gfx } // namespace gfx
...@@ -27,14 +27,21 @@ class GFX_EXPORT SelectionBound { ...@@ -27,14 +27,21 @@ class GFX_EXPORT SelectionBound {
void set_type(Type value) { type_ = value; } void set_type(Type value) { type_ = value; }
const gfx::PointF& edge_top() const { return edge_top_; } const gfx::PointF& edge_top() const { return edge_top_; }
const gfx::PointF& visible_edge_top() const { return visible_edge_top_; }
const gfx::Point& edge_top_rounded() const { return edge_top_rounded_; } const gfx::Point& edge_top_rounded() const { return edge_top_rounded_; }
void SetEdgeTop(const gfx::PointF& value); void SetEdgeTop(const gfx::PointF& value);
void SetVisibleEdgeTop(const gfx::PointF& value);
const gfx::PointF& edge_bottom() const { return edge_bottom_; } const gfx::PointF& edge_bottom() const { return edge_bottom_; }
const gfx::PointF& visible_edge_bottom() const {
return visible_edge_bottom_;
}
const gfx::Point& edge_bottom_rounded() const { return edge_bottom_rounded_; } const gfx::Point& edge_bottom_rounded() const { return edge_bottom_rounded_; }
void SetEdgeBottom(const gfx::PointF& value); void SetEdgeBottom(const gfx::PointF& value);
void SetVisibleEdgeBottom(const gfx::PointF& value);
void SetEdge(const gfx::PointF& top, const gfx::PointF& bottom); void SetEdge(const gfx::PointF& top, const gfx::PointF& bottom);
void SetVisibleEdge(const gfx::PointF& top, const gfx::PointF& bottom);
bool visible() const { return visible_; } bool visible() const { return visible_; }
void set_visible(bool value) { visible_ = value; } void set_visible(bool value) { visible_ = value; }
...@@ -46,8 +53,14 @@ class GFX_EXPORT SelectionBound { ...@@ -46,8 +53,14 @@ class GFX_EXPORT SelectionBound {
private: private:
Type type_; Type type_;
// The actual bounds of a selection end-point mgiht be invisible for
// occlusion.
gfx::PointF edge_top_; gfx::PointF edge_top_;
gfx::PointF edge_bottom_; gfx::PointF edge_bottom_;
// The visible bounds of a selection, which are equal to the above, when there
// is no occlusion.
gfx::PointF visible_edge_top_;
gfx::PointF visible_edge_bottom_;
gfx::Point edge_top_rounded_; gfx::Point edge_top_rounded_;
gfx::Point edge_bottom_rounded_; gfx::Point edge_bottom_rounded_;
bool visible_; bool visible_;
...@@ -64,6 +77,9 @@ GFX_EXPORT gfx::Rect RectBetweenSelectionBounds(const SelectionBound& b1, ...@@ -64,6 +77,9 @@ GFX_EXPORT gfx::Rect RectBetweenSelectionBounds(const SelectionBound& b1,
GFX_EXPORT gfx::RectF RectFBetweenSelectionBounds(const SelectionBound& b1, GFX_EXPORT gfx::RectF RectFBetweenSelectionBounds(const SelectionBound& b1,
const SelectionBound& b2); const SelectionBound& b2);
GFX_EXPORT gfx::RectF RectFBetweenVisibleSelectionBounds(
const SelectionBound& b1,
const SelectionBound& b2);
} // namespace ui } // namespace ui
#endif // UI_GFX_SELECTION_BOUND_H_ #endif // UI_GFX_SELECTION_BOUND_H_
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/rect_f.h"
namespace gfx { namespace gfx {
...@@ -21,6 +22,26 @@ TEST(SelectionBoundTest, RectBetweenSelectionBounds) { ...@@ -21,6 +22,26 @@ TEST(SelectionBoundTest, RectBetweenSelectionBounds) {
EXPECT_EQ(expected_rect, RectBetweenSelectionBounds(b1, b2)); EXPECT_EQ(expected_rect, RectBetweenSelectionBounds(b1, b2));
EXPECT_EQ(expected_rect, RectBetweenSelectionBounds(b2, b1)); EXPECT_EQ(expected_rect, RectBetweenSelectionBounds(b2, b1));
// Both bounds are invisible.
b1.SetVisibleEdge(gfx::PointF(10.f, 20.f), gfx::PointF(10.f, 25.f));
b2.SetVisibleEdge(gfx::PointF(100.f, 20.f), gfx::PointF(100.f, 25.f));
gfx::RectF expected_visible_rect(
b1.visible_edge_top().x(), b1.visible_edge_top().y(),
b2.visible_edge_top().x() - b1.visible_edge_top().x(),
b2.visible_edge_bottom().y() - b2.visible_edge_top().y());
EXPECT_EQ(expected_visible_rect, RectFBetweenVisibleSelectionBounds(b1, b2));
EXPECT_EQ(expected_visible_rect, RectFBetweenVisibleSelectionBounds(b2, b1));
// One of the bounds is invisible.
b1.SetVisibleEdge(gfx::PointF(0.f, 20.f), gfx::PointF(0.f, 25.f));
b2.SetVisibleEdge(gfx::PointF(100.f, 20.f), gfx::PointF(100.f, 25.f));
expected_visible_rect =
gfx::RectF(b1.visible_edge_top().x(), b1.visible_edge_top().y(),
b2.visible_edge_top().x() - b1.visible_edge_top().x(),
b2.visible_edge_bottom().y() - b2.visible_edge_top().y());
EXPECT_EQ(expected_visible_rect, RectFBetweenVisibleSelectionBounds(b1, b2));
EXPECT_EQ(expected_visible_rect, RectFBetweenVisibleSelectionBounds(b2, b1));
// Parallel vertical bounds of different heights // Parallel vertical bounds of different heights
b1.SetEdge(gfx::PointF(10.f, 20.f), gfx::PointF(10.f, 25.f)); b1.SetEdge(gfx::PointF(10.f, 20.f), gfx::PointF(10.f, 25.f));
b2.SetEdge(gfx::PointF(110.f, 0.f), gfx::PointF(110.f, 35.f)); b2.SetEdge(gfx::PointF(110.f, 0.f), gfx::PointF(110.f, 35.f));
......
...@@ -271,6 +271,15 @@ gfx::RectF TouchSelectionController::GetRectBetweenBounds() const { ...@@ -271,6 +271,15 @@ gfx::RectF TouchSelectionController::GetRectBetweenBounds() const {
return RectFBetweenSelectionBounds(start_, end_); return RectFBetweenSelectionBounds(start_, end_);
} }
gfx::RectF TouchSelectionController::GetVisibleRectBetweenBounds() const {
// Short-circuit for efficiency.
if (active_status_ == INACTIVE)
return gfx::RectF();
// Returns the rect of the entire visible selection rect.
return RectFBetweenVisibleSelectionBounds(start_, end_);
}
gfx::RectF TouchSelectionController::GetStartHandleRect() const { gfx::RectF TouchSelectionController::GetStartHandleRect() const {
if (active_status_ == INSERTION_ACTIVE) if (active_status_ == INSERTION_ACTIVE)
return insertion_handle_->GetVisibleBounds(); return insertion_handle_->GetVisibleBounds();
......
...@@ -119,6 +119,9 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController ...@@ -119,6 +119,9 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController
// the rect is simply a one-dimensional rect of that bound. If no selection // the rect is simply a one-dimensional rect of that bound. If no selection
// is active, an empty rect will be returned. // is active, an empty rect will be returned.
gfx::RectF GetRectBetweenBounds() const; gfx::RectF GetRectBetweenBounds() const;
// Returns the rect between the selection bounds (as above) but clipped by
// occluding layers.
gfx::RectF GetVisibleRectBetweenBounds() const;
// Returns the visible rect of specified touch handle. For an active insertion // Returns the visible rect of specified touch handle. For an active insertion
// these values will be identical. // these values will be identical.
...@@ -199,6 +202,9 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController ...@@ -199,6 +202,9 @@ class UI_TOUCH_SELECTION_EXPORT TouchSelectionController
InputEventType response_pending_input_event_; InputEventType response_pending_input_event_;
// The bounds at the begin and end of the selection, which might be vertical
// or horizontal line and represents the position of the touch handles or
// caret.
gfx::SelectionBound start_; gfx::SelectionBound start_;
gfx::SelectionBound end_; gfx::SelectionBound end_;
TouchHandleOrientation start_orientation_; TouchHandleOrientation start_orientation_;
......
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