-
-
Notifications
You must be signed in to change notification settings - Fork 130
Fix find element wrong coordinate #306
Fix find element wrong coordinate #306
Conversation
helpers.fixScaleTemplateImage = async function (b64Template, scale) { | ||
if (!scale) { | ||
return b64Template; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scale
might be destructed here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean const {xScale, yScale} = scale
, so you don't have to always add scele.
prefix
lib/basedriver/commands/find.js
Outdated
@@ -201,7 +201,8 @@ helpers.findByImage = async function (b64Template, { | |||
}) { | |||
const { | |||
imageMatchThreshold: threshold, | |||
fixImageTemplateSize | |||
fixImageTemplateSize, | |||
fixTemplateImageScale |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets be consistent fixImageTemplateSize + fixImageTemplateScale %)
lib/basedriver/commands/find.js
Outdated
* | ||
* @param {string} b64Template - base64-encoded image used as a template to be | ||
* matched in the screenshot | ||
* @param {?ScreenshotScale} scale - scale of screen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be better for the readability if you provide the default value for the argument if it is optional (scale=null
)
I'm about to merge this in order to publish this via at beta. |
// screenshot size like `@driver.window_rect #=>x=0, y=0, width=1080, height=1794` and | ||
// `"deviceScreenSize"=>"1080x1920"` | ||
let scale; | ||
if (screenWidth !== shotWidth && screenHeight !== shotHeight) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will consider different ratio case more (and create a PR)
The ratio between the device window size and taken screenshot size is always equal to 1 regardless of the device display density on Android platform.See for details: appium/appium-base-driver#306.
closes appium/appium#12209
@jlipps Can you give me a good example app for
if (screenAR === shotAR)
lines I can confirm?I simply commented out for now. If we do not need this, I would like to remove them.
will update https://github.com/appium/appium/blob/master/docs/en/advanced-concepts/image-elements.md later
problem
find by image feature returns wrong coordinate in Android and iOS case.
test cases in below test section fails even it compares same image/element.
For Android
In here, Appium scales screenshot to window size. Then, Appium uses the scaled image as an input for
compareImages
.e.g.
The screenshot is
1080x1920
. The window size is1080x1794
. OpenCV compares a template image with the screenshot which is scaled from1080x1920
to1080x1794
. Then Appium returns1080x1794
based coordinations if it succeeds to find the template image. (But the returns coordinate should be1080x1920
based one, sincefind_element :accessibility_id, xxxx
returns rect based on1080x1920
based size.For iOS
iOS returns
750 × 1334 pixels
screenshot evern the window size iswidth=375, height=667
.compareImages
uses scaled screenshot from750 × 1334 pixels
towidth=375, height=667
. The ratio is 2.Users should coordinate a template image scale if they would like to use
750 × 1334 pixels
screenshot based image as the template image to fixt towidth=375, height=667
.It means:
Cannot find proper image
Then, users should do:
Implementation
fixScaleTemplateImage
for iOS case to reduce user actions for an input imagetest
https://github.com/appium/ruby_lib_core/pull/193/files is an example to make sure this PR works.
In the test, I ensured the below case for Android and iOS:
step1
andstep3
They worked after this PR.
(This includes landscape and portrait mode.)