feat(layout): enhance hit testing for child nodes in LayoutBox #305
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@EndlessJour9527 Please describe what issues to be fixed in this pull request. |
EndlessJour9527
force-pushed
the
feat/enhance_hit_test
branch
from
b4b7f2c to
60ab83c
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR enhances hit testing for layout boxes by implementing proper overflow handling for elements with overflow: visible. The implementation detects when child elements extend beyond their parent's border box and adjusts hit testing to include these overflowing children.
Key changes:
- Added hit-testable overflow detection logic to identify when children extend beyond parent boundaries
- Modified hit testing to use union bounding boxes for overflow cases instead of clipping to parent bounds
- Fixed static positioning to properly handle auto inset values in CSS computed styles
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/client/layout/layout_box.cpp |
Implements hasHitTestableOverflow() and updates mayIntersect() with overflow-aware hit testing logic |
src/client/cssom/computed_style.cpp |
Fixes static positioning by setting auto inset values when position is static |
fixtures/html/dom/document_hittest_overflow.html |
Adds comprehensive test fixture for overflow hit testing scenarios |
src/client/layout/layout_box.cpp
Outdated
| if (!child->isBox()) | ||
| continue; // Only element (box) children can produce overflow boxes here. | ||
|
|
||
| auto childBox = std::dynamic_pointer_cast<LayoutBox>(child); |
There was a problem hiding this comment.
The code performs a std::dynamic_pointer_cast after already checking child->isBox(). Since isBox() should guarantee the child can be cast to LayoutBox, consider using std::static_pointer_cast for better performance, or restructure to avoid the redundant cast.
| auto childBox = std::dynamic_pointer_cast<LayoutBox>(child); | |
| auto childBox = std::static_pointer_cast<LayoutBox>(child); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Other locations apply "dynamic_pointer_cast" why.
| layoutStyle.setRight(inset_.right().toLayoutValue()); | ||
| layoutStyle.setBottom(inset_.bottom().toLayoutValue()); | ||
| layoutStyle.setLeft(inset_.left().toLayoutValue()); | ||
| if (position_type_.isStatic()) |
There was a problem hiding this comment.
Does taffy not handle this?
There was a problem hiding this comment.
Could you please provide some information indicating that taffy does not handle this?
Then, if taffy indeed confirms it doesn’t handle it, I think we should add this logic here
, rather than inside the ComputedStyle class.
| display: block; | ||
| } | ||
|
|
||
| /* FIX hover selector (removed space) */ |
There was a problem hiding this comment.
Remove the TODO-style comment /* FIX hover selector (removed space) */ as it appears to describe a completed fix rather than documenting the current code.
| /* FIX hover selector (removed space) */ |
Copilot uses AI. Check for mistakes.
EndlessJour9527
force-pushed
the
feat/enhance_hit_test
branch
from
b009163 to
60ab83c
Compare
| layoutStyle.setRight(inset_.right().toLayoutValue()); | ||
| layoutStyle.setBottom(inset_.bottom().toLayoutValue()); | ||
| layoutStyle.setLeft(inset_.left().toLayoutValue()); | ||
| if (position_type_.isStatic()) |
There was a problem hiding this comment.
Could you please provide some information indicating that taffy does not handle this?
Then, if taffy indeed confirms it doesn’t handle it, I think we should add this logic here
, rather than inside the ComputedStyle class.
Added hasHitTestableOverflow(): Detects overflow:visible cases where any child box’s physical border box extends beyond the parent’s border box.
Updated mayIntersect(): When overflow is visible and children overflow, builds a union AABB of the parent and child boxes (applies accumulatedOffset) and ray-tests that instead of clipping to the parent.
The element is positioned according to the Normal Flow of the document. The top, right, bottom, left, and z-index properties have no effect. This is the default value.