[webkit-reviews] review canceled: [Bug 84479] [chromium] Some background filters require inflating damage on the surface behind them : [Attachment 138145] Patch

[bugzilla-daemon at webkit.org]

Dana Jansens <danakj at chromium.org> has canceled Dana Jansens
<danakj at chromium.org>'s request for review:
Bug 84479: [chromium] Some background filters require inflating damage on the
surface behind them
https://bugs.webkit.org/show_bug.cgi?id=84479

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

------- Additional Comments from Dana Jansens <danakj at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=138145&action=review


>> Source/WebCore/ChangeLog:3
>> +	    [chromium] Some background filters require inflating damage on the
surface behind them
> 
> "on the surface behind them" is probably misleading here... =)   If I
understood correctly, should it say "Some background filters require inflating
damage that occurs behind it"

The only thing to consider is that the inflation happens in the space of the
surface, not the "blurring" layer's space. So really the damage is inflated in
the surface (through the lens of the layer above) kind of.. So I'd prefer to
keep the name as is.

>>> Source/WebCore/platform/graphics/chromium/cc/CCDamageTracker.cpp:64
>>> +
>> 
>> Personally I would prefer not to have a function that has two simultaneous
purposes like this.  Could we separate this into two helper functions:
>>   bool needsToExpandDamage()
>>   void expandPixelOutsetsForFilters()
> 
> Sure I'll split it up

needsToExpandDamage() == filters.hasFilterThatMovesPixels() so I just used that
directly.

This let me merge the 3 helper functions into a single
expandPixelOutsetsWithFilters() so yay.