This repository has been archived by the owner on Oct 25, 2023. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 130
Fix template scale in find element by image #307
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
8e733b7
add imageTemplateDefaultScale to scale arbitrary scale every time by …
KazuCocoa d0e713e
put all of template scaling into one method
KazuCocoa ea57050
increase test cases to cover all situations
KazuCocoa 69d0b84
define gnoreDefaultImageTemplateScale for click
KazuCocoa acf991f
add a comment
KazuCocoa 281457d
tweak comments
KazuCocoa 4ae5e24
fix typo, default value, docstring
KazuCocoa 7c09a98
fix review
KazuCocoa 17cb7da
fix ratio potential issue
KazuCocoa 57ed6ab
use const instead of let
KazuCocoa 952f83d
tweak a comment about scale factor
KazuCocoa de88d3e
remove _, tweak NaN comparison
KazuCocoa ce71874
use math.round to compare two float
KazuCocoa 6dde6f0
rename float round constant
KazuCocoa 1df9443
fix typo
KazuCocoa b536b10
update imgObj
KazuCocoa 07c246a
fix comments
KazuCocoa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,14 +4,18 @@ import _ from 'lodash'; | |
import { errors } from '../../..'; | ||
import { MATCH_TEMPLATE_MODE } from './images'; | ||
import { W3C_ELEMENT_KEY, MJSONWP_ELEMENT_KEY } from '../../protocol/protocol'; | ||
import { ImageElement } from '../image-element'; | ||
import { ImageElement, DEFAULT_TEMPLATE_IMAGE_SCALE } from '../image-element'; | ||
|
||
|
||
const commands = {}, helpers = {}, extensions = {}; | ||
|
||
const IMAGE_STRATEGY = '-image'; | ||
const CUSTOM_STRATEGY = '-custom'; | ||
|
||
// Used to compare ratio and screen width | ||
// Pixel is basically under 1080 for example. 100K is probably enough fo a while. | ||
const FLOAT_PRECISION = 100000; | ||
|
||
// Override the following function for your own driver, and the rest is taken | ||
// care of! | ||
|
||
|
@@ -183,6 +187,9 @@ commands.findByCustom = async function (selector, multiple) { | |
* image is merely to check staleness. If so we can bypass a lot of logic | ||
* @property {boolean} [multiple=false] - Whether we are finding one element or | ||
* multiple | ||
* @property {boolean} [ignoreDefaultImageTemplateScale=false] - Whether we | ||
* ignore defaultImageTemplateScale. It can be used when you would like to | ||
* scale b64Template with defaultImageTemplateScale setting. | ||
*/ | ||
|
||
/** | ||
|
@@ -198,11 +205,13 @@ commands.findByCustom = async function (selector, multiple) { | |
helpers.findByImage = async function (b64Template, { | ||
shouldCheckStaleness = false, | ||
multiple = false, | ||
ignoreDefaultImageTemplateScale = false, | ||
}) { | ||
const { | ||
imageMatchThreshold: threshold, | ||
fixImageTemplateSize, | ||
fixImageTemplateScale | ||
fixImageTemplateScale, | ||
defaultImageTemplateScale | ||
} = this.settings.getSettings(); | ||
|
||
log.info(`Finding image element with match threshold ${threshold}`); | ||
|
@@ -225,9 +234,10 @@ helpers.findByImage = async function (b64Template, { | |
try { | ||
const {b64Screenshot, scale} = await this.getScreenshotForImageFind(screenWidth, screenHeight); | ||
|
||
if (fixImageTemplateScale) { | ||
b64Template = await this.fixImageTemplateScale(b64Template, scale); | ||
} | ||
b64Template = await this.fixImageTemplateScale(b64Template, { | ||
defaultImageTemplateScale, ignoreDefaultImageTemplateScale, | ||
fixImageTemplateScale, ...scale | ||
}); | ||
|
||
rect = (await this.compareImages(MATCH_TEMPLATE_MODE, b64Screenshot, | ||
b64Template, {threshold})).rect; | ||
|
@@ -357,42 +367,126 @@ helpers.getScreenshotForImageFind = async function (screenWidth, screenHeight) { | |
// of coordinates returned by the image match algorithm, since we match based | ||
// on the screenshot coordinates not the device coordinates themselves. There | ||
// are two potential types of mismatch: aspect ratio mismatch and scale | ||
// mismatch. | ||
// mismatch. We need to detect and fix both | ||
|
||
const scale = {xScale: 1.0, yScale: 1.0}; | ||
|
||
const screenAR = screenWidth / screenHeight; | ||
const shotAR = shotWidth / shotHeight; | ||
if (Math.round(screenAR * FLOAT_PRECISION) === Math.round(shotAR * FLOAT_PRECISION)) { | ||
log.info(`Screenshot aspect ratio '${shotAR}' (${shotWidth}x${shotHeight}) matched ` + | ||
`screen aspect ratio '${screenAR}' (${screenWidth}x${screenHeight})`); | ||
} else { | ||
log.warn(`When trying to find an element, determined that the screen ` + | ||
`aspect ratio and screenshot aspect ratio are different. Screen ` + | ||
`is ${screenWidth}x${screenHeight} whereas screenshot is ` + | ||
`${shotWidth}x${shotHeight}.`); | ||
|
||
// In the case where the x-scale and y-scale are different, we need to decide | ||
// which one to respect, otherwise the screenshot and template will end up | ||
// being resized in a way that changes its aspect ratio (distorts it). For example, let's say: | ||
// this.getScreenshot(shotWidth, shotHeight) is 540x397, | ||
// this.getDeviceSize(screenWidth, screenHeight) is 1080x1920. | ||
// The ratio would then be {xScale: 0.5, yScale: 0.2}. | ||
// In this case, we must should `yScale: 0.2` as scaleFactor, because | ||
// if we select the xScale, the height will be bigger than real screenshot size | ||
// which is used to image comparison by OpenCV as a base image. | ||
// All of this is primarily useful when the screenshot is a horizontal slice taken out of the | ||
// screen (for example not including top/bottom nav bars) | ||
const xScale = (1.0 * shotWidth) / screenWidth; | ||
const yScale = (1.0 * shotHeight) / screenHeight; | ||
const scaleFactor = xScale >= yScale ? yScale : xScale; | ||
|
||
log.warn(`Resizing screenshot to ${shotWidth * scaleFactor}x${shotHeight * scaleFactor} to match ` + | ||
`screen aspect ratio so that image element coordinates have a ` + | ||
`greater chance of being correct.`); | ||
imgObj = imgObj.resize(shotWidth * scaleFactor, shotHeight * scaleFactor); | ||
|
||
scale.xScale *= scaleFactor; | ||
scale.yScale *= scaleFactor; | ||
|
||
shotWidth = imgObj.bitmap.width; | ||
shotHeight = imgObj.bitmap.height; | ||
} | ||
|
||
// Resize based on the screen dimensions only if both width and height are mismatched | ||
// since except for that, it might be a situation which is different window rect and | ||
// screenshot size like `@driver.window_rect #=>x=0, y=0, width=1080, height=1794` and | ||
// `"deviceScreenSize"=>"1080x1920"` | ||
let scale; | ||
if (screenWidth !== shotWidth && screenHeight !== shotHeight) { | ||
log.info(`Scaling screenshot from ${shotWidth}x${shotHeight} to match ` + | ||
`screen at ${screenWidth}x${screenHeight}`); | ||
imgObj = imgObj.resize(screenWidth, screenHeight); | ||
|
||
scale = {xScale: (1.0 * screenWidth) / shotWidth, yScale: (1.0 * screenHeight) / shotHeight}; | ||
scale.xScale *= (1.0 * screenWidth) / shotWidth; | ||
scale.yScale *= (1.0 * screenHeight) / shotHeight; | ||
} | ||
|
||
b64Screenshot = (await imgObj.getBuffer(imageUtil.MIME_PNG)).toString('base64'); | ||
return {b64Screenshot, scale}; | ||
}; | ||
|
||
/** | ||
* @typedef {Object} ImageTemplateSettings | ||
* @property {boolean} fixImageTemplateScale - fixImageTemplateScale in device-settings | ||
* @property {float} defaultImageTemplateScale - defaultImageTemplateScale in device-settings | ||
* @property {boolean} ignoreDefaultImageTemplateScale - Ignore defaultImageTemplateScale if it has true. | ||
mykola-mokhnach marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* If b64Template has been scaled to defaultImageTemplateScale or should ignore the scale, | ||
* this parameter should be true. e.g. click in image-element module | ||
* @property {float} xScale - Scale ratio for width | ||
* @property {float} yScale - Scale ratio for height | ||
|
||
*/ | ||
/** | ||
* Get a image that will be used for template maching. | ||
* Returns scaled image if scale ratio is provided. | ||
* | ||
* | ||
* @param {string} b64Template - base64-encoded image used as a template to be | ||
* matched in the screenshot | ||
* @param {ScreenshotScale} scale - scale of screen. Default to `{}` | ||
* @param {ImageTemplateSettings} opts - Image template scale related options | ||
* | ||
* @returns {string} base64-encoded scaled template screenshot | ||
*/ | ||
helpers.fixImageTemplateScale = async function (b64Template, scale = {}) { | ||
if (!scale) { | ||
const DEFAULT_FIX_IMAGE_TEMPLATE_SCALE = 1; | ||
helpers.fixImageTemplateScale = async function (b64Template, opts = {}) { | ||
if (!opts) { | ||
return b64Template; | ||
} | ||
|
||
let { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
fixImageTemplateScale = false, | ||
defaultImageTemplateScale = DEFAULT_TEMPLATE_IMAGE_SCALE, | ||
ignoreDefaultImageTemplateScale = false, | ||
xScale = DEFAULT_FIX_IMAGE_TEMPLATE_SCALE, | ||
yScale = DEFAULT_FIX_IMAGE_TEMPLATE_SCALE | ||
} = opts; | ||
|
||
if (ignoreDefaultImageTemplateScale) { | ||
defaultImageTemplateScale = DEFAULT_TEMPLATE_IMAGE_SCALE; | ||
} | ||
|
||
// Default | ||
if (defaultImageTemplateScale === DEFAULT_TEMPLATE_IMAGE_SCALE && !fixImageTemplateScale) { | ||
return b64Template; | ||
} | ||
|
||
// Calculate xScale and yScale Appium should scale | ||
if (fixImageTemplateScale) { | ||
xScale *= defaultImageTemplateScale; | ||
yScale *= defaultImageTemplateScale; | ||
} else { | ||
xScale = yScale = 1 * defaultImageTemplateScale; | ||
} | ||
|
||
// xScale and yScale can be NaN if defaultImageTemplateScale is string, for example | ||
if (!parseFloat(xScale) || !parseFloat(yScale)) { | ||
return b64Template; | ||
} | ||
|
||
const {xScale, yScale} = scale; | ||
if (!xScale || !yScale) { | ||
// Return if the scale is default, 1, value | ||
if (Math.round(xScale * FLOAT_PRECISION) === Math.round(DEFAULT_FIX_IMAGE_TEMPLATE_SCALE * FLOAT_PRECISION) | ||
&& Math.round(yScale * FLOAT_PRECISION === Math.round(DEFAULT_FIX_IMAGE_TEMPLATE_SCALE * FLOAT_PRECISION))) { | ||
return b64Template; | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import _ from 'lodash'; | ||
import log from './logger'; | ||
import { DEFAULT_MATCH_THRESHOLD } from './commands/images'; | ||
import { IMAGE_EL_TAP_STRATEGY_W3C } from './image-element'; | ||
import { IMAGE_EL_TAP_STRATEGY_W3C, DEFAULT_TEMPLATE_IMAGE_SCALE } from './image-element'; | ||
|
||
const GLOBAL_DEFAULT_SETTINGS = { | ||
// value between 0 and 1 representing match strength, below which an image | ||
|
@@ -25,6 +25,14 @@ const GLOBAL_DEFAULT_SETTINGS = { | |
// if a user use `width=750 × height=1334` pixels's base template image. | ||
fixImageTemplateScale: false, | ||
|
||
// Users might have scaled template image to reduce their storage size. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be nice to have this also described in some doc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I'm thinking to add this in https://github.com/appium/appium/blob/master/docs/en/advanced-concepts/image-elements.md |
||
// This setting allows users to scale a template image they send to Appium server | ||
// so that the Appium server compares the actual scale users originally had. | ||
// e.g. If a user has an image of 270 x 32 pixels which was originally 1080 x 126 pixels, | ||
// the user can set {defaultImageTemplateScale: 4.0} to scale the small image | ||
// to the original one so that Appium can compare it as the original one. | ||
defaultImageTemplateScale: DEFAULT_TEMPLATE_IMAGE_SCALE, | ||
|
||
// whether Appium should re-check that an image element can be matched | ||
// against the current screenshot before clicking it | ||
checkForImageElementStaleness: true, | ||
|
@@ -44,6 +52,8 @@ const BASEDRIVER_HANDLED_SETTINGS = [ | |
'imageMatchThreshold', | ||
'fixImageFindScreenshotDims', | ||
'fixImageTemplateSize', | ||
'fixImageTemplateScale', | ||
'defaultImageTemplateScale', | ||
'checkForImageElementStaleness', | ||
'autoUpdateImageElementPosition', | ||
'imageElementTapStrategy', | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
shouldn't we be comparing the new imgObj width and height here? it looks like we're comparing against the old shotWidth/shotHeight, but those could have changed in the previous block, right?
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.
Right. I added b536b10 to update
shotWidth
andshotHeight
if they should have been updated in the ratio comparisonThere 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.
@jlipps Do you have a chance to take a look ^?