| Summary: | Empty float rects should remain empty after integrally enclosing them. | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Matt Woodrow <mattwoodrow> | ||||||||||||||||
| Component: | Layout and Rendering | Assignee: | Matt Woodrow <mattwoodrow> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | bfulgham, changseok, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, jfernandez, kondapallykalyan, mmaxfield, pdr, rego, sabouhallawa, schenney, sergio, simon.fraser, svillar, webkit-bug-importer, zalan | ||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=132753 | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Matt Woodrow
2022-03-06 16:04:22 PST
Created attachment 453935 [details]
Patch
Created attachment 453941 [details]
Patch
Created attachment 453943 [details]
Patch
Created attachment 454014 [details]
Patch
Comment on attachment 454014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454014&action=review > Source/WebCore/platform/graphics/FloatRect.cpp:261 > + FloatPoint location = flooredIntPoint(rect.minXMinYCorner()); Why converting the floored (integral) location value to float? > Source/WebCore/platform/graphics/FloatRect.cpp:263 > + return IntRect(IntPoint(location), IntSize()); nit: maybe? return { flooredIntPoint(rect.location()), { } }; -isEmpty() may also indicate negative width/height values so we end up converting them to 0, but probably it's ok -or alternatively you could floor them as well. > Source/WebCore/platform/graphics/FloatRect.cpp:264 > + FloatPoint maxPoint = ceiledIntPoint(rect.maxXMaxYCorner()); same here. (In reply to zalan from comment #7) > Comment on attachment 454014 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=454014&action=review > > > Source/WebCore/platform/graphics/FloatRect.cpp:261 > > + FloatPoint location = flooredIntPoint(rect.minXMinYCorner()); > > Why converting the floored (integral) location value to float? It's so that the 'maxPoint - location' calculation happens using float range/precision, not int. This is the same behaviour as the existing 'enclosingIntRect' function has (which has an API test that requires this behaviour). > > > Source/WebCore/platform/graphics/FloatRect.cpp:263 > > + return IntRect(IntPoint(location), IntSize()); > > nit: maybe? return { flooredIntPoint(rect.location()), { } }; > -isEmpty() may also indicate negative width/height values so we end up > converting them to 0, but probably it's ok -or alternatively you could floor > them as well. I'm not sure there's a need to preserve negative sizes rects, but happy to make this change if you think it's worth it. Will make the {} change! Comment on attachment 454014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454014&action=review >>> Source/WebCore/platform/graphics/FloatRect.cpp:261 >>> + FloatPoint location = flooredIntPoint(rect.minXMinYCorner()); >> >> Why converting the floored (integral) location value to float? > > It's so that the 'maxPoint - location' calculation happens using float range/precision, not int. This is the same behaviour as the existing 'enclosingIntRect' function has (which has an API test that requires this behaviour). hah, I wrote that code and added the test case. :/ oh well. Created attachment 454170 [details]
Patch for landing
Created attachment 454191 [details]
Patch for landing
ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Created attachment 454274 [details]
Patch for landing
Committed r291080 (248245@main): <https://commits.webkit.org/248245@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454274 [details]. |