Bug 179924 - Some FELighting cleanup
Summary: Some FELighting cleanup
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: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-21 09:47 PST by Simon Fraser (smfr)
Modified: 2017-11-22 10:09 PST (History)
7 users (show)

See Also:


Attachments
Patch (22.20 KB, patch)
2017-11-21 09:49 PST, Simon Fraser (smfr)
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2017-11-21 09:47:42 PST
Some FELighting cleanup
Comment 1 Simon Fraser (smfr) 2017-11-21 09:49:44 PST
Created attachment 327418 [details]
Patch
Comment 2 Sam Weinig 2017-11-21 15:22:22 PST
Comment on attachment 327418 [details]
Patch

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

I continue to wish we had a vector type as IntSize is weird.  I can't remember why we don't have one.  Is it because we don't want another type representing a pair of ints?  Or is it naming? Or something else.

> Source/WebCore/platform/graphics/filters/FELighting.cpp:95
> +inline IntSize FELighting::LightingData::topLeftNormal(int offset) const

I wish we had a better type for this type of vector.
Comment 3 Simon Fraser (smfr) 2017-11-21 17:03:02 PST
https://trac.webkit.org/r225088
Comment 4 Radar WebKit Bug Importer 2017-11-21 17:10:34 PST
<rdar://problem/35661516>
Comment 5 Darin Adler 2017-11-22 10:09:41 PST
Comment on attachment 327418 [details]
Patch

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

> Source/WebCore/platform/graphics/filters/FELighting.cpp:248
> +        normalVector.setX(factorX * static_cast<float>(normal2DVector.width()) * data.surfaceScale);
> +        normalVector.setY(factorY * static_cast<float>(normal2DVector.height()) * data.surfaceScale);

The typecast to float here seems unnecessary. I expect the compiler can handle float * int * float -> float without a typecast.