Summary: | Fix usage of layout types in platform code | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Emil A Eklund <eae> | ||||||
Component: | Platform | Assignee: | Emil A Eklund <eae> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | eric, leviw, rakuco, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 60318 | ||||||||
Attachments: |
|
Description
Emil A Eklund
2012-05-02 11:18:21 PDT
Created attachment 139843 [details]
Patch
Comment on attachment 139843 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139843&action=review > Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:552 > + return enclosingIntRect(m_layerTransform.combined().inverse().clampedBoundsOfProjectedQuad(FloatQuad(FloatRect(m_webGraphicsLayerClient->visibleContentsRect())))); Longest line ever. :) I'm surprised the FloatRect() constructor is needed for this. Maybe even the FloatQuad() one is implicit too? > Source/WebKit/chromium/src/WebHitTestResult.cpp:50 > + return WebPoint(roundedIntPoint(m_private->localPoint())); Does WebPoint not implicitly construct from IntPoint? > Source/WebKit/chromium/src/WebSurroundingText.cpp:49 > + VisiblePosition visiblePosition(node->renderer()->positionForPoint(static_cast<IntPoint>(hitTestInfo.localPoint()))); Does .flooredPoint() not exist? static_cast seems like an odd way to do this. (In reply to comment #2) > (From update of attachment 139843 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=139843&action=review > > > Source/WebKit2/WebProcess/WebCoreSupport/WebGraphicsLayer.cpp:552 > > + return enclosingIntRect(m_layerTransform.combined().inverse().clampedBoundsOfProjectedQuad(FloatQuad(FloatRect(m_webGraphicsLayerClient->visibleContentsRect())))); > > Longest line ever. :) I'm surprised the FloatRect() constructor is needed for this. Maybe even the FloatQuad() one is implicit too? Was needed to avoid ambiguous type errors. > > > Source/WebKit/chromium/src/WebHitTestResult.cpp:50 > > + return WebPoint(roundedIntPoint(m_private->localPoint())); > > Does WebPoint not implicitly construct from IntPoint? It should but again there might be ambiguity. This really seems like it should work without it though. > > Source/WebKit/chromium/src/WebSurroundingText.cpp:49 > > + VisiblePosition visiblePosition(node->renderer()->positionForPoint(static_cast<IntPoint>(hitTestInfo.localPoint()))); > > Does .flooredPoint() not exist? static_cast seems like an odd way to do this. The static_cast is not used to floor the value, it is used to convert the WebPoint to an IntPoint (as the implicit conversion gets confused by the multiple available conversion paths). I described this in the ChangeLog, let me know if you'd like me to also add a comment to that effect. Created attachment 139861 [details]
Patch for landing
Comment on attachment 139861 [details] Patch for landing Clearing flags on attachment: 139861 Committed r115877: <http://trac.webkit.org/changeset/115877> All reviewed patches have been landed. Closing bug. |