[webkit-reviews] review denied: [Bug 103914] Add support for tracking hit test rectangles to enable fast event rejection in the compositor : [Attachment 177303] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Levi Weintraub
<leviw at chromium.org>'s request for review:
Bug 103914: Add support for tracking hit test rectangles to enable fast event
rejection in the compositor
https://bugs.webkit.org/show_bug.cgi?id=103914

Attachment 177303: Patch
https://bugs.webkit.org/attachment.cgi?id=177303&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=177303&action=review


> Source/WebCore/rendering/RenderBlock.cpp:6931
> +void RenderBlock::addLayerHitTestRects(LayerHitTestRects& layerRects, const
RenderLayer* currentCompositedLayer, const LayoutPoint& layerOffset) const

I don't understand why you're accumulating these rects relative to some
RenderLayer. Shouldn't they all just be root-relative?

Also the layerOffset stuff won't work with software-rendered transforms.

> Source/WebCore/rendering/RenderBlock.cpp:6960
> +    if (width() && height())
> +	   result.append(pixelSnappedIntRect(adjustedLayerOffset, size()));
> +
> +    if (hasHorizontalLayoutOverflow() || hasVerticalLayoutOverflow()) {
> +	   for (RootInlineBox* curr = firstRootBox(); curr; curr =
curr->nextRootBox()) {
> +	       LayoutUnit top = max<LayoutUnit>(curr->lineTop(), curr->top());
> +	       LayoutUnit bottom = min<LayoutUnit>(curr->lineBottom(),
curr->top() + curr->height());
> +	       LayoutRect rect(adjustedLayerOffset.x() + curr->x(),
adjustedLayerOffset.y() + top, curr->width(), bottom - top);
> +	       if (!rect.isEmpty())
> +		   result.append(pixelSnappedIntRect(rect));
> +	   }
> +    }
> +
> +    for (RenderObject* curr = firstChild(); curr; curr =
curr->nextSibling()) {
> +	   if (!curr->isText() && !curr->isListMarker() && curr->isBox()) {
> +	       RenderBox* box = toRenderBox(curr);
> +	       box->addLayerHitTestRects(layerRects, currentCompositedLayer, 
adjustedLayerOffset + box->locationOffset());
> +	   }
> +    }

Rather than have all this code here, can we make a generic RenderObject method
for computing the hit-test region for an arbitrary element?

> Source/WebCore/rendering/RenderObject.cpp:629
> +	   // Don't apply transforms as we should never pass through one.
> +	   mapLocalToContainer((*enclosingCompositedLayer)->renderer(),
transformState, ApplyContainerFlip | SnapOffsetForTransforms);

Sure you will; 2D transforms don't always make compositing layers.