Make IssueLensRequest use CenterRotatedBox
This fixes an issue I realized after https://crrev.com/c/5386207. The
mojom interface definition for IssueLensRequest is a bit ambiguous with
format of the region. This CL transitions to use CenterRotateBox over
RectF to be more clear about the format being sent through mojom.
Bug: b/328294932
Change-Id: Ide3edf8f87824762d272f91280b0b68b6b827679
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5406819
Reviewed-by: Jason Hu <hujasonx@google.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Duncan Mercer <mercerd@google.com>
Cr-Commit-Position: refs/heads/main@{#1280941}
diff --git a/chrome/browser/lens/core/mojom/geometry.mojom b/chrome/browser/lens/core/mojom/geometry.mojom
index 0d0c69f..11907f0 100644
--- a/chrome/browser/lens/core/mojom/geometry.mojom
+++ b/chrome/browser/lens/core/mojom/geometry.mojom
@@ -7,7 +7,7 @@
import "ui/gfx/geometry/mojom/geometry.mojom";
struct CenterRotatedBox {
- // The box
+ // The box with x and y positions at the center.
gfx.mojom.RectF box;
// The angle of rotation around the center of the rect in radians. The
diff --git a/chrome/browser/lens/core/mojom/lens.mojom b/chrome/browser/lens/core/mojom/lens.mojom
index 4af637f..3df25220 100644
--- a/chrome/browser/lens/core/mojom/lens.mojom
+++ b/chrome/browser/lens/core/mojom/lens.mojom
@@ -4,6 +4,7 @@
module lens.mojom;
+import "chrome/browser/lens/core/mojom/geometry.mojom";
import "chrome/browser/lens/core/mojom/text.mojom";
import "ui/gfx/geometry/mojom/geometry.mojom";
import "url/mojom/url.mojom";
@@ -29,9 +30,9 @@
// When this method is called, the C++ coordinator sends a Lens request with
// the given bounding region to the Lens servers and display results in the
// Chrome side panel. The region should be normalized between 0 and 1.
- // TODO(b/329262670): Verify normalization dpoes not cause off-by-1 pixel
+ // TODO(b/329262670): Verify normalization does not cause off-by-1 pixel
// errors.
- IssueLensRequest(gfx.mojom.RectF region);
+ IssueLensRequest(CenterRotatedBox region);
};
// WebUI page handler for request from Browser side. (C++ -> TypeScript)
diff --git a/chrome/browser/resources/lens/overlay/selection_overlay.ts b/chrome/browser/resources/lens/overlay/selection_overlay.ts
index 180c60b..7d16da9 100644
--- a/chrome/browser/resources/lens/overlay/selection_overlay.ts
+++ b/chrome/browser/resources/lens/overlay/selection_overlay.ts
@@ -5,25 +5,15 @@
import './text_layer.js';
import './region_selection.js';
-import type {RectF} from '//resources/mojo/ui/gfx/geometry/mojom/geometry.mojom-webui.js';
import {PolymerElement} from '//resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {BrowserProxyImpl} from './browser_proxy.js';
+import {CenterRotatedBox_CoordinateType} from './geometry.mojom-webui.js';
+import type {CenterRotatedBox} from './geometry.mojom-webui.js';
import {type RegionSelectionElement} from './region_selection.js';
import {getTemplate} from './selection_overlay.html.js';
import {DRAG_THRESHOLD, DragFeature, emptyGestureEvent, type GestureEvent, GestureState} from './selection_utils.js';
-/**
- * Returns a mojo RectF corresponding to the gesture provided.
- */
-function getRectFromGesture(gesture: GestureEvent): RectF {
- const width = Math.abs(gesture.clientX - gesture.startX);
- const height = Math.abs(gesture.clientY - gesture.startY);
- const topLeftX = Math.min(gesture.clientX, gesture.startX);
- const topLeftY = Math.min(gesture.clientY, gesture.startY);
- return {x: topLeftX, y: topLeftY, width: width, height: height};
-}
-
export interface SelectionOverlayElement {
$: {regionSelectionLayer: RegionSelectionElement};
}
@@ -85,7 +75,8 @@
// Drag has finished. Let the features respond to the end of a drag.
if (this.draggingRespondent === DragFeature.MANUAL_REGION) {
BrowserProxyImpl.getInstance().handler.issueLensRequest(
- getRectFromGesture(this.currentGesture));
+ this.getNormalizedCenterRotatedBoxFromGesture(
+ this.currentGesture));
}
break;
case GestureState.STARTING:
@@ -143,6 +134,34 @@
Math.abs(this.currentGesture.clientY - this.currentGesture.startY);
return xMovement > DRAG_THRESHOLD || yMovement > DRAG_THRESHOLD;
}
+
+ /**
+ * @returns a mojo CenterRotatedBox corresponding to the gesture provided,
+ * normalized to the selection overlay dimensions.
+ */
+ private getNormalizedCenterRotatedBoxFromGesture(gesture: GestureEvent):
+ CenterRotatedBox {
+ const parentRect = this.getBoundingClientRect();
+
+ const normalizedWidth =
+ Math.abs(gesture.clientX - gesture.startX) / parentRect.width;
+ const normalizedHeight =
+ Math.abs(gesture.clientY - gesture.startY) / parentRect.height;
+ const normalizedCenterX =
+ (gesture.clientX + gesture.startX) / 2 / parentRect.width;
+ const normalizedCenterY =
+ (gesture.clientY + gesture.startY) / 2 / parentRect.height;
+ return {
+ box: {
+ x: normalizedCenterX,
+ y: normalizedCenterY,
+ width: normalizedWidth,
+ height: normalizedHeight,
+ },
+ rotation: 0,
+ coordinateType: CenterRotatedBox_CoordinateType.kNormalized,
+ };
+ }
}
declare global {
diff --git a/chrome/browser/ui/lens/lens_overlay_controller.cc b/chrome/browser/ui/lens/lens_overlay_controller.cc
index 6e84079..25fad13 100644
--- a/chrome/browser/ui/lens/lens_overlay_controller.cc
+++ b/chrome/browser/ui/lens/lens_overlay_controller.cc
@@ -463,14 +463,15 @@
LensOverlayControllerTabGlue::CreateForWebContents(contents, this);
}
-void LensOverlayController::IssueLensRequest(const ::gfx::RectF& region) {
+void LensOverlayController::IssueLensRequest(
+ lens::mojom::CenterRotatedBoxPtr region) {
lens::proto::LensOverlayRequest request;
request.set_type(lens::proto::LensOverlayRequest::REGION_SEARCH);
auto* request_region = request.mutable_region();
- request_region->set_x(region.x());
- request_region->set_y(region.y());
- request_region->set_width(region.width());
- request_region->set_height(region.height());
+ request_region->set_x(region->box.x());
+ request_region->set_y(region->box.y());
+ request_region->set_width(region->box.width());
+ request_region->set_height(region->box.height());
lens_overlay_query_controller_->SendInteraction(
request, base::BindOnce(&LensOverlayController::HandleInteractionResponse,
diff --git a/chrome/browser/ui/lens/lens_overlay_controller.h b/chrome/browser/ui/lens/lens_overlay_controller.h
index 1905a1a..90c1cff 100644
--- a/chrome/browser/ui/lens/lens_overlay_controller.h
+++ b/chrome/browser/ui/lens/lens_overlay_controller.h
@@ -8,6 +8,7 @@
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
+#include "chrome/browser/lens/core/mojom/geometry.mojom.h"
#include "chrome/browser/lens/core/mojom/lens.mojom.h"
#include "chrome/browser/lens/core/mojom/text.mojom.h"
#include "chrome/browser/resources/lens/server/proto/lens_overlay_response.pb.h"
@@ -177,7 +178,7 @@
// lens::mojom::LensPageHandler overrides.
void CloseRequestedByOverlay() override;
- void IssueLensRequest(const ::gfx::RectF& region) override;
+ void IssueLensRequest(lens::mojom::CenterRotatedBoxPtr region) override;
// Calls CloseUI() asynchronously.
void CloseUIAsync();
diff --git a/chrome/test/data/webui/lens/overlay/region_selection_test.ts b/chrome/test/data/webui/lens/overlay/region_selection_test.ts
index 56209464..a6422d4 100644
--- a/chrome/test/data/webui/lens/overlay/region_selection_test.ts
+++ b/chrome/test/data/webui/lens/overlay/region_selection_test.ts
@@ -6,6 +6,8 @@
import type {RectF} from '//resources/mojo/ui/gfx/geometry/mojom/geometry.mojom-webui.js';
import {BrowserProxyImpl} from 'chrome-untrusted://lens/browser_proxy.js';
+import {CenterRotatedBox_CoordinateType} from 'chrome-untrusted://lens/geometry.mojom-webui.js';
+import type {CenterRotatedBox} from 'chrome-untrusted://lens/geometry.mojom-webui.js';
import type {SelectionOverlayElement} from 'chrome-untrusted://lens/selection_overlay.js';
import {assertDeepEquals} from 'chrome-untrusted://webui-test/chai_assert.js';
import {flushTasks, waitAfterNextRender} from 'chrome-untrusted://webui-test/polymer_test_util.js';
@@ -53,11 +55,26 @@
return flushTasks();
}
+ // Normalizes the given values to the size of selection overlay.
+ function normalizedBox(box: RectF): RectF {
+ const boundingRect = selectionOverlayElement.getBoundingClientRect();
+ return {
+ x: box.x / boundingRect.width,
+ y: box.y / boundingRect.height,
+ width: box.width / boundingRect.width,
+ height: box.height / boundingRect.height,
+ };
+ }
+
test(
'verify that completing a drag issues lens request via mojo',
async () => {
await doDrag({x: 10, y: 10}, {x: 100, y: 100});
- const expectedRect: RectF = {x: 10, y: 10, width: 90, height: 90};
+ const expectedRect: CenterRotatedBox = {
+ box: normalizedBox({x: 55, y: 55, width: 90, height: 90}),
+ rotation: 0,
+ coordinateType: CenterRotatedBox_CoordinateType.kNormalized,
+ };
const rect =
await testBrowserProxy.handler.whenCalled('issueLensRequest');
assertDeepEquals(expectedRect, rect);
diff --git a/chrome/test/data/webui/lens/overlay/test_overlay_browser_proxy.ts b/chrome/test/data/webui/lens/overlay/test_overlay_browser_proxy.ts
index b8a6144..bfb2201 100644
--- a/chrome/test/data/webui/lens/overlay/test_overlay_browser_proxy.ts
+++ b/chrome/test/data/webui/lens/overlay/test_overlay_browser_proxy.ts
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-import type {RectF} from '//resources/mojo/ui/gfx/geometry/mojom/geometry.mojom-webui.js';
import type {BrowserProxy} from 'chrome-untrusted://lens/browser_proxy.js';
+import type {CenterRotatedBox} from 'chrome-untrusted://lens/geometry.mojom-webui.js';
import {LensPageCallbackRouter, type LensPageHandlerInterface} from 'chrome-untrusted://lens/lens.mojom-webui.js';
import {TestBrowserProxy} from 'chrome-untrusted://webui-test/test_browser_proxy.js';
@@ -21,7 +21,7 @@
this.methodCalled('closeRequestedByOverlay');
}
- issueLensRequest(rect: RectF) {
+ issueLensRequest(rect: CenterRotatedBox) {
this.methodCalled('issueLensRequest', rect);
}
}
