Summary: | CoordinatedGraphics: Fix integer rounding when computing pixel alignment | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sergio Correia (qrwteyrutiyoup) <sergio> | ||||||
Component: | New Bugs | Assignee: | Sergio Correia (qrwteyrutiyoup) <sergio> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cmarcelo, commit-queue, kondapallykalyan, luiz, noam, tonikitoo | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Sergio Correia (qrwteyrutiyoup)
2013-10-09 12:09:24 PDT
Created attachment 213797 [details]
Patch
Proposed patch
Comment on attachment 213797 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213797&action=review > Source/WebCore/ChangeLog:24 > + In some situations, such as dealing with a mobile browser, which tries to > + calculate a scale factor to fit the contents to the viewport, we may get a > + small value for either width or height after scaling, which will cause the > + resulting FloatRect after being rounded to have this value be zero. > + > + After converting back to the layer coordinates, we will still have this one > + value be zero, and that will cause problems in createTilesIfNeeded(), since > + in a previous step it will remove the layer's backing store, due to failing > + the check in layerShouldHaveBackingStore(), as the layer will be considered > + empty. At this point, we hit ASSERT(backingStore) in createTilesIfNeeded(). > + > + In this patch we replace the roundedIntRect() call with enclosingIntRect() > + in CoordinatedGraphicsLayer::computePixelAlignment(). Too much information here! All you're doing is changing to enclosingIntRect so that very small rectangles (after content scaling) don't appear to be empty, thus creating a crash later. (In reply to comment #2) > (From update of attachment 213797 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=213797&action=review > > > Source/WebCore/ChangeLog:24 > > + In some situations, such as dealing with a mobile browser, which tries to > > + calculate a scale factor to fit the contents to the viewport, we may get a > > + small value for either width or height after scaling, which will cause the > > + resulting FloatRect after being rounded to have this value be zero. > > + > > + After converting back to the layer coordinates, we will still have this one > > + value be zero, and that will cause problems in createTilesIfNeeded(), since > > + in a previous step it will remove the layer's backing store, due to failing > > + the check in layerShouldHaveBackingStore(), as the layer will be considered > > + empty. At this point, we hit ASSERT(backingStore) in createTilesIfNeeded(). > > + > > + In this patch we replace the roundedIntRect() call with enclosingIntRect() > > + in CoordinatedGraphicsLayer::computePixelAlignment(). > > Too much information here! All you're doing is changing to enclosingIntRect so that very small rectangles (after content scaling) don't appear to be empty, thus creating a crash later. Oops sorry. I was trying to explain the problem I observed, but I will use your words and make it way shorter. (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 213797 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=213797&action=review > > > > > Source/WebCore/ChangeLog:24 > > > + In some situations, such as dealing with a mobile browser, which tries to > > > + calculate a scale factor to fit the contents to the viewport, we may get a > > > + small value for either width or height after scaling, which will cause the > > > + resulting FloatRect after being rounded to have this value be zero. > > > + > > > + After converting back to the layer coordinates, we will still have this one > > > + value be zero, and that will cause problems in createTilesIfNeeded(), since > > > + in a previous step it will remove the layer's backing store, due to failing > > > + the check in layerShouldHaveBackingStore(), as the layer will be considered > > > + empty. At this point, we hit ASSERT(backingStore) in createTilesIfNeeded(). > > > + > > > + In this patch we replace the roundedIntRect() call with enclosingIntRect() > > > + in CoordinatedGraphicsLayer::computePixelAlignment(). > > > > Too much information here! All you're doing is changing to enclosingIntRect so that very small rectangles (after content scaling) don't appear to be empty, thus creating a crash later. > > Oops sorry. I was trying to explain the problem I observed, but I will use your words and make it way shorter. It's ok! Better have too much information than too little :) Created attachment 213800 [details]
v2--comment=More succint ChangeLog.
Comment on attachment 213800 [details] v2--comment=More succint ChangeLog. Clearing flags on attachment: 213800 Committed r157180: <http://trac.webkit.org/changeset/157180> All reviewed patches have been landed. Closing bug. |