Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the Layer::getLayersUnderPoint and Layer::hitTestPoint methods, and add related unit test cases. #282

Merged
merged 36 commits into from
Oct 31, 2024

Conversation

shlzxjp
Copy link
Collaborator

@shlzxjp shlzxjp commented Oct 28, 2024

No description provided.

@shlzxjp shlzxjp closed this Oct 28, 2024
@shlzxjp shlzxjp reopened this Oct 28, 2024
@shlzxjp shlzxjp closed this Oct 28, 2024
@shlzxjp shlzxjp reopened this Oct 28, 2024
@shlzxjp shlzxjp closed this Oct 28, 2024
@shlzxjp shlzxjp reopened this Oct 28, 2024
if (hasLayerUnderPoint) {
results->push_back(weakThis.lock());
} else {
Rect layerBoundsRect = getBounds();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不需要测量自身的bounds。子项只要存在一个碰撞,父级图层就已经成功了,加进result数组就行。你在这里调用getBounds,又重新触发了一次遍历所有子项的操作。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的逻辑是因为如果当前Layer的所有子项都未命中,但实际上(x,y)落在了所有子项形成的包围盒内,是否认定为命中了Layer图层。如果认为该Layer图层未被命中,那只需要使用LayerContent获取Bounds即可。

}

bool hasLayerUnderPoint = false;
for (auto iter = _children.rbegin(); iter != _children.rend(); iter++) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iter避免用缩写,或者使用child/item

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改为item

if (!childLayer->visible()) {
continue;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

漏了处理scrollRect和mask的判断,如果存在遮罩应该要判定是否能点到。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是打算等mask逻辑合入后再添加,方便测试。先增加逻辑,后补测试用例。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mask的实现逻辑和你无关。它负责渲染。你的碰撞检测是独立的,只涉及了bounds计算。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已增加


Rect bounds = Rect::MakeEmpty();
bounds.join(content->getBounds());
getGlobalMatrix().mapRect(&bounds);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不应该每次去获取一个全局矩阵,这样每个layer都在反复做矩阵计算,应该是实现个内部方法,向下遍历的时候把xy坐标转换到Layer本地。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改为使用相对于当前Layer的局部坐标去判断,不需要获取global Matrix

Comment on lines 474 to 476
virtual bool hitTestByBounds(float x, float y);

virtual bool hitTestByPixel(float x, float y);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这两个方法不应该加在Layer上实现,你所有的碰撞都只需要获取Layer的LayerContent进行测试就行。只要存在一个Content命中或者子Layer的Content命中,那么这个Layer就算是命中的。所以这个方法应该加到LayerContent上实现。这样还能用多态来兼顾ImageContent只支持bounds检查的特例。只需要加一个LayerContent::hitTest(float x, float y, bool pixelHitest)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这两个方法已从Layer上迁移到LayerContent上

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已合并为一个方法

}

bool Layer::hitTestPointInternal(float localX, float localY, bool pixelHitTest) {
for (auto item = _children.rbegin(); item != _children.rend(); ++item) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个并没有顺序要求,可以不用倒过来遍历:for(auto& child : _children)就行了。

Copy link
Collaborator

@domchen domchen Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

建议是先hitTest自身的LayerContent再去遍历子项。命中了就不用遍历子项了。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改


Matrix inverseMatrix = Matrix::I();
Point pointInChildSpace = Point::Zero();
if (childLayer->matrix().invert(&inverseMatrix)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同样的要使用getMatrixWithSrollRect()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修复

}

if (nullptr != childLayer->_scrollRect) {
if (!childLayer->_scrollRect->contains(pointInChildSpace.x, pointInChildSpace.y)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

同上对应修改,考虑偏移量。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改

}
}

if (nullptr != childLayer->_mask) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果存在mask,并且需要精确碰撞的时候,也要精确碰撞这个mask。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改


if (nullptr != childLayer->_mask) {
const auto maskBounds = childLayer->_mask->getBounds();
if (!maskBounds.contains(pointInChildSpace.x, pointInChildSpace.y)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里同样,改成调用mask->hitTestPoint() 并且直接把pixelHitTest参数传递进去。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改

return imageBounds.contains(localX, localY);
}

const Rect imageBounds = Rect::MakeXYWH(0, 0, image->width(), image->height());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

声明变量都尽可能用auto关键字,提高可读性。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 31 to 35
if (pixelHitTest) {
// TODO: use bounding box judgment first, and then optimize it to pixel-level judgment.
const Rect imageBounds = Rect::MakeXYWH(0, 0, image->width(), image->height());
return imageBounds.contains(localX, localY);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些可以直接删除了。应该加个注释说明:// The pixelHitTest flag is ignored because we cannot read pixels from images before they are drawn.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改

return textBounds.contains(localX, localY);
}

Path TextContent::getTextContentPath() const {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

给GlyphRunList加个精确碰撞的方法,深入到内部去逐个glyph进行检测,命中任何一个就返回,不需要遍历完整的所有字符。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已增加

@@ -43,4 +43,14 @@ void ComposeContent::draw(Canvas* canvas, const Paint& paint) const {
content->draw(canvas, paint);
}
}

bool ComposeContent::hitTestPoint(float localX, float localY, bool pixelHitTest) {
for (auto item = contents.rbegin(); item != contents.rend(); ++item) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里也用for(auto&:),正序就行了。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改

@@ -31,4 +31,8 @@ void SolidContent::draw(Canvas* canvas, const Paint& paint) const {
canvas->drawRRect(_rRect, solidPaint);
}

bool SolidContent::hitTestPoint(float localX, float localY, bool /*pixelHitTest*/) {
return getBounds().contains(localX, localY);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里_rRect.rect.contains()就行。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getBounds方法的实现就是 return _rRect.rect;

} else {
Path glyphPath = {};
if (font.getPath(glyphID, &glyphPath)) {
auto offsetMatrix = Matrix::MakeTrans(position.x, position.y);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里transform path的开销太高了,可以改成反向transform要测试的坐标点。

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto offsetMatrix = Matrix::MakeTrans(position.x, position.y);
auto inverseMatrix = Matrix::I();
if (offsetMatrix.invert(&inverseMatrix)) {
auto checkPoint = inverseMatrix.mapXY(localX, localY);
if (glyphPath.contains(checkPoint.x, checkPoint.y)) {
return true;
}
}

Comment on lines 47 to 54
/**
* Checks if the given point P(localX, localY) is within the rendered content area of the layer.
* localX is the local x-coordinate relative to the current layer,
* and localY is the local y-coordinate relative to the current layer.
* If pixelHitTest is true, performs a pixel-level hit test.
* Note that this method is time-consuming and should only be used when precise checking is needed.
* If pixelHitTest is false, uses bounding box for hit testing.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/**

  • Checks if the layer content overlaps or intersects with the specified point (localX, localY).
  • The localX and localY coordinates are in the layer's local coordinate space. If the
  • pixelHitTest flag is true, it checks the actual pixels of the layer content; otherwise, it
  • checks the bounding box.
    */

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@domchen domchen merged commit bc64712 into main Oct 31, 2024
8 checks passed
@domchen domchen deleted the feature/xjp_get_layers_under_point branch October 31, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants