Bug 181720 - [Cairo] Use one-time ShadowBlur objects when performing shadowing
Summary: [Cairo] Use one-time ShadowBlur objects when performing shadowing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-01-16 23:02 PST by Zan Dobersek
Modified: 2018-01-17 01:21 PST (History)
3 users (show)

See Also:


Attachments
Patch (39.26 KB, patch)
2018-01-16 23:15 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch for landing (39.11 KB, patch)
2018-01-17 01:19 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2018-01-16 23:02:46 PST
[Cairo] Use one-time ShadowBlur objects when performing shadowing
Comment 1 Zan Dobersek 2018-01-16 23:15:06 PST
Created attachment 331471 [details]
Patch
Comment 2 Carlos Garcia Campos 2018-01-17 00:54:21 PST
Comment on attachment 331471 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331471&action=review

> Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:188
> +    ShadowBlur shadow(FloatSize { shadowState.blur, shadowState.blur },

Do you really need to use FloatSize explicitly?

> Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:472
> +bool ShadowState::shadowVisible() const

shadow is redundant in method name, I would use isVisible

> Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:477
> +bool ShadowState::shadowRequired(PlatformContextCairo& platformContext) const

Ditto.

> Source/WebCore/platform/graphics/cairo/CairoOperations.h:116
> +    float blur;

Why did you remove the initialization here?
Comment 3 Zan Dobersek 2018-01-17 01:17:47 PST
Comment on attachment 331471 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=331471&action=review

>> Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:188
>> +    ShadowBlur shadow(FloatSize { shadowState.blur, shadowState.blur },
> 
> Do you really need to use FloatSize explicitly?

No, I can change it to use braces.

>> Source/WebCore/platform/graphics/cairo/CairoOperations.cpp:472
>> +bool ShadowState::shadowVisible() const
> 
> shadow is redundant in method name, I would use isVisible

OK.

>> Source/WebCore/platform/graphics/cairo/CairoOperations.h:116
>> +    float blur;
> 
> Why did you remove the initialization here?

Mistake.
Comment 4 Zan Dobersek 2018-01-17 01:19:38 PST
Created attachment 331475 [details]
Patch for landing
Comment 5 Zan Dobersek 2018-01-17 01:20:57 PST
Comment on attachment 331475 [details]
Patch for landing

Clearing flags on attachment: 331475

Committed r227051: <https://trac.webkit.org/changeset/227051>
Comment 6 Zan Dobersek 2018-01-17 01:21:01 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2018-01-17 01:21:31 PST
<rdar://problem/36576268>