[Webkit-unassigned] [Bug 40197] Enhance the hit testing to take a rectangle instead of a point

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 21 06:31:08 PDT 2010


Antonio Gomes <tonikitoo at webkit.org> changed:

           What    |Removed                     |Added
  Attachment #62029|0                           |1
        is obsolete|                            |
  Attachment #62175|                            |review?, commit-queue-
               Flag|                            |

--- Comment #42 from Antonio Gomes <tonikitoo at webkit.org>  2010-07-21 06:31:06 PST ---
Created an attachment (id=62175)
 --> (https://bugs.webkit.org/attachment.cgi?id=62175)
patch v4 - feature + tests

Adressing Kenneth's comments.

> WebCore/dom/Document.cpp:1055
>  +      float zoomFactor = frameView->pageZoomFactor();
> What about scale? The QGraphicsWebView can have a scale set which will be applied to the QPainter - will that work?

I do not know we have to scale the padding area based on the current zoom factor. The centerPoint has to consider scale, not the padding (since our thumb and index finger do not enlarge :)

> WebCore/dom/Document.cpp:1056
>  +      IntPoint point = roundedIntPoint(FloatPoint(x * zoomFactor  + view()->scrollX(), y * zoomFactor + view()->scrollY()));
> Is there any need to specify that it is an int point?

All hit testing system is based on int value, so we cast back it from FroatPoint to IntPoint.

> WebCore/page/EventHandler.h:108
>  +      HitTestResult hitTestResultAtPoint(const IntPoint&, bool allowShadowContent, bool ignoreClipping = false, HitTestScrollbars scrollbars = DontHitTestScrollbars, const IntSize& padding = IntSize(-1, -1));
> I believe that we have something like IntPoint::zero() or zeroPoint or so, maybe we want a special constructor for this? Darin?

I tried to convince Darin Adler about the usage of a static IntSize::invalid { return IntSize(-1, -1); } and IntSize::isValid() const { return m_width >= 0 && m_height >= 0; } without success. See bug 42653.

I left it as it was.

> WebCore/rendering/HitTestResult.h:59
>  +      bool isRegionTest() const { return m_isRegionTest && hasValidPadding(); }
> You talk about rect based hit testing, and here you call it region? Is it region or is it rect?

You are right. I change any reference to region to rectBased. For example: isRegionTest became isRectBasedTest

> WebCore/rendering/HitTestResult.h:97
>  +      inline bool hasValidPadding() const { return m_padding.width() >= 0 && m_padding.height() >= 0; }
> wouldn't hasPadding be sufficient? Padding, region, rect... it is a bit confusing

This method has gone.

> WebCore/rendering/RenderBlock.cpp:3847
>  +                  hitTestContents(request, result, x, y, finalX, finalY, hitTestAction);
> no return needed here? 

No return needed here, since the caller site handles it.

Furthers changes:

1) Renamed HitTestResult::addRawNodeForRegionTest to addNodeToRectBasedTestResult
2) Renamed HitTestResult::isRegionTest to isRectBasedTest
3) Renamed HitTestResult::rawNodeList to rectBasedTestResult
4) Renamed HitTestResult::regionFromPoint to rectFromPoint
5) Made addRawNodeForRegionTest no-op in case of non rect-based hit tests.

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list